Conform Behavior Annex expression precedence, admissible forms, and grouping to AS5506/3 🤖 - #3220
Merged
Merged
Conversation
Model the AS5506/3 Rev A D.7 expression forms whose grouping the front end does not preserve, and the forms its grammar admits although the standard grammar does not. The accepted fixture combines mixed logical operators, a parenthesized logical group, a signed exponentiation, a signed multiplying term, and a parenthesized exponentiation chain in one assignment sequence. The assertions render the declarative model as a prefix tree and the strict model in the standard's value_expression/relation/simple_expression/term/ factor nesting, so a grouping difference cannot be hidden by textual association. They record that the declarative model currently associates 'or' below 'and' rather than left to right, that the strict model splices a parenthesized logical group into the enclosing expression and so turns 'left and (middle or right)' into '(left and middle) or right', that '-2 ** 2' becomes '(-2) ** 2', that the unary minus of '-total mod 3' is dropped outright, and that the unparser drops explicit parentheses while inventing parentheses for its own association. Four separate fixtures cover the forms the standard grammar does not admit: an unparenthesized exponentiation chain, a repeated unary adding operator, a signed operand of 'abs', and a repeated 'not'. Each is currently accepted without a diagnostic. The parenthesized exponentiation chain is the positive control: its strict structure is already correct and must stay so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the one-rule-per-operator expression grammar with the D.7 productions themselves: value_expression, relation, simple_expression, term, factor, and value. All logical operators now share one precedence level and associate from left to right, so 'a or b and c' is '(a or b) and c'. The unary adding operator leads a whole simple expression and therefore binds below the multiplying and numeric operators, making '-a mod b' mean '-(a mod b)' and '-2 ** 2' mean '-(2 ** 2)'. A factor admits at most one exponentiation and a single value as the operand of 'abs' or 'not', so 'a ** b ** c', '- -a', 'abs -a', and 'not not a' are syntax errors as the standard grammar requires. Unary plus stays accepted; rejecting it is issue #3170. A parenthesized value expression becomes a ParenthesizedExpression of its own, which lets the translator stop inferring grouping from node text. appendLogical flattens only the left-associated chain the grammar builds, so a group stays one relation instead of being spliced into the enclosing expression, and toValue turns it into the nested strict value expression D.7's value production admits. Deleting the text heuristic also restores two operators the old translator dropped: a unary minus inside a term was discarded outright, and a signed exponentiation base kept the sign in the wrong place. Because a group is now an object, the serializer emits the requested parentheses from the model rather than reconstructing them from the parse text, and each expression object's reported source range excludes the enclosing parentheses. That range correction is the only change to the existing characterization baselines: four positions goldens move a parenthesized operand's span onto the new group, and no diagnostics, resolved-model, or unparse golden changes, because the model goldens record containment rather than operator attributes. Regenerate the Xtext artifacts and update the conformance report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3177
Cause and correction
The expression grammar had one rule per operator level, which did not match the AS5506/3 Rev. A D.7 productions:
LogicalAndExpressionsat belowLogicalOrExpression, soandbound tighter thanor/xor. D.7 puts every logical operator on one precedence level and associates a level from left to right, makinga or b and cmean(a or b) and c.UnaryExpressionwas the operand ofPowerExpression, so a sign bound inside the exponentiation. D.7 admits the unary adding operator only ahead of the first term of asimple_expression, so it binds below the multiplying and numeric operators:-2 ** 2is-(2 ** 2)and-a mod bis-(a mod b).PowerExpressionandUnaryExpressionwere both recursive, acceptinga ** b ** c,- -a,abs -a, andnot not a. D.7'sfactoradmits at most one exponentiation and a singlevalueas the operand ofabsornot, so those forms require parentheses.PrimaryExpression, so a group left no object behind.DeclarativeToStrictTranslator.appendLogicaltherefore flattened logical expressions without regard for grouping, andtoSimpleExpression/toFactor/toValuetried to recover the grouping from node text with anisParenthesizedheuristic and aconsumedParenthesesre-entry guard.The correction makes the grammar the D.7 productions themselves —
value_expression,relation,simple_expression,term,factor,value— and adds an explicitParenthesizedExpression. With a group present as an object, the text heuristic and its guard are deleted:appendLogicalflattens only the left-associated chain the grammar builds, so a group stays one relation, andtoValueturns it into the nested strict value expression D.7'svalueproduction admits.Removing the heuristic also restores two operators the translator silently dropped.
toFactorhandled aUnaryExpressionwhose operator was neitherabsnornotby setting no operator at all, so a unary minus inside a term vanished:-total mod 3translated tototal mod 3. And-2 ** 2translated as(-2) ** 2.Unary plus stays accepted. Rejecting it is #3170, tracked separately, and its conformance expectation remains skipped.
Two consequences worth noting for review. The serializer now emits the requested parentheses from the model instead of reconstructing them from the parse text, so an explicitly grouped expression survives a model-to-text round trip. And each expression object's reported source range now excludes the enclosing parentheses, because the group owns them.
Regression model and assertions
ba/org.osate.ba.tests/models/issue3177/holds five fixtures in one project.Issue3177.aadlis the accepted case: one assignment sequence with mixed logical operators, a nested logical group, a signed exponentiation, a signed multiplying term, and a parenthesized exponentiation chain.PowerChain.aadl,RepeatedUnaryMinus.aadl,AbsNegatedOperand.aadl, andRepeatedNot.aadleach hold one form the standard grammar does not admit.Issue3177Testrenders the declarative model as a prefix tree and the strict model in the standard'svalue_expression/relation/simple_expression/term/factornesting, parenthesizing a nested value expression only, so a grouping difference cannot be hidden by textual association. It asserts both renderings per expression, an unparse/reparse round trip through the registered annex unparser on a copy with no node model, and a syntax error for each rejected fixture. The group is read reflectively byeClass()name so the test compiles against the pre-fix model.All 11 tests fail on the regression commit for their intended reasons:
left and (middle or right)left and middle or right— a changed meaningleft and (middle or right)-2 ** 2(-2) ** 2-2 ** 2-total mod 3total mod 3— sign dropped-total mod 3left or middle and right(or left (and middle right))(and (or left middle) right)(total ** 2) ** 3is the positive control: its structure was already correct and stays so.Characterization baselines
25 new goldens for the new fixtures. Only four existing goldens change, all in
expected/positions, and all because a parenthesized operand's source range moves onto the new group — for examplenot (input'fresh)inGrammarHazards.aadl, where theReferenceExpressionhad claimed the parentheses. Nodiagnostics,diagnostics-validated,resolved-model, orunparsegolden changes, because the model goldens record containment rather than operator attributes; that limitation is item 4 of "What the existing tests do and do not prove" inba/doc/conformance.md, and it is whyIssue3177Testasserts the attributes directly.Validation
Focused regression, from the repository root:
Tests run: 11, Failures: 0after the fix;Tests run: 11, Failures: 11on the regression commit alone.Both Behavior Annex test bundles, run separately so a zero-match selection could not hide behind a green reactor:
org.osate.ba.tests144 tests andorg.osate.xtext.aadl2.ba.tests15 tests, 0 failures.Clean root-reactor build with tests:
BUILD SUCCESS, 1604 tests, 0 failures, 0 errors, 2 skipped. The two skips are the conformance expectations tracked against #3170 and #3173, enforced by
BehaviorAnnexConformanceTest.everyStandardExpectationNamesItsTrackingIssue.The Xtext artifacts were regenerated with the
GenerateBehaviorAnnex.mwe2launch in Eclipse. A control run on the unmodified grammar reproduced the committed tree byte for byte beforehand, and the Eclipse and headless runs of the changed grammar produced byte-identical output, so the generated diff is exactly the generator's.org.osate.ba/model/aadlba.ecoreneeds no change: the strictValueExpressionis already aValue, andAadlBaTypeChecker.checkValuealready recurses into a nested value expression.Dependencies and merge order
None. The branch is based on current
master(de416eaf4a, the #3219 merge) and touches no file that another open pull request changes. It does not depend on #3170.Residual risk
:=rather than at the offending second operator, so the message location is coarser than ideal. The goldens record the exact text.andbinds tighter thanor, or that a parenthesized expression leaves no object, sees different structure. The only consumers in the reactor are the translator andIssue3169Test; both are covered by the build above.BehaviorAnnexProposalProvideroverrides nothing for the renamed rules, so content assist follows the regenerated grammar access with no hand-written counterpart to update.conformance.mdtracking table and its closed-since-the-review list were inconsistent for findings closed by earlier pull requests. They now agree, verified by comparing the two sets, and the stale claim that G12 and its follow-ups were unmerged is gone.🤖 Generated with Claude Code