-
Notifications
You must be signed in to change notification settings - Fork 176
Declare impure #4686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Declare impure #4686
Conversation
Since they may delete a file there is no real discussion as far as I can see.
If we make an exception for print this could be skipped. But I don't see any problem with changing these examples as they should be _examples_, and their side-effect is what matters.
|
Looks good, but it might have been a better idea to not mix bug fixing and example enhancements in the same PR. |
As far as I can tell this is just bug-fixing. |
|
I wish I could provide a review after checking this with a non-Dymola tool implementing the purity rules… Who has such a tool? |
To me the tool-part would primarily be to check all uses of impure to see that there no additional issues (something that should have been done when pure/impure was added). But that shouldn't block this one. These changes one can be inspected by hand, and can easily be verified to be correct. The impacts of these are:
That lead to the following additional changes for ModelicaTest (seems I missed testing that previously):
That should be everything. |
None of these are problematic as far I can see and currently some of them are currently needed.