rewrote subtype semantics of start to simulate the semantics of the interpreter. Fixes #2496 #2499

jurgenvinju · 2025-10-27T11:11:20Z

Fixes Tree of start[A] declared as A #2496 by redefining alub, aglb and asubtype such that start symbols are exclusively subtypes of themselves and Tree and node.
adds a syntaxRole field to AType::\start to make it more compatible with aadt and avoid case distinctions later in the compiler.
every time a start modifier is detected on a syntax rule for X, the collector:
- declares three things:
  1. an implicit start type "start[X]" as AType::start(X, contextFreeSyntax())
  2. a start production rule with a top field prod(X, [label(X, "top")]. _),
  3. the top field within that production to start[X].top with type X.
- To identify each of these three implicit definititions independently, the location of the start keyword is used in three ways. This avoids the multiplekey problem for the mapping between facts/types and their declaration locations.
  1. the loc of start as a whole, for the generated start type.
  2. the loc of the first s in start only, for the generated rule.
  3. the loc of the first t in start only: for the generated field.
removes all downstream special cases for the top field and start symbol handling, which are not needed anymore due to the above. everything is automatic now due to the right TModel.
some new tests test the subtype semantics between start[A] and A and the existence and type of the top field of a start[A] non-terminal (which is A)

…nterpreter. Fixes #2496

codecov · 2025-10-27T11:15:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47%. Comparing base (5046bef) to head (37715c1).

Additional details and impacted files

@@           Coverage Diff           @@
##              main   #2499   +/-   ##
=======================================
  Coverage       47%     47%           
- Complexity    6705    6711    +6     
=======================================
  Files          791     791           
  Lines        65239   65239           
  Branches      9769    9769           
=======================================
+ Hits         30916   30933   +17     
+ Misses       31909   31896   -13     
+ Partials      2414    2410    -4

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

toinehartman · 2025-10-28T08:31:04Z

@jurgenvinju Added the tests as requested in #2496 (and an additional one), but two of them still fail. I had a look, but did not see an obvious solution.

… different (incomparable) type from A

…e of an aadt without exceptions

Finish adding the syntax role to start types

jurgenvinju

Checked for funny side effects. Still not working!

jurgenvinju · 2025-11-16T15:15:50Z

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

                               !isTerminalSym(sym),
                               tsym := s.getType(sym),
                               isNonTerminalAType(tsym),
                               stp := getSyntaxType(removeChainRule(tsym), s)


Check later: what is remove chain rule?

@PaulKlint I don't remember chain rule unwrapping as a feature for production fields. Do we have this in the interpreter?

I copied this from the behavior of the interpreter

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

jurgenvinju · 2025-11-17T15:11:08Z

This PR doesn't get through any type-checker test with a function or constructor call in it.

My internal notes, for later:

The changes made only pertain to the start symbol/atype so that is a surprise.

The intersection between start symbol and function calls happens probably when filtering the overloaded candidates. Each candidate's parameters are matched using acomparable which uses asubtype, which was changed in this PR.

All candidates are always filtered currently. So asubtype returns false for at least one of the function parameters. Possibly for all of them.

This points to a new bug introduced into asubtype. If not, then alub may also be a candidate. It was also changed in this PR and it can influence parameter types and then influence asubtype. Finally aglb but that seems unlikely since it is not used very often and even the simplest test fails.

If asubtype is broken then I should definitely check aglb and alub too. Also it would be interesting to see if these three functions can be randomly tested like their counterparts in Type...

toinehartman · 2025-11-18T16:16:10Z

Everything succeeded at e6fbb9e. What motivated the changes made since then?

