Accept a Behavior Annex loop that iterates an enclosing iterator 🤖 - #3228
Merged
Merged
Conversation
Issue3226.aadl nests iteration over an array of arrays: the outer loop iterates the behavior variable of an array-of-arrays type, and the inner loop iterates the outer iterator. Both loops write their classifier, so the model does not depend on #3173. AS5506/3 Rev A D.6 gives the iterator the name scope of the construct, where it acts as a local variable, and admits an array data component reference as the iterated values, so the inner loop is legal and its iterator takes the element type of the outer one. Two assertions cover it: validation reports nothing, and the translated inner loop carries the outer iterator as its iterated values. The second one is needed because a loop with no iterated values at all would also validate and would say nothing about what the loop iterates. NonIterableValues.aadl is the other side of the same cast: an integer literal and a data port are the two shapes the shared D.7 integer value grammar accepts as iterated values and no strict ElementValues can represent. Each must be reported on the values as written rather than throw out of translation. All three assertions fail today. Translation casts the translated value to ElementValues, and IterativeVariableHolder, BehaviorIntegerLiteral, and DataPortHolder are not ElementValues, so the ClassCastException escapes into Xtext's validator dispatch and both models report the opaque 'Error executing EValidator' at line 0. The corpus goldens record that current behavior: the validated diagnostics projection holds that one message for each model and the resolved model is <unparsed>, while the plain-AADL diagnostics goldens are empty, which is what pins that neither fixture has an unrelated core-AADL error. BehaviorAnnexCharacterizationTest and BehaviorAnnexTranslationTest fail on the same exception until the fix lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AS5506/3 Rev A D.6 admits an array data component reference as the iterated values of a for or forall, and a data component reference starts with a local variable identifier. D.6 also gives the iterator of the construct the name scope of that construct, where it acts as a local variable, so an enclosing iterator is one of the names the iterated values may write. Nested iteration over an array of arrays is written that way: the inner loop iterates the outer iterator, and its own iterator takes the element type of that one. IterativeVariableHolder did not implement ElementValues, so it could not be the iterated values of a strict loop. Translation cast the translated value to ElementValues unconditionally, and DataHolder is an IntegerValueVariable, so the holder passed the inner cast and failed the outer one. The ClassCastException escaped into Xtext's validator dispatch and the user got 'Error executing EValidator' at line 0 with no strict model at all. Make IterativeVariableHolder an ElementValues in aadlba.ecore, with the regenerated model code, switch case, and child creation descriptor. The existing type check needs nothing further: the holder is a ValueVariable, so it already resolves through the value branch of getTypeHolder, which processes the Data Model array representation for a value in a loop and gives the iterated values the element type and dimension of the iterator they name. The same cast could fail for every other shape the grammar accepts there and no ElementValues represents: an integer literal, a property reference, and a name that resolves to a data port or a port value. Test the translated value instead of casting it, leave what it cannot represent out of the strict model, and report it from the validator on the values as written. The check gates the strict checkers, like the neighboring checks for constructs the strict model cannot carry, since a loop that iterates nothing is not a loop the checkers can say anything useful about. Fixes #3226 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 #3226
Cause and correction
A
fororforallwhoseelement_valuesnamed an iterative variable threwClassCastExceptionout ofDeclarativeToStrictTranslator.Builder.toElementValues, and the Xtext validator turned that into the opaqueError executing EValidatormarker at line 0 with no strict model for the annex.Two things were wrong on that one line.
The nesting is legal and had nothing to become. AS5506/3 Rev A D.6 admits an array data component reference as the iterated values, a data component reference starts with a local variable identifier, and D.6 gives the iterator of the construct the name scope of that construct, where it acts as a local variable. Nested iteration over an array of arrays is written that way.
IterativeVariableHoldernevertheless did not implementElementValues, unlike every other holder that can appear there, soaadlba.ecorenow lists it, with the regenerated model code,AadlBaSwitchcase, and child creation descriptor. The existing type check needed nothing further: the holder is aValueVariable, so it resolves through the value branch ofAadlBaUtils.getTypeHolder, which processes the Data Model array representation for a value inside a loop and gives the iterated values the element type and dimension of the classifier the iterator names.The cast could fail for other shapes too. The iterated values share the D.7 integer value grammar with the rest of the annex, so an integer literal, a property reference, and a name that resolves to a data port or a port value reach the same cast, and none of them is an
ElementValueseither.toElementValuesnow tests the translated value instead of casting it, leaves what it cannot represent out of the strict model the way the rest of the translator does, andBehaviorAnnexValidator.checkIteratedValuesreports it on the values as written. That check gates the strict checkers, like the neighboring checks for constructs the strict model cannot carry, since a loop that iterates nothing is not a loop the checkers can say anything useful about.Regression
ba/org.osate.ba.tests/models/issue3226/Issue3226.aadlnests iteration over an array of arrays: the outer loop iterates a behavior variable of an array-of-arrays type and the inner loop iterates the outer iterator. Both loops write their classifier, so the model does not depend on #3173.Issue3226Testasserts that validation reports nothing, and separately that the translated inner loop carries the outer iterator as its iterated values — a loop with no iterated values at all would also validate and would say nothing about what the loop iterates.models/issue3226/NonIterableValues.aadlis the other side of the cast: an integer literal and a data port, each of which must be reported on the values as written rather than throw. The corpus goldens record both models; the plain-AADL diagnostics goldens are empty, which pins that neither fixture has an unrelated core-AADL error.All three assertions fail on
master— two on theClassCastException, one on theError executing EValidatormarker it produces — andBehaviorAnnexCharacterizationTestandBehaviorAnnexTranslationTestfail on the same exception with the fixtures present and the fix absent.Validation
Focused regression, before the fix (regression commit only):
Tests run: 3, Failures: 2, Errors: 1— the error is theClassCastExceptionatDeclarativeToStrictTranslator.toElementValues.Same command with the fix:
Tests run: 3, Failures: 0, Errors: 0.Clean root reactor with the complete test suite:
BUILD SUCCESS, no failures or errors in any module, including the wholeorg.osate.ba.testscorpus. Only the fourissue3226goldens changed between the regression commit and the fix commit; no other corpus model's diagnostics or resolved model moved.Dependencies and merge order
Based on
master(0def9cf), independent of the open Behavior Annex PRs, and it can merge in any order relative to them. It does textually conflict with #3227 (#3173) inBehaviorAnnexValidator.java: both add a check constant and a line to the same representability chain, adjacent to each other. Whichever merges second needs that one trivial resolution, keeping both checks.The two changes are complementary rather than competing: #3227 makes the iterator classifier optional and derives the iterator's type from the iterated values, and this PR makes an enclosing iterator one of the iterated values that can be written, so after both a nested loop can also omit the inner classifier.
Residual risk
Adding an interface supertype to an existing EClass shifts no classifier ID and adds no feature, and the regenerated diff is confined to the
implementsclause, onegetESuperTypes().add, one switch case, and one child creation descriptor plus its icon.org.osate.ba.aadlba.ElementValueshas no consumer outside the translator andAadlBaTypeChecker.checkElementValues, both exercised here.The new diagnostic is the first report for iterated values the strict model cannot carry, so models that previously crashed the validator now get an error instead. No model in the corpus writes one — the existing fixtures iterate event data ports, behavior variables, data fields, and integer ranges — so nothing else in the test suite changes.
Type conformance between the iterated values and the iterator is not checked here, because
AdaLikeDataTypeChecker.conformsTois disabled repo-wide; only the arrayness of the values is. This PR does not change that.🤖 Generated with Claude Code