Systems-Modeling / SysML-v2-Pilot-Implementation

Proof-of-concept pilot implementation of the SysML v2 textual notation and visualization
GNU Lesser General Public License v3.0
130 stars 24 forks source link

ST6RI-776/779 Updates from KerML FTF2 Ballot 1/Concrete and abstract syntax updates related to imports #582

Closed seidewitz closed 4 months ago

seidewitz commented 4 months ago

This PR implements resolutions of the following issues from KerML FTF2 Ballot 1:

The resolution of the following issue from KerML FTF2 Ballot 1 was already consistent with previous implementation:

The PR also implements resolution of the following related issues from SysML v2 FTF2 Ballot 1:

Finally, the PR implements resolution to two issues found during implementation of other resolutions, even though these resolutions have not been voted on yet by the KerML FTF2 (because implementation would otherwise have been blocked):

The major functional change is the resolution to KERML_-74, which requires imports to be private by default in the abstract syntax and for visibility to be shown explicitly on import declarations in the textual notation. In order to allow a transitional period for users, this change is not fully implemented in this PR. Instead, default-public import declarations are still allowed in the textual notation, but they result in a warning message: "Default public import is deprecated; make private if possible". Such declarations will be invalid in a subsequent release.

himi commented 4 months ago

I would like to review it, hopefully tonight...

himi commented 4 months ago

KERML_-37 said the name of a new derived property is targetFeature while the name of the implementation is featureTarget. Is it correct? I'm guessing the actual approved resolution may be consistent but I cannot access to the original Jira issue.

seidewitz commented 4 months ago

KERML_-37 said the name of a new derived property is targetFeature while the name of the implementation is featureTarget. Is it correct? I'm guessing the actual approved resolution may be consistent but I cannot access to the original Jira issue.

I filled two new issues due to implementation problems with the resolutions to KERML-37 and KERML-73. I meant to mention this in the PR description, but I forgot to do that. I have now updated the description to mention it.

The resolution to KERML-88 calls for using the name featureTarget instead of targetFeature, in order to avoid conflict with the existing use of targetFeature in subclasses of Feature. The resolution to KERML-90 revises the OCL for the new constraint validateImportTopLevelVisibility added by the resolution to KERML_-73.

seidewitz commented 4 months ago

Note also that, yesterday, I seem to have accidentally pushed commits from branch ST6RI-777 onto ST6RI-779. I have now force pushed branch ST6RI-777 back to the commit it should be on for review.

himi commented 4 months ago

I tested https://issues.omg.org/issues/KERML_-67 and it worked as intended:

package TestEval0 {
    behavior B {in x; out y = x + 1; out z = x + y;}
    feature y1 = B(1).y;
    feature b = B(2);
    feature y2 = b.y;
    feature z2 = b.z;
}

it is visualized with EVAL as below: image

However, I also tried the following SysML by using calc but it cannot be parsed. I think it will be supported in the future: Sorry I think it should be in SysMLv2FTF2 issues.

package TestEval1 {
    calc B { in x; out y = x + 1; out z = x + y;}
    attribute a1 = B(1).y;
    attribute a2 = B(2);
    attribute a3 = a2.y;
    attribute a4 = a2.z; 
}
himi commented 4 months ago

KERML_-37 said the name of a new derived property is targetFeature while the name of the implementation is featureTarget. Is it correct? I'm guessing the actual approved resolution may be consistent but I cannot access to the original Jira issue.

I filled two new issues due to implementation problems with the resolutions to KERML-37 and KERML-73. I meant to mention this in the PR description, but I forgot to do that. I have now updated the description to mention it.

The resolution to KERML-88 calls for using the name featureTarget instead of targetFeature, in order to avoid conflict with the existing use of targetFeature in subclasses of Feature. The resolution to KERML-90 revises the OCL for the new constraint validateImportTopLevelVisibility added by the resolution to KERML_-73.

Thank you. I understand the situation.

seidewitz commented 4 months ago

@ujhelyiz

Furthermore, there are a few intersect specifications added to datatypes, e.g. NumericalVectorValues and CartesianVectorValues in the file VectorValues.kerml, or OrderedSet in Collections.kerml. These changes seem correct, but were not mentioned in the PR description (or I could not find why they were required).

These are old changes in the standard library models (see commit 992c6c5). The updates in this PR are to the copies of the library models used with the Xpect tests. It seems that the earlier library model changes adding the "intersects" never got copied into the Xpect library directories (since we don't do any checking on "intersects", it doesn't effect the tests anyway). Since the adding of visibility to imports was fairly pervasive, I simply updated the Xpect library models with all the latest standard library models. That is why you happened to see the unexpected addition of "intersects" in this PR.

seidewitz commented 4 months ago

@himi

However, I also tried the following SysML by using calc but it cannot be parsed.

The equivalent of a KerML behavior in SysML v2 is an action def, not a calc. Also, the result of B(2) will now be an action, not an atttribute value. So, the following parses:

package TestEval1 {
    action def B { in x; out y = x + 1; out z = x + y;}
    attribute a1 = B(1).y;
    action a2 = B(2);
    attribute a3 = a2.y;
    attribute a4 = a2.z; 
}

And it evaluates correctly: image