Skip to content
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

syntax role modifiers #1835

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from
Draft

syntax role modifiers #1835

wants to merge 53 commits into from

Conversation

jurgenvinju
Copy link
Member

@jurgenvinju jurgenvinju commented Jul 7, 2023

This PR implements syntax role modifiers.

SyntaxModifiers.pdf

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Attention: Patch coverage is 28.35821% with 432 lines in your changes are missing coverage. Please review.

Project coverage is 49%. Comparing base (ab243ce) to head (d6ed722).

Files Patch % Lines
src/org/rascalmpl/types/ModifySyntaxRole.java 23% 208 Missing and 27 partials ⚠️
src/org/rascalmpl/ast/SyntaxRoleModifier.java 20% 90 Missing and 1 partial ⚠️
src/org/rascalmpl/library/Type.java 35% 24 Missing ⚠️
src/org/rascalmpl/ast/Type.java 34% 20 Missing and 1 partial ⚠️
src/org/rascalmpl/types/RascalTypeFactory.java 43% 12 Missing and 1 partial ⚠️
...org/rascalmpl/values/parsetrees/SymbolFactory.java 35% 10 Missing and 1 partial ⚠️
src/org/rascalmpl/ast/ShellCommand.java 0% 7 Missing ⚠️
src/org/rascalmpl/ast/NullASTVisitor.java 0% 6 Missing ⚠️
src/org/rascalmpl/types/RascalType.java 0% 6 Missing ⚠️
src/org/rascalmpl/types/TypeReifier.java 0% 4 Missing ⚠️
... and 9 more
Additional details and impacted files
@@           Coverage Diff            @@
##              main   #1835    +/-   ##
========================================
- Coverage       49%     49%    -1%     
- Complexity    6226    6266    +40     
========================================
  Files          662     665     +3     
  Lines        59419   60050   +631     
  Branches      8613    8726   +113     
========================================
+ Hits         29236   29474   +238     
- Misses       27996   28357   +361     
- Partials      2187    2219    +32     

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

@DavyLandman
Copy link
Member

I'll start reviewing this soon. or are you still writing it @jurgenvinju ?

@DavyLandman
Copy link
Member

Impressive, if you ignore the generated code, it's actually quite compact.


test bool useOfDifferentModifierTypesWithinASingleScope() {
data[A] anExample = a();
syntax[A] anotherExample = [syntax[A]] "a";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it also legal to write parse(#syntax[A], "a") ? and how does this relate to start[A]

Copy link
Member Author

@jurgenvinju jurgenvinju Dec 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent question!

syntax[X] is equivalent to X in any context. So #X produces the same as #syntax[X] as long as X is not ambiguous.

If X is ambiguous then it could happen that #X produces an ADT definition instead of a syntax definition.

For X you can substitute other non-terminal forms; like syntax[start[X]] and syntax[A*] but that is implemented on a different branch. In the current branch you can type start[syntax[X]] to get start[X] while syntax[start[X]] is undefined. Also syntax[A*] is undefined here. With -ea asserts enabled, during the tests such an application will fail. In a deployed interpreter, without -ea, the unsupported combinations will be idempotent. In other words, syntax[A*] will simply reduce to A*, and it will behave as it as always behaved.

On the other branch, after we have this one working, and merged into it, we can use syntax[A*] to introduce layout symbols in all the right places, and also to pattern match on lexical lists, mutually exclusively with context-free lists: i.e. int size(syntax[&T*] l) and int size(lexical[&T*] l) would be two different overloads to handle the different way of counting elements in either kind of list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah interresting. I would have expected syntax[start[A]] to be valid indeed. good that it's incoming as well.

@jurgenvinju
Copy link
Member Author

I'll start reviewing this soon. or are you still writing it @jurgenvinju ?

The util::Explode code is actually more of a integration test; the rest should be almost done. Because of bootstrapping issues, where the standard library needs to be type-checking correctly, before we can release the new code, I think I have to park that use of the new feature until after we have extended the type-checker.

IOW: I'll merge this without util::Explode for bootstrapping purposes. The next step is extending and releasing rascal-core with this feature if we think we are done here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants