[GitHub] groovy pull request #416: GROOVY-7926 - Don't use a CastExpression if the re...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/416#discussion_r78228482 --- Diff: src/main/org/codehaus/groovy/transform/trait/TraitComposer.java --- @@ -293,7 +293,8 @@ private static void createForwarderMethod( ClassNode[] exceptionNodes = correctToGenericsSpecRecurse(genericsSpec, copyExceptions(helperMethod.getExceptions())); ClassNode fixedReturnType = correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType()); -Expression forwardExpression = genericsSpec.isEmpty()?mce:new CastExpression(fixedReturnType,mce); +boolean noCastRequired = genericsSpec.isEmpty() || fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName()); --- End diff -- Thanks for the explanation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (GROOVY-7926) Method that returns void in a Trait with generics produces questionable byte code
[
https://issues.apache.org/jira/browse/GROOVY-7926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15477920#comment-15477920
]
ASF GitHub Bot commented on GROOVY-7926:
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/416#discussion_r78228482
--- Diff: src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
---
@@ -293,7 +293,8 @@ private static void createForwarderMethod(
ClassNode[] exceptionNodes =
correctToGenericsSpecRecurse(genericsSpec,
copyExceptions(helperMethod.getExceptions()));
ClassNode fixedReturnType =
correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType());
-Expression forwardExpression = genericsSpec.isEmpty()?mce:new
CastExpression(fixedReturnType,mce);
+boolean noCastRequired = genericsSpec.isEmpty() ||
fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName());
--- End diff --
Thanks for the explanation.
> Method that returns void in a Trait with generics produces questionable byte
> code
> -
>
> Key: GROOVY-7926
> URL: https://issues.apache.org/jira/browse/GROOVY-7926
> Project: Groovy
> Issue Type: Bug
>Affects Versions: 2.4.7
>Reporter: Graeme Rocher
>
> Given the following trait:
> {code}
> trait MyTrait {
> void delete() {
> // no-op
> }
> }
> class MyImpl implements MyTrait {
> }
> {code}
> The delete method produces byte code that when decompiled looks like:
> {code}
> public void delete() {
> CallSite[] var1 = $getCallSiteArray();
> void var1 = (void)var1[1].call(Helper.class, this);
> }
> {code}
> As you can see "void" is used as a type of a variable. This results in the
> following exception on the IBM JVM (it works fine in Oracle and OpenJDK):
> {code}
> Caused by: java.lang.NoClassDefFoundError: void
> at it.dedagroup.Test.delete(Test.groovy)
> at org.grails.datastore.gorm.GormEntity$delete$0.call(Unknown Source)
> at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
> {code}
> Since the issue occurs only in IBM JVM it could be argued that this is a
> problem with that JVM, however the byte code produced does seem suspicious to
> me as "void" cannot be used as a type of a local variable. Thoughts?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] groovy pull request #416: GROOVY-7926 - Don't use a CastExpression if the re...
Github user graemerocher commented on a diff in the pull request:
https://github.com/apache/groovy/pull/416#discussion_r78217077
--- Diff: src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
---
@@ -293,7 +293,8 @@ private static void createForwarderMethod(
ClassNode[] exceptionNodes =
correctToGenericsSpecRecurse(genericsSpec,
copyExceptions(helperMethod.getExceptions()));
ClassNode fixedReturnType =
correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType());
-Expression forwardExpression = genericsSpec.isEmpty()?mce:new
CastExpression(fixedReturnType,mce);
+boolean noCastRequired = genericsSpec.isEmpty() ||
fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName());
--- End diff --
Just seemed more efficient. ClassNode.equals does this:
```groovy
public boolean equals(Object o) {
if (redirect!=null) return redirect().equals(o);
if (!(o instanceof ClassNode)) return false;
ClassNode cn = (ClassNode) o;
return (cn.getText().equals(getText()));
}
```
The `getText()` method calls `getName()` anyway
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (GROOVY-7926) Method that returns void in a Trait with generics produces questionable byte code
[
https://issues.apache.org/jira/browse/GROOVY-7926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15477650#comment-15477650
]
ASF GitHub Bot commented on GROOVY-7926:
Github user graemerocher commented on a diff in the pull request:
https://github.com/apache/groovy/pull/416#discussion_r78217077
--- Diff: src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
---
@@ -293,7 +293,8 @@ private static void createForwarderMethod(
ClassNode[] exceptionNodes =
correctToGenericsSpecRecurse(genericsSpec,
copyExceptions(helperMethod.getExceptions()));
ClassNode fixedReturnType =
correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType());
-Expression forwardExpression = genericsSpec.isEmpty()?mce:new
CastExpression(fixedReturnType,mce);
+boolean noCastRequired = genericsSpec.isEmpty() ||
fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName());
--- End diff --
Just seemed more efficient. ClassNode.equals does this:
```groovy
public boolean equals(Object o) {
if (redirect!=null) return redirect().equals(o);
if (!(o instanceof ClassNode)) return false;
ClassNode cn = (ClassNode) o;
return (cn.getText().equals(getText()));
}
```
The `getText()` method calls `getName()` anyway
> Method that returns void in a Trait with generics produces questionable byte
> code
> -
>
> Key: GROOVY-7926
> URL: https://issues.apache.org/jira/browse/GROOVY-7926
> Project: Groovy
> Issue Type: Bug
>Affects Versions: 2.4.7
>Reporter: Graeme Rocher
>
> Given the following trait:
> {code}
> trait MyTrait {
> void delete() {
> // no-op
> }
> }
> class MyImpl implements MyTrait {
> }
> {code}
> The delete method produces byte code that when decompiled looks like:
> {code}
> public void delete() {
> CallSite[] var1 = $getCallSiteArray();
> void var1 = (void)var1[1].call(Helper.class, this);
> }
> {code}
> As you can see "void" is used as a type of a variable. This results in the
> following exception on the IBM JVM (it works fine in Oracle and OpenJDK):
> {code}
> Caused by: java.lang.NoClassDefFoundError: void
> at it.dedagroup.Test.delete(Test.groovy)
> at org.grails.datastore.gorm.GormEntity$delete$0.call(Unknown Source)
> at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
> {code}
> Since the issue occurs only in IBM JVM it could be argued that this is a
> problem with that JVM, however the byte code produced does seem suspicious to
> me as "void" cannot be used as a type of a local variable. Thoughts?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7926) Method that returns void in a Trait with generics produces questionable byte code
[
https://issues.apache.org/jira/browse/GROOVY-7926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15477398#comment-15477398
]
ASF GitHub Bot commented on GROOVY-7926:
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/416#discussion_r78204083
--- Diff: src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
---
@@ -293,7 +293,8 @@ private static void createForwarderMethod(
ClassNode[] exceptionNodes =
correctToGenericsSpecRecurse(genericsSpec,
copyExceptions(helperMethod.getExceptions()));
ClassNode fixedReturnType =
correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType());
-Expression forwardExpression = genericsSpec.isEmpty()?mce:new
CastExpression(fixedReturnType,mce);
+boolean noCastRequired = genericsSpec.isEmpty() ||
fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName());
--- End diff --
Just curious if you saw problems using just
`fixedReturnType.equals(ClassHelper.VOID_TYPE)` that neccessitated comparing by
name? Most places that I see that compare types don't see to use the getName
so am just curious.
> Method that returns void in a Trait with generics produces questionable byte
> code
> -
>
> Key: GROOVY-7926
> URL: https://issues.apache.org/jira/browse/GROOVY-7926
> Project: Groovy
> Issue Type: Bug
>Affects Versions: 2.4.7
>Reporter: Graeme Rocher
>
> Given the following trait:
> {code}
> trait MyTrait {
> void delete() {
> // no-op
> }
> }
> class MyImpl implements MyTrait {
> }
> {code}
> The delete method produces byte code that when decompiled looks like:
> {code}
> public void delete() {
> CallSite[] var1 = $getCallSiteArray();
> void var1 = (void)var1[1].call(Helper.class, this);
> }
> {code}
> As you can see "void" is used as a type of a variable. This results in the
> following exception on the IBM JVM (it works fine in Oracle and OpenJDK):
> {code}
> Caused by: java.lang.NoClassDefFoundError: void
> at it.dedagroup.Test.delete(Test.groovy)
> at org.grails.datastore.gorm.GormEntity$delete$0.call(Unknown Source)
> at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
> {code}
> Since the issue occurs only in IBM JVM it could be argued that this is a
> problem with that JVM, however the byte code produced does seem suspicious to
> me as "void" cannot be used as a type of a local variable. Thoughts?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7926) Method that returns void in a Trait with generics produces questionable byte code
[
https://issues.apache.org/jira/browse/GROOVY-7926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15477391#comment-15477391
]
ASF GitHub Bot commented on GROOVY-7926:
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/416#discussion_r78203740
--- Diff:
src/test/org/codehaus/groovy/transform/traitx/Groovy7926Bug.groovy ---
@@ -0,0 +1,50 @@
+package org.codehaus.groovy.transform.traitx
+
+import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
+import org.codehaus.groovy.classgen.asm.InstructionSequence
+
+/**
+ * Created by graemerocher on 08/09/2016.
+ */
+class Groovy7926Bug extends AbstractBytecodeTestCase {
--- End diff --
Would probably be good to remove the class doc and add the license header.
> Method that returns void in a Trait with generics produces questionable byte
> code
> -
>
> Key: GROOVY-7926
> URL: https://issues.apache.org/jira/browse/GROOVY-7926
> Project: Groovy
> Issue Type: Bug
>Affects Versions: 2.4.7
>Reporter: Graeme Rocher
>
> Given the following trait:
> {code}
> trait MyTrait {
> void delete() {
> // no-op
> }
> }
> class MyImpl implements MyTrait {
> }
> {code}
> The delete method produces byte code that when decompiled looks like:
> {code}
> public void delete() {
> CallSite[] var1 = $getCallSiteArray();
> void var1 = (void)var1[1].call(Helper.class, this);
> }
> {code}
> As you can see "void" is used as a type of a variable. This results in the
> following exception on the IBM JVM (it works fine in Oracle and OpenJDK):
> {code}
> Caused by: java.lang.NoClassDefFoundError: void
> at it.dedagroup.Test.delete(Test.groovy)
> at org.grails.datastore.gorm.GormEntity$delete$0.call(Unknown Source)
> at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
> {code}
> Since the issue occurs only in IBM JVM it could be argued that this is a
> problem with that JVM, however the byte code produced does seem suspicious to
> me as "void" cannot be used as a type of a local variable. Thoughts?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
