paulk-asert commented on a change in pull request #1361:
URL: https://github.com/apache/groovy/pull/1361#discussion_r487374598
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
Removing interface `Opcodes` is a binary breaking change. I agree it is
ugly to leak that implementation class out through our API and acknowledge that
style of usage dates back to before Java had static imports. Never-the-less, I
think we should make such a change as a separate issue and target just Groovy
4. I'll create such an issue and merge this one without that change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
See GROOVY-9736 for proposed separate `Opcodes` change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
Removing interface `Opcodes` is a binary breaking change. I agree it is
ugly to leak that implementation class out through our API and acknowledge that
style of usage dates back to before Java had static imports. Never-the-less, I
think we should make such a change as a separate issue and target just Groovy
4. I'll create such an issue and merge this one without that change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
See GROOVY-9736 for proposed separate `Opcodes` change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
Removing interface `Opcodes` is a binary breaking change. I agree it is
ugly to leak that implementation class out through our API and acknowledge that
style of usage dates back to before Java had static imports. Never-the-less, I
think we should make such a change as a separate issue and target just Groovy
4. I'll create such an issue and merge this one without that change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
See GROOVY-9736 for proposed separate `Opcodes` change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
Removing interface `Opcodes` is a binary breaking change. I agree it is
ugly to leak that implementation class out through our API and acknowledge that
style of usage dates back to before Java had static imports. Never-the-less, I
think we should make such a change as a separate issue and target just Groovy
4. I'll create such an issue and merge this one without that change.
##########
File path:
src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
##########
@@ -31,25 +30,47 @@
import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-public class CompareToNullExpression extends BinaryExpression implements
Opcodes {
+import static org.objectweb.asm.Opcodes.GOTO;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IFNONNULL;
+import static org.objectweb.asm.Opcodes.IFNULL;
+
+public class CompareToNullExpression extends BinaryExpression {
Review comment:
See GROOVY-9736 for proposed separate `Opcodes` change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
