Paul,

thanks for elucidation!

Incidentally, as for the runtime — in my personal opinion the point is in that 
void is void is void, i.e., whilst

void foo() { return 42 }

should be an error (I'd rather vote for compile-time, though it of course is 
arguable whether not to move it to runtime),

void bar() { return (void)42 }
void baz() { return bar() }

should, in my personal opinion, be a valid an quite proper code both run- and 
compile-time, for returning a void from a void function makes perfect sense.

Of course, it is not too important; it's a rare construction which normally 
does not happen :)

Thanks and all the best,
OC


> On 25 Aug 2018, at 5:15 AM, Paul King <pa...@asert.com.au> wrote:
> 
> 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
>  
> <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 <mailto: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> 
> ===

Reply via email to