Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions #3716

kazcw · 2022-09-15T18:28:47Z

Pull Request Description

I believe all parse failures remaining after these changes are because the new parser is intentionally stricter about some things. I'll be reviewing those failures and opening a bug to change the library/tests code.

Implements:

https://www.pivotaltracker.com/story/show/182941610: full type def syntax
https://www.pivotaltracker.com/story/show/182497490: field syntax
https://www.pivotaltracker.com/story/show/182497395: complex operator sections
https://www.pivotaltracker.com/story/show/182497236: template functions
codeRepr without leading whitespace
text literals: interpret escape sequences in lexer
the multiline text-literal left-trim algorithm
type operator-methods
the <= operator is no longer treated as a modifier
https://www.pivotaltracker.com/story/show/183315038: eliminate Unsupported
use ArgumentDefinition for type constructor arguments
more detailed ArgumentDefinition type

Breaking API changes

TypeDef is more specifically typed:

name has been restricted from Tree to Ident
argument values have been restricted from Tree to ArgumentDefinition

ArgumentDefinition now parses the default-value, ascribed-type, and execution-suspension-operator into fields, so that information can no longer be obtained by decomposing the pattern field.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

[ci no changelog needed]

kazcw · 2022-09-20T21:14:41Z

(Not ready for integration: I'm planning further changes to TypeDef tomorrow.)

radeusgd · 2022-09-30T07:27:06Z

distribution/lib/Standard/Table/0.0.0-dev/src/Data/Column.enso

-        data = ['data', self.to_vector.take (First max_data)])
+        data = ['data', self.to_vector.take (First max_data)]


Wow, the fact that we did not notice this is slightly concerning

JaroslavTulach · 2022-09-30T14:20:17Z

engine/runtime/src/main/java/org/enso/compiler/TreeToIr.java

+  }
+
+  private Option<IdentifiedLocation> getIdentifiedLocation(Token ast) {
+    return Option.apply(ast).map(ast_ -> {


There is a lot of functional style rewrites in this file! @kazcw, looks like you can (unlike me) write functionally (even) in Java.

JaroslavTulach · 2022-10-01T02:15:19Z

engine/runtime/src/main/java/org/enso/compiler/TreeToIr.java

+              Tree treeBody = null;
+              Tree.OprApp head = app;
+              while (head != null) {
+                // FIXME: Handle parameterized types here.


Do we have an Enso source example that needs handling?

Added a test case, with @Ignore for now. Before this PR, the test case fails due to incorrect IR (the previous code here flattened all types and their parameters into one list); after this PR, the unhandled case throws an exception from the line under this comment.

JaroslavTulach

I see there is a failure in javac compilation:

INFO ide_ci::program::command: sbtℹ️ [warn] java.lang.AssertionError
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.util.Assert.error(Assert.java:155)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.util.Assert.check(Assert.java:46)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.code.Scope$ScopeImpl.leave(Scope.java:386)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.visitBlock(Attr.java:1474)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1082)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribTree(Attr.java:696)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribStat(Attr.java:770)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.visitMethodDef(Attr.java:1262)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:912)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribTree(Attr.java:696)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribStat(Attr.java:770)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribClassBody(Attr.java:5637)

I've had similar error yesterday and had to rewrite my code to be less fancy. The hardest task is to find the code that is causing the assert, however.

JaroslavTulach · 2022-10-04T03:27:44Z

engine/runtime/src/main/java/org/enso/compiler/TreeToIr.java

+    } else if (ast.getOpen2() != null) {
+      begin = ast.getOpen2().getStartCode();
+    } else if (ast.getSuspension() != null) {
+      begin = ast.getSuspension().getStartCode();


Open, open2, suspension!? That's complicated. Simple ArgumentDefinition.getOpen() with proper behavior would be better.

getOpen returns the outer parenthesis, if present. This logic is finding the textual begin and end of the ArgumentDefinition; which token begins the object depends on what syntax occurred within the argument definition. A pattern is always present, but there are optional parts that can extend to either side of it: A type annotation, a default value, and a ~ suspension operator, along with up to two pairs of parentheses (one around the type annotation and one around the default value). As noted in the comment earlier in the function, I think the proper solution is to refactor the IR so that its DefinitionArgument is not a node, and doesn't need a code span; however, I looked in to doing that, and it would require changes all across the compiler. I don't think it's worth it; this works.

kazcw · 2022-10-04T05:25:50Z

I see there is a failure in javac compilation:

INFO ide_ci::program::command: sbtℹ️ [warn] java.lang.AssertionError
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.util.Assert.error(Assert.java:155)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.util.Assert.check(Assert.java:46)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.code.Scope$ScopeImpl.leave(Scope.java:386)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.visitBlock(Attr.java:1474)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1082)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribTree(Attr.java:696)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribStat(Attr.java:770)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.visitMethodDef(Attr.java:1262)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:912)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribTree(Attr.java:696)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribStat(Attr.java:770)
 INFO ide_ci::program::command: sbtℹ️ [warn] 	at com.sun.tools.javac.comp.Attr.attribClassBody(Attr.java:5637)

