Repository navigation
Conversation
- Added assertion on evaluation of an enumeration meta-property (visibility). - Test fails.
- Extended EvaluationUtil.elementFor and results to handle metamodel enum values. - Revised the use of EvaluationUtil.results in ModelLevelExpressionEvaluatior.
|
@adaussy |
| object instanceof Double? literalRational((Double)object): | ||
| object instanceof Element? ElementUtil.getMetaclassFeatureFor((Element)object): | ||
| null; | ||
| enumUsage(context, object); |
There was a problem hiding this comment.
It is usually bad practice to nest some many ternary expression.
I would propose:
// At least
switch (object) {
case Boolean b -> literalBoolean(b);
case String s -> literalString(s);
case Integer i -> literalInteger(i);
case Double d -> literalRational(d);
case Element el -> ElementUtil.getMetaclassFeatureFor(el);
default -> enumUsage(context, object);
}
//Maybe better
switch (object) {
case Boolean b -> literalBoolean(b);
case String s -> literalString(s);
case Integer i -> literalInteger(i);
case Double d -> literalRational(d);
case Element el -> ElementUtil.getMetaclassFeatureFor(el);
case Enum<?> enumeration -> enumUsage(context, object);
default -> throw new IllegalArgumentException("Unexpected expression value: " + object);
}
There was a problem hiding this comment.
I don't think the change you propose necessary to make for this PR. In any case, I think I prefer the old school ternary expressions.
There was a problem hiding this comment.
As you wish, but my point was that the second version explicitly distinguishes between case Enum<?> and the default case, allowing us to detect invalid objects early.
With this implementation (which could also be written using a ternary operator), you can be sure that enumUsage is only called with an actual enum value. I did not test that code and it is just a suggestion.
There was a problem hiding this comment.
I see your point, but I would prefer not to use an exception here, since that has not been the convention elsewhere. The change as coded means that elementFor will still return null if it does not recognize the input object, as it did before. This is a more conservative change that seems sufficient for the current implementation.
| return SysMLFactory.eINSTANCE.createLiteralInfinity(); | ||
| } | ||
|
|
||
| public static Feature enumUsage(Element context, Object object) { |
There was a problem hiding this comment.
It is really hard for me to test this part because I don't know yet how to launch an evaluation but I trust it is working.
There was a problem hiding this comment.
You can install a development version of the Jupyter deployment locally as follows:
- Install the latest public release of the Jupyter deployment from the URL below (follow the
README).
https://github.com/Systems-Modeling/SysML-v2-Release/tree/master/install/jupyter - Install the latest development version of the Jupyter kernel from
org.omg.sysml.jupyter.kernel. There is anotherREADME, but the basic steps are as follows.(for Windows, runorg.omg.sysml.jupyter.kernel % jupyter kernelSpec remove sysml org.omg.sysml.jupyter.kernel % ./installKernel.shinstallKernel.bat) - Run
jupyter lab.
Alternately, org.omg.sysml.interactive.SysMLInteractive, if executed directly, has a simple "Jupyter-like" REPL for testing and debuging. There is a launcher for this in org.omg.sysml.interactive, so it can be run from inside the Eclipse workbench.
1> package P { attribute x; }
Package P (0c550358-243d-4cf0-9c94-84e27af5a6cf)
2> %eval (P meta SysML::Package).ownedMembership.visibility
Feature public (7b966ee0-066c-550b-9684-89243d16c6b1)
3>
Note that quotes are required around this expression argument to %eval in the Jupyter magic command syntax, but not when using the SysMLInteractive REPL directly.
| Class<?> class_ = object.getClass(); | ||
| if (class_ != null) { | ||
| String name = class_.getSimpleName() + "::" + object; | ||
| Type type = SysMLLibraryUtil.getLibraryType(context, |
There was a problem hiding this comment.
I guess it is acceptable for now but should not we have an exhaustive list of enumeration available in the core librairies in order to have a direct mapping between java instances and Kerml elements ?
There was a problem hiding this comment.
Do you mean a list of enumeration definitions kept in memory? If so, that would basically be caching what is computed here. Calling getLibraryType would still be necessary, since the actual Java object varies each time the library is loaded and parsed. But such a cache does not seem necessary for performance, since the method getLibraryType has proven to be pretty fast (at least the Xtext version).
Further, we could pre-initialize with the current "exhaustive list" of enumeration types in the abstract syntax, but this list would change if a new enumeration was added to the abstract syntax in a future language revision, which could be easily missed in maintenance. It is better to do the look up what is actually in the library.
There was a problem hiding this comment.
That works for me.
My main point was to have an explicit mapping between Java objects and their counterparts in the library. I tend to find naming conventions a bit less robust, but I'm fine with this approach as well.
There was a problem hiding this comment.
In this case, the Java enum and the reflective KerML/SysML enumerations are ultimately generated from the same source: the normative UML/MOF abstract syntax model. Unless there has been a release development error, the names have to correspond.
This PR fixes a bug in model-level evaluation that prevented the evaluation of abstract syntax properties with enumeration types.
Issue
Evaluating a metamodel feature-chain expression ending in a meta-property with an enumeration type caused an exception. For example, given the model
evaluating
(P meta SysML::Package).ownedMembership.visibilityin Jupyter resulted in the exceptionNote that this issue happened only for reflective access to abstract syntax meta-properties with UML/MOF enumeration types. There is no issue with accessing regular SysML v2 enumeration or attribute usages with SysML v2 enumeration definitions.
Causes
EvaluationUtil.elementFordid not handle enumeration literals, returning null.Evaluation.resultsreturned a list containing a Java null when there was only a single result value andelementForreturned null, rather than returning the empty list (which is "null" in KerML/SysML v2 expression semantics). It was this Java null that, later, caused the exception.Changes
enumUsage, which looks up the proper enumerated value in the KerML or SysML reflective model. Note that this requires that acontextelement to be used withSysMLLibraryUtil.getLibraryType. If no such enumerated value existsts, null is returned.elementForto callenumUsageas the final option, rather than just returning null. Also added acontextparameter to be passed toenumUsage.resultsand added acontextparameter.evaluateFeature, revised calls toresultsto pass thefeatureas thecontext.