search

pboyer / antlr4

ANTLR (ANother Tool for Language Recognition) is a powerful parser generator for reading, processing, executing, or translating structured text or binary files.
http://antlr.org
Other
26 stars 1 forks source link

Remaining work #40

Closed willfaught closed 8 years ago

willfaught commented 8 years ago

@pboyer @wjkohnen

(Ignore the diff for now, it's just a placeholder.)

Guys, we're almost there! Let's finish this thing!

CLA

We all still need to sign the CLA (contributors.txt). Let's do it now so the upstream PR isn't blocked on the whole group when it's ready.

Testing

We're down to 9 failures and 1 error. We're so close! :)

I've been rooting out several errors in the tests themselves, which mainly are caused by Go being different than the other target languages in various ways. This is all low-hanging fruit.

Some tests fail because there's no output at all, either <[]> or just null. No idea yet what's going on there, but it seems like it should be easy to debug.

Some tests fail because they expect something like:

<(1 2)
[1
2]>

but get:

<(1 2)
[]>

which is a little more intuitive to debug (e.g. something isn't printing for each integer, possibly a rule action).

There's one that sets LLAmbigSomethingSomething and I have no idea why that's not working.

@pboyer, your knowledge of why some parts of the runtime don't match the other implementations (like tree/trees.go, it seems?) is invaluable here. I could compare the Go runtime to the Java one line-by-line, but it's too tedious. There are some runtime TODOs that still need to be addressed, which seems easy, and it isn't clear whether these are related to the test failures.

I haven't done an entire test run for all languages in a while, so I actually don't know whether the test fixes I've made have affected the other languages, although any problems there should be easy to fix. And I think Travis is testing all languages anyway?

Polish

I haven't done a full polish pass through the runtime or generated code, as that seems lower priority than getting tests passing and acceptable for upstream PR, so I'm waiting on this until everything passes.

Aside from general code style consistency, there's also some design changes that need to happen to be idiomatic, like renaming GetFoo() funcs to Foo(), hiding implementations of interfaces that don't need to be exposed, etc.

I'm also not sure whether BaseFoo is idiomatic; I could see how it might be DefaultFoo or something, but I haven't looked into it yet to verify.

It might also make sense to make token types something other than int so they're distinct values and not easily used incorrectly with other ints.

It would be good to add more interface implementation assertions via blank var decls.

pboyer commented 8 years ago

@willfaught I'm already on the CLA on the antlr/antlr4.

It looks like we also need to merge the latest changes from that repo.

pboyer commented 8 years ago

Here are all of the current test error/failures:

testReturnValueAndActionsList1_2(org.antlr.v4.test.runtime.go.TestLeftRecursion)  Time elapsed: 2.364 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<...pr (expr a) , (expr [c) >> (expr x])) <EOF>)
> but was:<...pr (expr a) , (expr [(expr c) >> (expr x)])) <EOF>)
>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestLeftRecursion.testReturnValueAndActionsList1_2(TestLeftRecursion.java:2082)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testReturnValueAndActionsList1_4(org.antlr.v4.test.runtime.go.TestLeftRecursion)  Time elapsed: 2.195 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<...* (expr b)) , (expr [c) , (expr (expr x) * (expr y)) >> (expr r)]) <EOF>)
> but was:<...* (expr b)) , (expr [(expr c) , (expr (expr (expr x) * (expr y)) >> (expr r))) <EOF>]) <EOF>)
>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestLeftRecursion.testReturnValueAndActionsList1_4(TestLeftRecursion.java:2134)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testBasic(org.antlr.v4.test.runtime.go.TestListeners)  Time elapsed: 2.195 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<(a 1 2)
[1
2
]> but was:<(a 1 2)
[]>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestListeners.testBasic(TestListeners.java:54)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testAmbiguityNoLoop(org.antlr.v4.test.runtime.go.TestFullContextParsing)  Time elapsed: 1.921 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<[alt 1
]> but was:<[]>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestFullContextParsing.testAmbiguityNoLoop(TestFullContextParsing.java:53)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testLoopsSimulateTailRecursion(org.antlr.v4.test.runtime.go.TestFullContextParsing)  Time elapsed: 2.325 sec  <<< FAILURE!
java.lang.AssertionError: expected:<line 1:3 reportAttemptingFullContext d=3 (expr_primary), input='a(i)'
line 1:7 reportAmbiguity d=3 (expr_primary): ambigAlts={2, 3}, input='a(i)<-x'
> but was:<null>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotEquals(Assert.java:743)
    at org.junit.Assert.assertEquals(Assert.java:118)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestFullContextParsing.testLoopsSimulateTailRecursion(TestFullContextParsing.java:412)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testInvalidATNStateRemoval(org.antlr.v4.test.runtime.go.TestParserErrors)  Time elapsed: 60.004 sec  <<< ERROR!
