metaborg / stratego

Apache License 2.0
10 stars 6 forks source link

Stratego 2 runtime does not respect modules #29

Open Thom747 opened 2 years ago

Thom747 commented 2 years ago

Describe the bug The Stratego 2 runtime bunches all definitions of a rule across different modules in one location, effectively removing the limited scope of rules within modules. The type-checker does respect module boundaries, so safe behavior according to the type-checker can result in unsafe behavior at run-time.

Project

Occurs when compiling and running Stratego 2.

Versions See Spoofax (Meta); Report Issue for information to copy here. No idea where to find this in Spoofax, looked for 15 minutes. Spoofax LWB 0.16.17.

To Reproduce

Observed behaviour

The usage of rule in B will non-deterministically choose between the rule in A and B, even though A is not imported in B. Note that the type-checker will not error here, even if the signatures of the two rules are incompatible, as it does respect module boundaries.

Expected behaviour

The usage of rule in B should always use the definition in B, as the definition of rule in A is not in scope.

Additional context This problem causes completely unexpected behavior, which is not caught by the type-checker.

Apanatshka commented 2 years ago

Sorry you ran into this.

Stratego 2 is a continuation of Stratego 1, which was originally designed and implemented within the world of C-like languages and therefore didn't have module discipline. There's a comment in the compiler code that suggests that the original implementors thought about it, but proper module discipline where you can have multiple distinct strategies with the same name in a project would require heavy modification in many parts of the compiler.

When I added the type system I wasn't aware of the bug you describe, and I implemented module discipline in the static semantics. I noticed this edge case during implementation, but it's been on my todo list ever since.

The first fix I'll likely implement for this is to make your example program illegal, i.e. you cannot have multiple distinct definitions of a strategy. In the long term, perhaps the compiler can refactored to allow for multiple distinct definitions of a strategy, but that's not something that can easily be done in the short term.

Thom747 commented 2 years ago

That sounds like a good approach for now at least. Another option would be to not allow multiple definitions across modules in general, as the non-deterministic choice that is done in such a case, even when neatly imported, can almost surely not be intended behavior for any programmers.

eelcovisser commented 2 years ago

That sounds like a good approach for now at least. Another option would be to not allow multiple definitions across modules in general, as the non-deterministic choice that is done in such a case, even when neatly imported, can almost surely not be intended behavior for any programmers.

Multiple definitions across modules should be non-overlapping. But it would be good to check that and give warnings in the case that there is overlap.

-- Eelco

— Reply to this email directly, view it on GitHub https://github.com/metaborg/stratego/issues/29#issuecomment-1022109911, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACDAFIVFQQFEP4JEAQSCW3UX7K63ANCNFSM5MYSUJEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Professor of Computer Science, TU Delft https://eelcovisser.org | https://pl.ewi.tudelft.nl/