I've had similar error yesterday and had to rewrite my code to be less fancy. The hardest task is to find the code that is causing the assert, however.

Wow, I thought that was a transient glitch. We actually have to deal with a flaky compiler? I don't even know where to start debugging this, since it builds fine on my machine.

…ertionError

JaroslavTulach · 2022-10-04T06:12:53Z

Wow, I thought that was a transient glitch. We actually have to deal with a flaky compiler? I don't even know where to start debugging this, since it builds fine on my machine.

Fixed in 5869412 - I could reproduce the problem by doing:

enso$ sbt> runtime/clean
enso$ sbt> buildEngineDistribution

now it seems to be fixed.

Btw. please take a look at my conflict resolution: e1c4b0a - it may not be correct.

JaroslavTulach · 2022-10-04T14:55:50Z

lib/rust/parser/src/macros/resolver.rs

@@ -438,7 +438,6 @@ impl<'s> Resolver<'s> {
            self.replace_current_with_parent_macro(parent_macro);
            Step::MacroStackPop(token.into())
        } else if let Some(segments) = root_macro_map.get(repr) {
-//        } else if let Some(segments) = root_macro_map.get(repr) && token.variant.can_start_macro() {


I wasn't sure which line to keep...

Your choice was correct, thanks for handling that merge. And thanks for fixing my too-fancy-for-the-compiler attempt at downcasting, I think instanceof is probably the more idiomatic implementation there anyway.

JaroslavTulach · 2022-10-04T14:57:10Z

Another lint: https://github.com/enso-org/enso/actions/runs/3182851703/jobs/5189331859 - did I merge improperly?

GitHub
Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions · enso-org/enso@ec06748
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

kazcw · 2022-10-04T15:58:52Z

No, the code here is fine. There was a develop commit with broken formatting, and apparently CI merges current develop before linting (which makes sense, but was totally baffling until I figured out that was the problem).

kazcw added 2 commits September 15, 2022 14:32

refactor

33e1988

type defs

cc41895

kazcw self-assigned this Sep 15, 2022

kazcw added 2 commits September 15, 2022 18:51

test cases for accessor operator

e3ca452

handle operator block edge case

3168fc6

kazcw marked this pull request as ready for review September 15, 2022 20:47

kazcw requested review from wdanilo and JaroslavTulach as code owners September 15, 2022 20:47

kazcw changed the title ~~Parser: Full constructor syntax for type definitions~~ Parser: Full constructor syntax for type definitions; Field syntax Sep 15, 2022

kazcw added 4 commits September 16, 2022 13:47

reject from-as-import

78e97db

complex operator sections

2e5699e

number-parsing edge case

2352dc0

template functions

8ec3805

kazcw changed the title ~~Parser: Full constructor syntax for type definitions; Field syntax~~ Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions Sep 16, 2022

kazcw added 4 commits September 20, 2022 14:44

edge cases

0073ade

trimmed codeRepr

24de8a9

Merge branch 'develop' into wip/kw/parser/typedefs

37d8328

interpret escape sequences in lexer

4dd6bf1

kazcw requested review from MichaelMauderer, mwu-tow and farmaazon as code owners September 20, 2022 19:08

left-trim multiline text sections

8c818ec

kazcw marked this pull request as draft September 20, 2022 21:13

kazcw added 6 commits September 21, 2022 18:31

type operator-methods

391c866

type operator-method type-annotations

8a40833

eliminate Unsupported

c2df5a7

support <= operator

e7751de

use ArgumentDefinition for type constructor arguments

f106b2b

move more details into ArgumentDefinition

951a055

kazcw marked this pull request as draft September 30, 2022 05:57

kazcw marked this pull request as ready for review September 30, 2022 05:57

jdunkerley approved these changes Sep 30, 2022

View reviewed changes

radeusgd reviewed Sep 30, 2022

View reviewed changes

JaroslavTulach reviewed Sep 30, 2022

View reviewed changes

Merge remote-tracking branch 'origin/develop' into parser-typedefs

f8dba6f

JaroslavTulach reviewed Oct 1, 2022

View reviewed changes

kazcw added 2 commits October 3, 2022 16:06

test case for parameterized types

644fa4d

add test for parameterized types in function arguments

db82f51

kazcw marked this pull request as draft October 3, 2022 16:27

kazcw marked this pull request as ready for review October 3, 2022 16:27

JaroslavTulach approved these changes Oct 4, 2022

View reviewed changes

instanceof compiles on Java 19 while switch statements fails with Ass…

5869412

…ertionError

JaroslavTulach force-pushed the wip/kw/parser/typedefs branch from e1c4b0a to 5869412 Compare October 4, 2022 12:09

JaroslavTulach and others added 2 commits October 4, 2022 14:28

Merging with origin/develop branch

bd9904d

lint

ec06748

JaroslavTulach reviewed Oct 4, 2022

View reviewed changes

Merge branch 'develop' into wip/kw/parser/typedefs

5ebacab

kazcw and others added 2 commits October 5, 2022 02:34

add Jaroslav to codeowners of reflect

88b5a32

Merge branch 'develop' into wip/kw/parser/typedefs

a88f12f

wdanilo approved these changes Oct 5, 2022

View reviewed changes

JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Oct 5, 2022

mergify bot merged commit 44a031f into develop Oct 5, 2022

mergify bot deleted the wip/kw/parser/typedefs branch October 5, 2022 04:45

This was referenced Feb 6, 2023

Engine should send notification about node status #4595

Closed

Replace existing AST with Rust based AST #4691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions #3716

Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions #3716

kazcw commented Sep 15, 2022 •

edited

Loading

kazcw commented Sep 20, 2022

radeusgd Sep 30, 2022

JaroslavTulach Sep 30, 2022

JaroslavTulach Oct 1, 2022

kazcw Oct 3, 2022

JaroslavTulach left a comment

JaroslavTulach Oct 4, 2022

kazcw Oct 4, 2022

kazcw commented Oct 4, 2022

JaroslavTulach commented Oct 4, 2022

JaroslavTulach Oct 4, 2022

kazcw Oct 4, 2022

JaroslavTulach commented Oct 4, 2022 •

edited by unfurl-links bot

Loading

kazcw commented Oct 4, 2022

		data = ['data', self.to_vector.take (First max_data)])
		data = ['data', self.to_vector.take (First max_data)]

Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions #3716

Parser: Full constructor syntax for type definitions; Field syntax; Complex operator sections; Template functions; Text improvements; Operator methods; eliminate Unsupported; better ArgumentDefinitions #3716

Conversation

kazcw commented Sep 15, 2022 • edited Loading

Pull Request Description

Breaking API changes

Checklist

kazcw commented Sep 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazcw commented Oct 4, 2022

JaroslavTulach commented Oct 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach commented Oct 4, 2022 • edited by unfurl-links bot Loading

kazcw commented Oct 4, 2022

kazcw commented Sep 15, 2022 •

edited

Loading

JaroslavTulach commented Oct 4, 2022 •

edited by unfurl-links bot

Loading