I think you are correct that we should do this differently. In fact, there is a comment along those lines in the code:
https://github.com/apache/groovy/blob/master/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java#L601 You wouldn't want the check too early so that AST transforms could make changes, e.g.: @MakeVoidReturnSelf // some fluent api AST transform class Foo { void setBar() {...} void setBaz() {...} } new Foo().setBar('x').setBaz('y') Moving this to the Verifier would allow a more traditional error message to occur during compilation. Of course there is an argument (like you mentioned) that we should go the other way and not report the error which would allow scenarios like this: class Foo { void baz() { return 42 } } Foo.metaClass.baz = { throw new RuntimeException('Boom!')} try { new Foo().baz() println 'ERROR: Should not get here!' } catch(RuntimeException ex) { println ex.message } So instead of the weird "runtime" error during parsing, we could throw an exception at runtime if the return statement is ever reached. Cheers, Paul. On Sat, Aug 25, 2018 at 7:20 AM ocs@ocs <o...@ocs.cz> wrote: > Ladies and gentlemen, > > I've just bumped into the problem shown below. > > Whilst it is arguable whether the situation should be valid or not > (myself, I strongly believe it should be valid and compiled without a > glitch), even if invalid, it should not cause an uncaught exception, but > simply report a syntax error, I believe? > > All the best, > OC > > === > 941 */tmp>* >e.groovy > class foo { > void bar() { } > void bax() { return bar() } > } > 942 */tmp>* groovy e > WARNING: Using incubator modules: jdk.incubator.httpclient > org.codehaus.groovy.control.MultipleCompilationErrorsException: startup > failed: > General error during class generation: Cannot use return statement with an > expression on a method that returns void > . At [3:16] /private/tmp/e.groovy > > org.codehaus.groovy.syntax.RuntimeParserException: Cannot use return > statement with an expression on a method that returns void > . At [3:16] /private/tmp/e.groovy > at > org.codehaus.groovy.classgen.AsmClassGenerator.throwException(AsmClassGenerator.java:718) > at > org.codehaus.groovy.classgen.asm.StatementWriter.writeReturn(StatementWriter.java:602) > at > org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.writeReturn(OptimizingStatementWriter.java:368) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitReturnStatement(AsmClassGenerator.java:686) > at > org.codehaus.groovy.ast.stmt.ReturnStatement.visit(ReturnStatement.java:49) > at > org.codehaus.groovy.classgen.asm.StatementWriter.writeBlockStatement(StatementWriter.java:93) > at > org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.writeBlockStatement(OptimizingStatementWriter.java:210) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitBlockStatement(AsmClassGenerator.java:636) > at > org.codehaus.groovy.ast.stmt.BlockStatement.visit(BlockStatement.java:71) > at > org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitClassCodeContainer(ClassCodeVisitorSupport.java:110) > at > org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitConstructorOrMethod(ClassCodeVisitorSupport.java:121) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitStdMethod(AsmClassGenerator.java:496) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitConstructorOrMethod(AsmClassGenerator.java:432) > at > org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitMethod(ClassCodeVisitorSupport.java:132) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitMethod(AsmClassGenerator.java:577) > at org.codehaus.groovy.ast.ClassNode.visitContents(ClassNode.java:1140) > at > org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitClass(ClassCodeVisitorSupport.java:54) > at > org.codehaus.groovy.classgen.AsmClassGenerator.visitClass(AsmClassGenerator.java:270) > at > org.codehaus.groovy.control.CompilationUnit$18.call(CompilationUnit.java:850) > at > org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:1087) > at > org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:631) > at > org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:609) > at > org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:586) > at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:354) > at groovy.lang.GroovyClassLoader.access$300(GroovyClassLoader.java:87) > at groovy.lang.GroovyClassLoader$5.provide(GroovyClassLoader.java:323) > at groovy.lang.GroovyClassLoader$5.provide(GroovyClassLoader.java:320) > at > org.codehaus.groovy.runtime.memoize.StampedCommonCache.compute(StampedCommonCache.java:162) > at > org.codehaus.groovy.runtime.memoize.StampedCommonCache.getAndPut(StampedCommonCache.java:153) > at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:318) > at groovy.lang.GroovyShell.parseClass(GroovyShell.java:547) > at groovy.lang.GroovyShell.run(GroovyShell.java:376) > at groovy.lang.GroovyShell.run(GroovyShell.java:366) > at groovy.ui.GroovyMain.processOnce(GroovyMain.java:589) > at groovy.ui.GroovyMain.run(GroovyMain.java:332) > at groovy.ui.GroovyMain.access$1400(GroovyMain.java:69) > at groovy.ui.GroovyMain$GroovyCommand.process(GroovyMain.java:291) > at groovy.ui.GroovyMain.processArgs(GroovyMain.java:134) > at groovy.ui.GroovyMain.main(GroovyMain.java:116) > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native > Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114) > at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136) > > 1 error > > 943 */tmp>* groovy -version > WARNING: Using incubator modules: jdk.incubator.httpclient > Groovy Version: 3.0.0-alpha-3 JVM: 10.0.1 Vendor: "Oracle Corporation" OS: > Mac OS X > 944 */tmp>* > === >