jurgenvinju · 2025-11-19T10:44:42Z

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectExpression.rsc

                 }
                 reportMissingNonTerminalCases(current, overloads, validOverloads, actuals, s);
                 next_cons:
                 for(ovl: <key,idRole, tp> <- overloads){


I'm worried about reusing a tp variable here since it occurs more often in the code.

… start symbol of ATYpe

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

jurgenvinju · 2025-11-20T15:07:08Z

@toinehartman it didn't work for me at that commit. That's one reason I kept searching for better solutions.

While searching I also saw that start's top field implementation was scattered, and the top field test was exactly the failing test I was working on. So now the top field handling is all done in the collect for the Start modifier.

In the meantime my failing test was due to an old open typepal project. 🙈

…backend, and removed all special-case handling of the start symbol from the checker code

…s the module interface, from now on

…mmon/general case

jurgenvinju · 2025-11-26T10:43:06Z

src/org/rascalmpl/compiler/lang/rascalcore/check/ATypeUtils.rsc

 bool isADTAType(aparameter(_,AType tvb)) = isADTAType(tvb);
 bool isADTAType(aadt(_,_,_)) = true;
 bool isADTAType(areified(_)) = true;
-bool isADTAType(\start(AType s)) = isADTAType(s);


I have my doubts here. What does it mean to be a ADTAType? Should it be obly aadt's? Then this should be false for start. If it should be all left hand side of syntax rules, then it should be true.

Perhaps @PaulKlint can answer this?

(Note: this line is changed, not removed; look at the full diff)

…l case

PaulKlint · 2025-11-27T14:26:43Z

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

+    c.define("<aStartSym>", nonterminalId(), current, st);
+    
+    startProd = defType(aprod(prod(aStartSym, [nonterminalType[alabel="top"]])));
+    sPos = current@\loc.top(current@\loc.offset, 1);


This looks a bit tricky. Replace by original loc + modifier.

PaulKlint · 2025-11-27T14:30:19Z

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectStatement.rsc


 private AType computeFieldAssignableType(Statement current, AType receiverType, Tree field, str operator, AType rhs, loc scope, Solver s){
    //println("computeFieldAssignableType: <current>");
    fieldName = unescape("<field>");


So where is "top" declared these days?

rascal/src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

Lines 137 to 139 in 37715c1

fieldDef = defType(nonterminalType[alabel="top"]);

tPos = current@\loc.top(current@\loc.offset + 1, 1);

c.define("top", fieldId(), tPos, fieldDef);

PaulKlint · 2025-11-27T14:33:15Z

Overall looks good.

All these "notes to self" are noise and complicate this review.

Minor concern: at the moment we do not support a start for lexical symbols. The original code allowed this while the new code always hard wires that it is context free.

I cannot assess whether this new code will break the code generator.

toinehartman

Looks good! I like how this removes all kinds of special casing, e.g. around the top field.
I have a couple of minor comments.

toinehartman · 2025-11-28T14:10:08Z

src/org/rascalmpl/compiler/lang/rascalcore/check/ADTandGrammar.rsc


        for(AType adtType <- allStarts){
-            syntaxDefinitions[\start(adtType)] = achoice(\start(adtType), { prod(\start(adtType), [definedLayout, adtType[alabel="top"], definedLayout]) });
+            // TODO JV: there are more places where layout is added to the start non-terminal...


Does this TODO indicate more work to in this PR before merging?

toinehartman · 2025-11-28T14:12:31Z

src/org/rascalmpl/compiler/lang/rascalcore/check/ATypeUtils.rsc

 bool isADTAType(aparameter(_,AType tvb)) = isADTAType(tvb);
 bool isADTAType(aadt(_,_,_)) = true;
 bool isADTAType(areified(_)) = true;
-bool isADTAType(\start(AType s)) = isADTAType(s);


Perhaps @PaulKlint can answer this?

toinehartman · 2025-11-28T14:12:57Z

src/org/rascalmpl/compiler/lang/rascalcore/check/ATypeUtils.rsc

 bool isADTAType(aparameter(_,AType tvb)) = isADTAType(tvb);
 bool isADTAType(aadt(_,_,_)) = true;
 bool isADTAType(areified(_)) = true;
-bool isADTAType(\start(AType s)) = isADTAType(s);


(Note: this line is changed, not removed; look at the full diff)

toinehartman · 2025-11-28T14:15:04Z

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectStatement.rsc


 private AType computeFieldAssignableType(Statement current, AType receiverType, Tree field, str operator, AType rhs, loc scope, Solver s){
    //println("computeFieldAssignableType: <current>");
    fieldName = unescape("<field>");


rascal/src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc

Lines 137 to 139 in 37715c1

fieldDef = defType(nonterminalType[alabel="top"]);

tPos = current@\loc.top(current@\loc.offset + 1, 1);

c.define("top", fieldId(), tPos, fieldDef);

toinehartman · 2025-11-28T14:18:17Z

src/org/rascalmpl/compiler/lang/rascalcore/check/RascalConfig.rsc

-        msg = error("Constructor `<ovl1.atype.alabel>` is overloaded, maybe<qualifyHint><argHint>",
+        msg = error("Constructor `<ovl1.atype.alabel>` is overloaded, maybe <qualifyHint><argHint>",
                         current@\loc);
+        iprintln(overloads);


Shouldn't this be removed?

toinehartman · 2025-11-28T14:19:02Z

src/org/rascalmpl/compiler/lang/rascalcore/check/ComputeType.rsc

            } else
                continue;
        }
+        // println("require: <i>, <formalTypesU[i]> -- <aiU>");


Does the compiler have a verbose/debug flag that we can hide this under?

rewrote subtype semantics of start to simulate the semantics of the i…

5178961

…nterpreter. Fixes #2496

jurgenvinju mentioned this pull request Oct 27, 2025

Tree of start[A] declared as A #2496

Open

jurgenvinju mentioned this pull request Oct 27, 2025

can not write generic functions for start non-terminals #2500

Open

3 tasks

Add start symbol tests.

9e00a6f

jurgenvinju and others added 16 commits October 28, 2025 13:15

start[A] AType wrapper should not be removed anymore, because it is a…

044d6ac

… different (incomparable) type from A

fixed Issue465c regression

1c599c3

make sure we can not redefine start

b5921bd

debug commit

9d41d5b

Merge branch 'main' into fix/issue-2496

e6fbb9e

gave start AType also a SyntaxRole so it can more easily play the rol…

3b1903c

…e of an aadt without exceptions

added syntax role to start non-terminal

0c4656c

Merge branch 'main' into fix/issue-2496

1fa2caa

Update Symbols.rsc

e0a3392

Update ADTandGrammar.rsc

fcd6f99

Update ADTandGrammar.rsc

f066014

Update ADTandGrammar.rsc

095c307

Update AType.rsc

90b3abe

commented debug prints

fe39645

forgot syntax role on start with glb

7c9ebbc

Update ATypeUtils.rsc

aaa5d48

Finish adding the syntax role to start types

jurgenvinju commented Nov 16, 2025

View reviewed changes

Merge branch 'main' into fix/issue-2496

6116554

jurgenvinju commented Nov 19, 2025

View reviewed changes

jurgenvinju added 3 commits November 19, 2025 11:50

removed debug prints and added some more syntaxrole positions for the…

1c553b0

… start symbol of ATYpe

removed comments and debug print

5561dbf

finishing touches: declare the top field

c5c3877

jurgenvinju added 3 commits November 20, 2025 13:51

fixed MultipleKeyIssues

0d36ca3

getting the scopes right

806d47b

fixes

3b274ed

jurgenvinju commented Nov 20, 2025

View reviewed changes

src/org/rascalmpl/compiler/lang/rascalcore/check/CollectSyntaxDeclaration.rsc Outdated Show resolved Hide resolved

jurgenvinju requested a review from PaulKlint November 20, 2025 13:42

Merge branch 'main' into fix/issue-2496

45f2af8

jurgenvinju marked this pull request as draft November 26, 2025 09:34

jurgenvinju added 4 commits November 26, 2025 10:59

Merge branch 'main' into fix/issue-2496

19591d8

added underscore for syntaxRole positions of the start symbol in the …

fa222aa

…backend, and removed all special-case handling of the start symbol from the checker code

adding or removing a start modifier _does_ change the grammar and thu…

ec81ff7

…s the module interface, from now on

fixed typo and changed order of visit cases for faster path to the co…

2415607

…mmon/general case

jurgenvinju commented Nov 26, 2025

View reviewed changes

removed unneeded comments, debug code and the last superfluous specia…

bcd36ca

…l case

jurgenvinju self-assigned this Nov 26, 2025

jurgenvinju marked this pull request as ready for review November 26, 2025 10:58

PaulKlint reviewed Nov 27, 2025

View reviewed changes

Merge branch 'main' into fix/issue-2496

37715c1

toinehartman reviewed Nov 28, 2025

View reviewed changes

	fieldDef = defType(nonterminalType[alabel="top"]);
	tPos = current@\loc.top(current@\loc.offset + 1, 1);
	c.define("top", fieldId(), tPos, fieldDef);

rewrote subtype semantics of start to simulate the semantics of the interpreter. Fixes #2496 #2499

Are you sure you want to change the base?

rewrote subtype semantics of start to simulate the semantics of the interpreter. Fixes #2496 #2499

Conversation

jurgenvinju commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

toinehartman commented Oct 28, 2025

Uh oh!

jurgenvinju left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jurgenvinju commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

toinehartman commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jurgenvinju commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaulKlint commented Nov 27, 2025

Uh oh!

toinehartman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jurgenvinju commented Oct 27, 2025 •

edited

Loading

codecov bot commented Oct 27, 2025 •

edited

Loading

jurgenvinju commented Nov 17, 2025 •

edited

Loading

jurgenvinju commented Nov 20, 2025 •

edited

Loading