From commits-return-6524-archive-asf-public=cust-asf.ponee.io@groovy.apache.org Tue May 15 15:59:09 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 39A88180634 for ; Tue, 15 May 2018 15:59:08 +0200 (CEST) Received: (qmail 6244 invoked by uid 500); 15 May 2018 13:59:07 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 6235 invoked by uid 99); 15 May 2018 13:59:07 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 May 2018 13:59:07 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0E2B3E184D; Tue, 15 May 2018 13:59:07 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sunlan@apache.org To: commits@groovy.apache.org Message-Id: <6d03f1d9104a438884bd2ce125a4c86a@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: groovy git commit: GROOVY-8355: Instanceof inference does not work on field assigning(closes #706) Date: Tue, 15 May 2018 13:59:07 +0000 (UTC) Repository: groovy Updated Branches: refs/heads/master 1f0c0ac6e -> 5df88c8f0 GROOVY-8355: Instanceof inference does not work on field assigning(closes #706) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/5df88c8f Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/5df88c8f Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/5df88c8f Branch: refs/heads/master Commit: 5df88c8f05242e828038652b0f6f762fe9b84840 Parents: 1f0c0ac Author: sunlan Authored: Tue May 15 21:59:00 2018 +0800 Committer: sunlan Committed: Tue May 15 21:59:00 2018 +0800 ---------------------------------------------------------------------- .../asm/sc/StaticTypesCallSiteWriter.java | 27 ++++- .../stc/StaticTypeCheckingVisitor.java | 93 +++++++++++++++ src/spec/test/typing/TypeCheckingTest.groovy | 2 + src/test/groovy/bugs/Groovy8355Bug.groovy | 112 +++++++++++++++++++ 4 files changed, 232 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java index b717c42..61e239c 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java @@ -670,8 +670,15 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes rType = receiver.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE); if (receiver instanceof VariableExpression && rType == null) { // TODO: can STCV be made smarter to avoid this check? - VariableExpression ve = (VariableExpression) ((VariableExpression)receiver).getAccessedVariable(); - rType = ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE); + Variable accessedVariable = ((VariableExpression)receiver).getAccessedVariable(); + VariableExpression ve; + if (accessedVariable instanceof FieldNode) { + FieldNode fieldNode = (FieldNode) accessedVariable; + rType = getInferredTypeOfField(fieldNode); + } else { + ve = (VariableExpression) accessedVariable; + rType = ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE); + } } if (rType!=null && trySubscript(receiver, message, arguments, rType, aType, safe)) { return; @@ -684,6 +691,22 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes "this error and file a bug report at https://issues.apache.org/jira/browse/GROOVY"); } + private ClassNode getInferredTypeOfField(FieldNode fieldNode) { + ClassNode rType = null; + + Map nodeMetaData = fieldNode.getNodeMetaData(controller.getMethodNode()); + if (null != nodeMetaData) { + rType = (ClassNode) nodeMetaData.get(StaticTypesMarker.INFERRED_TYPE); +// if (null == rType) { +// nodeMetaData = fieldNode.getNodeMetaData(controller.getClassNode()); +// if (null != nodeMetaData) { +// rType = (ClassNode) nodeMetaData.get(StaticTypesMarker.INFERRED_TYPE); +// } +// } + } + return rType; + } + private boolean trySubscript(final Expression receiver, final String message, final Expression arguments, ClassNode rType, final ClassNode aType, boolean safe) { if (getWrapper(rType).isDerivedFrom(Number_TYPE) && getWrapper(aType).isDerivedFrom(Number_TYPE)) { http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 0ab1ac5..490d924 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -595,6 +595,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } else if (accessedVariable instanceof FieldNode) { FieldNode fieldNode = (FieldNode) accessedVariable; + FieldVariableExpression fieldVariableExpression = new FieldVariableExpression(fieldNode); ClassNode parameterizedType = GenericsUtils.findParameterizedType(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode()); if (null != parameterizedType) { @@ -602,6 +603,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { ClassNode actualType = findActualTypeByPlaceholderName(originalType.getUnresolvedName(), GenericsUtils.extractPlaceholders(parameterizedType)); if (null != actualType) { + fieldVariableExpression.storeInferredType(actualType); storeType(vexp, actualType); return; } @@ -845,6 +847,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { // if right expression is a ClosureExpression, store parameter type information if (leftExpression instanceof VariableExpression) { + VariableExpression variableExpression = (VariableExpression) leftExpression; + + final Variable accessedVariable = variableExpression.getAccessedVariable(); + if (accessedVariable instanceof FieldNode) { + new FieldVariableExpression((FieldNode) accessedVariable).storeInferredType(resultType); + } + if (rightExpression instanceof ClosureExpression) { Parameter[] parameters = ((ClosureExpression) rightExpression).getParameters(); leftExpression.putNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS, parameters); @@ -1751,6 +1760,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (init instanceof ConstructorCallExpression) { inferDiamondType((ConstructorCallExpression) init, node.getOriginType()); } + +// new FieldVariableExpression(node).storeInferredType(getType(init)); } } finally { currentField = null; @@ -4519,6 +4530,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (cn != null) { return cn; } + + if (exp instanceof FieldNode) { + ClassNode inferredType = new FieldVariableExpression((FieldNode) exp).getInferredType(); + + if (null != inferredType) { + return inferredType; + } + } + if (exp instanceof ClassExpression) { ClassNode node = CLASS_Type.getPlainNodeReference(); node.setGenericsTypes(new GenericsType[]{ @@ -5457,4 +5477,77 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } + private class FieldVariableExpression extends VariableExpression { + private final FieldNode fieldNode; + + FieldVariableExpression(FieldNode fieldNode) { + super(fieldNode); + this.fieldNode = fieldNode; + + ClassNode inferred = getInferredType(); + if (inferred == null) { + storeInferredType(fieldNode.getOriginType()); + } + } + + @Override + public ListHashMap getMetaDataMap() { + AnnotatedNode node = getKey(); + + return getMetaDataMap(node); + } + + private ListHashMap getMetaDataMap(AnnotatedNode key) { + if (null == key) { + return new ListHashMap(); // all valid cases should have a methodNode as key. return a empty map is just to avoid NPE, the map will be abandoned finally + } + + ListHashMap metaDataMap = fieldNode.getNodeMetaData(key); + + if (null == metaDataMap) { + metaDataMap = new ListHashMap(); + fieldNode.putNodeMetaData(key, metaDataMap); + } + + return metaDataMap; + } + + @Override + public int hashCode() { + return fieldNode.hashCode(); + } + + @Override + public boolean equals(Object other) { + return fieldNode.equals(other); + } + + @SuppressWarnings("unchecked") + private ClassNode getInferredType() { + Map enclosingMetaData = getMetaDataMap(); + + ClassNode inferredType = (ClassNode) enclosingMetaData.get(StaticTypesMarker.INFERRED_TYPE); + + return inferredType; +// return null == inferredType ? (ClassNode) getMetaDataMap(getDefaultKey()).get(StaticTypesMarker.INFERRED_TYPE) : inferredType; + } + + @SuppressWarnings("unchecked") + private void storeInferredType(ClassNode inferredType) { + Map enclosingMetaData = getMetaDataMap(); + + enclosingMetaData.put(StaticTypesMarker.INFERRED_TYPE, inferredType); + } + + private AnnotatedNode getKey() { + AnnotatedNode node = typeCheckingContext.getEnclosingMethod(); + + return node; +// return null == node ? getDefaultKey() : node; + } + +// private ClassNode getDefaultKey() { +// return typeCheckingContext.getEnclosingClassNode(); +// } + } } http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/spec/test/typing/TypeCheckingTest.groovy ---------------------------------------------------------------------- diff --git a/src/spec/test/typing/TypeCheckingTest.groovy b/src/spec/test/typing/TypeCheckingTest.groovy index fc87413..6574c37 100644 --- a/src/spec/test/typing/TypeCheckingTest.groovy +++ b/src/spec/test/typing/TypeCheckingTest.groovy @@ -518,6 +518,7 @@ class TypeCheckingTest extends StaticTypeCheckingTestCase { } + /* void testTypeInferenceFieldVsLocalVariable() { shouldFailWithMessages ''' // tag::typeinference_field_vs_local_variable[] @@ -544,6 +545,7 @@ class TypeCheckingTest extends StaticTypeCheckingTestCase { SomeClass ''', 'Cannot find matching method java.lang.Object#toUpperCase()' } + */ void testLeastUpperBound() { assertScript '''import org.codehaus.groovy.ast.ClassHelper http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/test/groovy/bugs/Groovy8355Bug.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/bugs/Groovy8355Bug.groovy b/src/test/groovy/bugs/Groovy8355Bug.groovy new file mode 100644 index 0000000..b512249 --- /dev/null +++ b/src/test/groovy/bugs/Groovy8355Bug.groovy @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + + + + + +package groovy.bugs + +class Groovy8355Bug extends GroovyTestCase { + void testGroovy8355() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Foo { + Object str = new Object() + def bar() { + str = "str" + str.toUpperCase() + } + } + + assert "STR" == new Foo().bar() + ''' + } + + void test2() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Foo { + Object str = new Object() + def bar() { + str = "str" + str.toUpperCase() + } + + def bar2() { + str = 1 + str + 2 + } + } + + Foo foo = new Foo() + assert "STR" == foo.bar() + assert 3 == foo.bar2() + ''' + } + + void testTypeInferenceFieldVsLocalVariable() { + assertScript ''' + // tag::typeinference_field_vs_local_variable[] + class SomeClass { + def someUntypedField // <1> + String someTypedField // <2> + + void someMethod() { + someUntypedField = '123' // <3> + someUntypedField = someUntypedField.toUpperCase() // compile-time error // <4> + } + + void someSafeMethod() { + someTypedField = '123' // <5> + someTypedField = someTypedField.toUpperCase() // <6> + } + + void someMethodUsingLocalVariable() { + def localVariable = '123' // <7> + someUntypedField = localVariable.toUpperCase() // <8> + } + } + // end::typeinference_field_vs_local_variable[] + SomeClass + ''' + } + + /* + void test3() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Foo { + Object str = "str" + def bar() { + str.toUpperCase() + } + } + + assert "STR" == new Foo().bar() + ''' + } + */ +}