java.lang.Exception: test timed out after 60000 milliseconds
    at java.lang.Object.wait(Native Method)
    at java.lang.Object.wait(Object.java:502)
    at java.lang.UNIXProcess.waitFor(UNIXProcess.java:181)
    at org.antlr.v4.test.runtime.go.BaseTest.execModule(BaseTest.java:463)
    at org.antlr.v4.test.runtime.go.BaseTest.execRecognizer(BaseTest.java:447)
    at org.antlr.v4.test.runtime.go.BaseTest.execParser(BaseTest.java:394)
    at org.antlr.v4.test.runtime.go.TestParserErrors.testInvalidATNStateRemoval(TestParserErrors.java:152)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testPositionAdjustingLexer(org.antlr.v4.test.runtime.go.TestLexerExec)  Time elapsed: 0.897 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<...ns',<6>,1:0]
[@1,7:1[2='tokens',<4>,2:0]
[@2,14:14='{',<3>,2:7]
[@3,16:23='notLabel',<6>,3:0]
[@4,25:30='label1',<5>,4:0]
[@5,32:32='=',<1>,4:7]
[@6,34:39='label2',<5>,5:0]
[@7,41:42='+=',<2>,5:7]
[@8,44:51='notLabel',<6>,6:0]
[@9],53:52='<EOF>',<-1>,...> but was:<...ns',<6>,1:0]
[@1,7:1[4='tokens {',<4>,2:0]
[@2,16:23='notLabel',<6>,3:0]
[@3,25:32='label1 =',<5>,4:0]
[@4,34:42='label2 +=',<5>,5:0]
[@5,44:51='notLabel',<6>,6:0]
[@6],53:52='<EOF>',<-1>,...>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestLexerExec.testPositionAdjustingLexer(TestLexerExec.java:4759)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)

testZeroLengthToken(org.antlr.v4.test.runtime.go.TestLexerExec)  Time elapsed: 0.902 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<[@0,0:[4=''xxx'',<1>,1:0]
[@1],5:4='<EOF>',<-1>,1:...> but was:<[@0,0:[0=''',<2>,1:0]
[@1,1:1='x',<3>,1:1]
[@2,4:4=''',<2>,1:4]
[@3],5:4='<EOF>',<-1>,1:...>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.antlr.v4.test.runtime.go.TestLexerExec.testZeroLengthToken(TestLexerExec.java:4955)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:74)
pboyer commented 8 years ago

Just to reiterate. There are 8 tests that need to be fixed.

willfaught commented 8 years ago

@parrt Seemed willing to help debug test failures, FWIW.

pboyer commented 8 years ago

@willfaught I just gave you push rights, BTW. It's about time, methinks. :)

https://github.com/pboyer/antlr4/invitations

pboyer commented 8 years ago

I'll mark you as reviewer on my future PR's.

pboyer commented 8 years ago

@willfaught Enter Github projects: https://github.com/pboyer/antlr4/projects/1

I'm closing this.

willfaught commented 8 years ago

Cool, thanks!