You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2017/03/08 05:05:08 UTC
[2/2] groovy git commit: GROOVY-8107: Binary incompatibility problems
between compiled code in Groovy 2.4.7 vs 2.4.9 (closes PR#508)
GROOVY-8107: Binary incompatibility problems between compiled code in Groovy 2.4.7 vs 2.4.9 (closes PR#508)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/83037298
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/83037298
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/83037298
Branch: refs/heads/master
Commit: 83037298daa86a4fcc7613f98fcebae6102ce90d
Parents: d55329c
Author: paulk <pa...@asert.com.au>
Authored: Mon Mar 6 07:52:26 2017 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Wed Mar 8 15:03:37 2017 +1000
----------------------------------------------------------------------
.../transform/trait/TraitASTTransformation.java | 87 ++++++++++++--------
.../groovy/transform/trait/TraitComposer.java | 81 +++++++++---------
2 files changed, 94 insertions(+), 74 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/83037298/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index 99d1c0d..da71e9e 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -409,45 +409,42 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
returnS(initialExpression)
);
helper.addMethod(fieldInitializer);
+ }
+ VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
+ ExpressionStatement initCode = new ExpressionStatement(initialExpression);
+ processBody(thisObject, initCode, trait, helper, fieldHelper, knownFields);
+ BlockStatement code = (BlockStatement) selectedMethod.getCode();
+ MethodCallExpression mce;
+ if (field.isStatic()) {
+ mce = new MethodCallExpression(
+ new ClassExpression(INVOKERHELPER_CLASSNODE),
+ "invokeStaticMethod",
+ new ArgumentListExpression(
+ thisObject,
+ new ConstantExpression(Traits.helperSetterName(field)),
+ initCode.getExpression()
+ )
+ );
} else {
- VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
- ExpressionStatement initCode = new ExpressionStatement(initialExpression);
- processBody(thisObject, initCode, trait, helper, fieldHelper, knownFields);
- BlockStatement code = (BlockStatement) selectedMethod.getCode();
- MethodCallExpression mce;
- if (field.isStatic()) {
- mce = new MethodCallExpression(
- new ClassExpression(INVOKERHELPER_CLASSNODE),
- "invokeStaticMethod",
- new ArgumentListExpression(
- thisObject,
- new ConstantExpression(Traits.helperSetterName(field)),
- initCode.getExpression()
- )
- );
- } else {
- mce = new MethodCallExpression(
- new CastExpression(createReceiverType(field.isStatic(), fieldHelper), thisObject),
- Traits.helperSetterName(field),
- new CastExpression(field.getOriginType(),initCode.getExpression())
- );
- }
- mce.setImplicitThis(false);
- mce.setSourcePosition(initialExpression);
- code.addStatement(new ExpressionStatement(mce));
+ mce = new MethodCallExpression(
+ new CastExpression(createReceiverType(field.isStatic(), fieldHelper), thisObject),
+ Traits.helperSetterName(field),
+ new CastExpression(field.getOriginType(),initCode.getExpression())
+ );
}
+ mce.setImplicitThis(false);
+ mce.setSourcePosition(initialExpression);
+ code.addStatement(new ExpressionStatement(mce));
}
- // define setter/getter helper methods
- if (!Modifier.isFinal(field.getModifiers())) {
- fieldHelper.addMethod(
- Traits.helperSetterName(field),
- ACC_PUBLIC | ACC_ABSTRACT,
- field.getOriginType(),
- new Parameter[]{new Parameter(field.getOriginType(), "val")},
- ClassNode.EMPTY_ARRAY,
- null
- );
- }
+ // define setter/getter helper methods (setter added even for final fields for legacy compatibility)
+ fieldHelper.addMethod(
+ Traits.helperSetterName(field),
+ ACC_PUBLIC | ACC_ABSTRACT,
+ field.getOriginType(),
+ new Parameter[]{new Parameter(field.getOriginType(), "val")},
+ ClassNode.EMPTY_ARRAY,
+ null
+ );
fieldHelper.addMethod(
Traits.helperGetterName(field),
ACC_PUBLIC | ACC_ABSTRACT,
@@ -474,6 +471,24 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
GeneralUtils.copyAnnotatedNodeAnnotations(field, copied, notCopied);
dummyField.addAnnotations(copied);
fieldHelper.addField(dummyField);
+
+ // retain legacy field (will be given lower precedence than above)
+ dummyFieldName = (field.isStatic() ? Traits.STATIC_FIELD_PREFIX : Traits.FIELD_PREFIX) +
+ (field.isPublic()? Traits.PUBLIC_FIELD_PREFIX : Traits.PRIVATE_FIELD_PREFIX)+
+ Traits.remappedFieldName(field.getOwner(), field.getName());
+ dummyField = new FieldNode(
+ dummyFieldName,
+ ACC_STATIC | ACC_PUBLIC | ACC_FINAL | ACC_SYNTHETIC,
+ field.getOriginType(),
+ fieldHelper,
+ null
+ );
+ // copy annotations from field to legacy dummy field
+ copied = new LinkedList<AnnotationNode>();
+ notCopied = new LinkedList<AnnotationNode>();
+ GeneralUtils.copyAnnotatedNodeAnnotations(field, copied, notCopied);
+ dummyField.addAnnotations(copied);
+ fieldHelper.addField(dummyField);
}
private MethodNode processMethod(ClassNode traitClass, ClassNode traitHelperClass, MethodNode methodNode, ClassNode fieldHelper, Collection<String> knownFields) {
http://git-wip-us.apache.org/repos/asf/groovy/blob/83037298/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
index 704162d..1025e61 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -217,34 +217,38 @@ public abstract class TraitComposer {
int fieldMods = 0;
int isStatic = 0;
boolean publicField = true;
- FieldNode helperField = fieldHelperClassNode.getField(Traits.FIELD_PREFIX + Traits.PUBLIC_FIELD_PREFIX + fieldName);
- if (helperField==null) {
- publicField = false;
- helperField = fieldHelperClassNode.getField(Traits.FIELD_PREFIX + Traits.PRIVATE_FIELD_PREFIX + fieldName);
+ FieldNode helperField = null;
+ fieldMods = 0;
+ isStatic = 0;
+
+ // look first for field with encoded modifier information
+ for (Integer mod : Traits.FIELD_PREFIXES) {
+ helperField = fieldHelperClassNode.getField(String.format("$0x%04x", mod) + fieldName);
+ if (helperField != null) {
+ if ((mod & Opcodes.ACC_STATIC) != 0) isStatic = Opcodes.ACC_STATIC;
+ fieldMods = fieldMods | mod;
+ break;
+ }
}
- if (helperField==null) {
- publicField = true;
- // try to find a static one
- helperField = fieldHelperClassNode.getField(Traits.STATIC_FIELD_PREFIX+Traits.PUBLIC_FIELD_PREFIX+fieldName);
+
+ if (helperField == null) {
+ // look for possible legacy fields (trait compiled pre 2.4.8)
+ helperField = fieldHelperClassNode.getField(Traits.FIELD_PREFIX + Traits.PUBLIC_FIELD_PREFIX + fieldName);
if (helperField==null) {
publicField = false;
- helperField = fieldHelperClassNode.getField(Traits.STATIC_FIELD_PREFIX+Traits.PRIVATE_FIELD_PREFIX +fieldName);
+ helperField = fieldHelperClassNode.getField(Traits.FIELD_PREFIX + Traits.PRIVATE_FIELD_PREFIX + fieldName);
}
- fieldMods = fieldMods | Opcodes.ACC_STATIC;
- isStatic = Opcodes.ACC_STATIC;
- }
- if (helperField == null) {
- fieldMods = 0;
- isStatic = 0;
- for (Integer mod : Traits.FIELD_PREFIXES) {
- helperField = fieldHelperClassNode.getField(String.format("$0x%04x", mod) + fieldName);
- if (helperField != null) {
- if ((mod & Opcodes.ACC_STATIC) != 0) isStatic = Opcodes.ACC_STATIC;
- fieldMods = fieldMods | mod;
- break;
+ if (helperField==null) {
+ publicField = true;
+ // try to find a static one
+ helperField = fieldHelperClassNode.getField(Traits.STATIC_FIELD_PREFIX+Traits.PUBLIC_FIELD_PREFIX+fieldName);
+ if (helperField==null) {
+ publicField = false;
+ helperField = fieldHelperClassNode.getField(Traits.STATIC_FIELD_PREFIX+Traits.PRIVATE_FIELD_PREFIX +fieldName);
}
+ fieldMods = fieldMods | Opcodes.ACC_STATIC;
+ isStatic = Opcodes.ACC_STATIC;
}
- } else {
fieldMods = fieldMods | (publicField?Opcodes.ACC_PUBLIC:Opcodes.ACC_PRIVATE);
}
if (getter) {
@@ -279,29 +283,30 @@ public abstract class TraitComposer {
}
Expression fieldExpr = varX(cNode.getField(fieldName));
+ boolean finalSetter = !getter && (fieldMods & Opcodes.ACC_FINAL) != 0;
Statement body =
getter ? returnS(fieldExpr) :
- stmt(
+ (finalSetter ? null : stmt(
new BinaryExpression(
fieldExpr,
Token.newSymbol(Types.EQUAL, 0, 0),
varX(newParams[0])
)
- );
- if (getter || (fieldMods & Opcodes.ACC_FINAL) == 0) {
- MethodNode impl = new MethodNode(
- methodNode.getName(),
- Opcodes.ACC_PUBLIC | isStatic,
- returnType,
- newParams,
- ClassNode.EMPTY_ARRAY,
- body
- );
- AnnotationNode an = new AnnotationNode(COMPILESTATIC_CLASSNODE);
- impl.addAnnotation(an);
- cNode.addTransform(StaticCompileTransformation.class, an);
- cNode.addMethod(impl);
- }
+ ));
+ // add getter/setter even though setter not strictly needed for final fields
+ // but add empty body for setter for legacy compatibility
+ MethodNode impl = new MethodNode(
+ methodNode.getName(),
+ Opcodes.ACC_PUBLIC | isStatic,
+ returnType,
+ newParams,
+ ClassNode.EMPTY_ARRAY,
+ body
+ );
+ AnnotationNode an = new AnnotationNode(COMPILESTATIC_CLASSNODE);
+ impl.addAnnotation(an);
+ cNode.addTransform(StaticCompileTransformation.class, an);
+ cNode.addMethod(impl);
}
}
}