SemanticApplicationDesignLanguage / sadl

Semantic Application Design Language (SADL) Open Source Code
http://semanticapplicationdesignlanguage.github.io/sadl/
Eclipse Public License 1.0
30 stars 12 forks source link

Error with particular ordering of declarations #820

Closed AbhaMoitra closed 2 years ago

AbhaMoitra commented 2 years ago

I am using SADL Feature dated 05/10/2021 and I am seeing an error "Unexpected error checking value in range" for particular ordering of the declarations. Having the declaration prior to using it gives the error but the error goes away if the declaration is moved below the usage (which seems counter-intuitive to me). Note that a list is involved. Also, the Test.sadl file mimics what is coming from writing out a model so I have X1 is a {ExpNode and Node and Variable} where Variable is a type of ExpNode and ExpNode is a type of Node. The error also goes away if I use "X1 is a Variable".

So there may be two root issues in this example.

image

image

Test.zip

crapo commented 2 years ago

@AbhaMoitra , I believe that the particular error you describe has been fixed for some time. In current versions of the IDE you will get the same warning on the declaration of instance X1 whether it is at lilne 10 or at line 22. The warning will read,

"Multiple markers at this line

I believe this is correct. ExpNode is a subclass of Node, giving rise to the first marker. Variable is a subclass of ExpNode, giving rise to the second marker. The intersection of a class and a subclass is just the subclass, which is what is being used, i.e., X1 is a Variable. image

There does appear to be a List type checking bug in the current version of SADL, however, that is illustrated by your example. I get the warning on line 15, or an error if type checking errors as warnings is not checked in preferences,

"Multiple markers at this line

If I declare X1 and X0 to be instances of ExpNode I still get a single marker. I'll look into this.

AbhaMoitra commented 2 years ago

@crapo : Thank you. I updated my eclipse and SADL so that I am using eclipse 2021-09 and SADL 3.5.0 build of 20210831 (the latest). I have set "Type checking issues as warning only".

If I comment out lines 10-12 but keep lines 22-24 then I have 5 warnings but no errors. 4 of the warnings are about "Cycle encountered" and 1 warning on line 15 is "Type comparison not possible...".

If I keep lines 10-12 but comment out lines 22-24 then I have an error on line 14 "Unexpected error checking value in range". and I have 5 warnings. 4 of the warnings are about "Cycle encountered" and 1 warning on line 15 is "Type comparison not possible" (he warning is shorter than in the other scenario above).

This I think is different than what you wrote above.

crapo commented 2 years ago

@tuxji , @agabaldon , what is our release strategy? @AbhaMoitra was working with a version from 5/10/21 and even with an update of 8/31/21 has pretty different editor markers than the current development branch. Some 7 issues have been closed since 8/31, and there are other issues that are still open but may have been resolved. Is there a need for more process around verification and closing of issues and creating new releases? I will fix the issue that I find in @AbhaMoitra 's example, which is not the issue she was concerned about, and when all tests pass will submit a PR, What will happen thereafter and when?

crapo commented 2 years ago

@AbhaMoitra , the fix to this bug was checked in with commit on Oct 18. With your example using the current development branch, and after commenting out lines 10-12 and uncommenting lines 22-24, there are two warnings at line 15, which are from the bug I am currently working on fixing with respect to type checking of lists, two warnings at line 18 because of the intersection of a class with its subclass, and two more warnings at line 22 for the same reason.

tuxji commented 2 years ago

@crapo , I would appreciate your suggestion regarding SADL's release strategy. I don't think any of us has a release strategy.

crapo commented 2 years ago

@tuxji , I don't pretend to have a good answer. I doubt that any of us has the resources to "do it right." I would loosely characterize the strategy of the past as do beta releases often and alpha releases with a bit more verification less often. From a user's perspective, correct me @AbhaMoitra or @agabaldon if you disagree, the strategy was to create a new release, either a beta release on github or by dropping an unofficial release in a shared folder, to fix a problem impacting a user. Then that user could move forward without the problem and most other users could just continue using what they had. That is probably less adequate with more users, including external users. But I think there may be some merit in doing frequent beta releases whenever an issue deemed significant is resolved. The risk is that a new beta release may have some regression errors. I've tried to adopt the strategy that TypeFox followed of always adding one or more tests with any bug fix or new functionality to provide continued verification of the functionality. Our test coverage is far from complete, but it is enough to often be useful. For example, I did a fix to the bug that shows up on line 15 of @AbhaMoitra 's example in a new code branch, but when I did a build a number of other type checking tests failed. So now I'm fine-tuning my fix to have a much narrower impact.

AbhaMoitra commented 2 years ago

@crapo @tuxji @agabaldon : What Andy proposes sounds fine to me. Very often when I put in an issue, it is something that is blocking me and whenever that gets fixed in say an "unofficial release in a shared folder", I then to try it out and provide my feedback. Often the illustrative example I post is simplified from my actual project, so my testing of the fix may provide additional evidence.

crapo commented 2 years ago

I used the term beta release, but I should have used the GitHub term pre-release. One can mark a release on GitHub with the flag pre-release.

tuxji commented 2 years ago

OK, so we ought to do pre-releases whenever we fix a significant problem for a user and stable releases less often with more verification? Let's do that, although the most difficult part will be writing release notes or changelogs since I don't know very well everything that's changed between SADL releases (especially from a user's viewpoint, not a developer's viewpoint). Are you willing to make a pre-release and write notes, or are you willing to edit a pre-release's notes after I create a pre-release if I don't know what to say in the notes?

crapo commented 2 years ago

With respect to the List type checking bug, this should now be resolved.

@tuxji , I would be happy to write release/pre-release notes. I think it best if you create the release/pre-release after a significant merge. I will only create PRs, not push to development. One low-cost approach to releases is to turn a pre-release into a release after it has been used by at least some users for a while without encountering any issues.

The PR resolving this bug is significant enough to warrant a pre-release.

tuxji commented 2 years ago

@crapo I've made a pre-release (SADL 3.5.0 build 20211130). Will you please add some release notes mentioning any important changes between the last release and this pre-release from a user's viewpoint?

crapo commented 2 years ago

@tuxji , I've added release notes. The change list was very helpful! Not sure how you generated it, but thanks! @AbhaMoitra , I assume that this issue can now be closed once you've verified that you have the desired behavior for your use case.

tuxji commented 2 years ago

Thanks, @crapo! I simply went to the pre-release page (SADL v3.5.0 build 20211130), clicked the "Compare" button on the right side of the release's title, and picked the previous release's tag to compare against. Then I gave you that new page's URL.

I assume this issue can be closed too, but Abha opened it so let's wait for her to try the new pre-release and confirm her issue is fixed.

AbhaMoitra commented 2 years ago

@crapo @tuxji : I tried it out and the issue has been corrected. Thank you.