Fixes an issue related to prefix shared alternatives with nesting restrictions

[arnoldlankamp]

Fix for #1915 .

Changes:

• The last node of a production is no longer prefix-shared when nesting restrictions are associated with it, to prevent incorrect behavior. E.g: For the grammar S::= AB | ABC, both A's will be shared, but B will only be shared in case no nesting restrictions are associated with the AB alternative.
  Theoretically the B's could still be shared, but this would involve adding even more special cases to an already fairly complicated piece of code, so I opted not to do this, since any potential gain would be minimal.
• Adds a test case for the grammar provided in the bug report.
• Minor modification of the parser generator.
• Updates previously generated / written parsers (e.g. the rascal parser).

NOTE: This change is not backward compatible with any previously generated parsers. If any exist outside this project, they will need to be regenerated.

[codecov]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (8e0f7b2) 49% compared to head (1eb2fee) 49%.
Report is 12 commits behind head on main.

Files	Patch %	Lines
...almpl/library/lang/rascal/syntax/RascalParser.java	92%	10 Missing ⚠️
...calmpl/parser/gtd/preprocessing/ExpectBuilder.java	75%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##              main   #1919   +/-   ##
=======================================
  Coverage       49%     49%           
- Complexity    6164    6166    +2     
=======================================
  Files          661     661           
  Lines        58702   58701    -1     
  Branches      8547    8548    +1     
=======================================
+ Hits         28962   28965    +3     
  Misses       27550   27550           
+ Partials      2190    2186    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jurgenvinju]

Thanks @arnoldlankamp ! I was "under water" the last weeks with too many distractions to look at your fixes. Will have a look now. The fix sounds logical.

[jurgenvinju]

@arnoldlankamp why is this test not useful anymore?

[arnoldlankamp]

It's still there. This is just a tabs vs spaces formatting diff.

[jurgenvinju]

smart and elegant fix. not binary compatible, so let's watch out for that when releasing the new version. Thanks @arnoldlankamp

[jurgenvinju]

I'm merging this because we have to move on, but we should realize that a careful bootstrap sequence is required if we want to avoid class loading errors of previously generated parsers.

It may be necessary to also include a compatibility mode.

[jurgenvinju]

Propose to include the old constructor as well @deprecated, such that we can bootstrap without too many issues.

[arnoldlankamp]

Adding a constructor with the old signature, which uses an empty dontNest map, would make the change backwards compatible and should make everything work as before.
I considered this myself, but ultimately decided against it, since it (naturally) also retains the bug. Making this a breaking change forces people to regenerate any existing parsers and prevents them from continuing to use a deprecated/buggy version.
But feel free to add one if this is preferred.