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 2018/04/02 23:15:47 UTC
groovy git commit: GROOVY-8523 Static type checking instanceof_not
(closes #680)
Repository: groovy
Updated Branches:
refs/heads/master 1bcaaeaf6 -> 0e737d7b9
GROOVY-8523 Static type checking instanceof_not (closes #680)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/0e737d7b
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/0e737d7b
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/0e737d7b
Branch: refs/heads/master
Commit: 0e737d7b9f9a0e695b179babd37b19fad5e078c1
Parents: 1bcaaea
Author: Nikolay Chugunov <ni...@github.com>
Authored: Sun Apr 1 23:13:23 2018 +0300
Committer: Paul King <pa...@asert.com.au>
Committed: Tue Apr 3 08:50:18 2018 +1000
----------------------------------------------------------------------
.../stc/StaticTypeCheckingVisitor.java | 92 +++++++++++++++
.../transform/stc/TypeCheckingContext.java | 18 +++
src/test/groovy/bugs/Groovy8523Bug.groovy | 111 +++++++++++++++++++
3 files changed, 221 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/0e737d7b/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 4419379..d335b6d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -73,6 +73,8 @@ import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.UnaryMinusExpression;
import org.codehaus.groovy.ast.expr.UnaryPlusExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.ast.expr.NotExpression;
+import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.CaseStatement;
import org.codehaus.groovy.ast.stmt.CatchStatement;
import org.codehaus.groovy.ast.stmt.EmptyStatement;
@@ -96,6 +98,7 @@ import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.syntax.SyntaxException;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.TokenUtil;
+import org.codehaus.groovy.syntax.Types;
import org.codehaus.groovy.transform.StaticTypesTransformation;
import org.codehaus.groovy.transform.trait.Traits;
import org.codehaus.groovy.util.ListHashMap;
@@ -3465,6 +3468,95 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} finally {
popAssignmentTracking(oldTracker);
}
+ BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
+ if (instanceOfExpression == null) {
+ } else {
+ if(typeCheckingContext.enclosingBlocks.size()>0) {
+ visitInstanceofNot(instanceOfExpression);
+ }
+ }
+ }
+
+
+ public void visitInstanceofNot(BinaryExpression be) {
+ final BlockStatement currentBlock = typeCheckingContext.enclosingBlocks.getFirst();
+ assert currentBlock != null;
+ if (typeCheckingContext.blockStatements2Types.containsKey(currentBlock)) {
+ // another instanceOf_not was before, no need store vars
+ } else {
+ // saving type of variables to restoring them after returning from block
+ Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
+ getTypeCheckingContext().pushTemporaryTypeInfo();
+ typeCheckingContext.blockStatements2Types.put(currentBlock, oldTracker);
+ }
+ pushInstanceOfTypeInfo(be.getLeftExpression(), be.getRightExpression());
+ }
+
+
+ @Override
+ public void visitBlockStatement(BlockStatement block) {
+ if (block != null) {
+ typeCheckingContext.enclosingBlocks.addFirst(block);
+ }
+ super.visitBlockStatement(block);
+ if (block != null) {
+ visitClosingBlock(block);
+ }
+ }
+
+ public void visitClosingBlock(BlockStatement block) {
+ BlockStatement peekBlock = typeCheckingContext.enclosingBlocks.removeFirst();
+ boolean found = typeCheckingContext.blockStatements2Types.containsKey(peekBlock);
+ if (found) {
+ Map<VariableExpression, List<ClassNode>> oldTracker = typeCheckingContext.blockStatements2Types.remove(peekBlock);
+ getTypeCheckingContext().popTemporaryTypeInfo();
+ popAssignmentTracking(oldTracker);
+ }
+ }
+
+ /**
+ * Check IfStatement matched pattern :
+ * Object var1;
+ * if (!(var1 instanceOf Runnable)){
+ * return
+ * }
+ * // Here var1 instance of Runnable
+ *
+ * Return expression , which contains instanceOf (without not)
+ * Return null, if not found
+ */
+ public BinaryExpression findInstanceOfNotReturnExpression(IfStatement ifElse) {
+ Statement elseBlock = ifElse.getElseBlock();
+ if (!(elseBlock instanceof EmptyStatement)) {
+ return null;
+ }
+ Expression conditionExpression = ifElse.getBooleanExpression().getExpression();
+ if (!(conditionExpression instanceof NotExpression)) {
+ return null;
+ }
+ NotExpression notExpression = (NotExpression) conditionExpression;
+ Expression expression = notExpression.getExpression();
+ if (!(expression instanceof BinaryExpression)) {
+ return null;
+ }
+ BinaryExpression instanceOfExpression = (BinaryExpression) expression;
+ int op = instanceOfExpression.getOperation().getType();
+ if (op != Types.KEYWORD_INSTANCEOF) {
+ return null;
+ }
+ Statement block = ifElse.getIfBlock();
+ if (!(block instanceof BlockStatement)) {
+ return null;
+ }
+ BlockStatement bs = (BlockStatement) block;
+ if (bs.getStatements().size() == 0) {
+ return null;
+ }
+ Statement last = DefaultGroovyMethods.last(bs.getStatements());
+ if (!(last instanceof ReturnStatement)) {
+ return null;
+ }
+ return instanceOfExpression;
}
@Override
http://git-wip-us.apache.org/repos/asf/groovy/blob/0e737d7b/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java b/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java
index 57fcee0..4d0b097 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java
@@ -27,6 +27,7 @@ import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.StaticMethodCallExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.control.CompilationUnit;
import org.codehaus.groovy.control.ErrorCollector;
import org.codehaus.groovy.control.SourceUnit;
@@ -34,6 +35,7 @@ import org.codehaus.groovy.control.SourceUnit;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
@@ -51,6 +53,8 @@ public class TypeCheckingContext {
protected final LinkedList<ClassNode> enclosingClassNodes = new LinkedList<ClassNode>();
protected final LinkedList<MethodNode> enclosingMethods = new LinkedList<MethodNode>();
protected final LinkedList<Expression> enclosingMethodCalls = new LinkedList<Expression>();
+ protected final LinkedList<BlockStatement> enclosingBlocks = new LinkedList<BlockStatement>();
+
// used for closure return type inference
protected final LinkedList<EnclosingClosure> enclosingClosures = new LinkedList<EnclosingClosure>();
@@ -68,6 +72,20 @@ public class TypeCheckingContext {
* be the LUB of (int, String)
*/
protected Map<VariableExpression, List<ClassNode>> ifElseForWhileAssignmentTracker = null;
+
+
+ /**
+ * This field used for type derivation
+ * Check IfStatement matched pattern :
+ * Object var1;
+ * if (!(var1 instanceOf Runnable)){
+ * return
+ * }
+ * // Here var1 instance of Runnable
+ */
+ protected final IdentityHashMap<BlockStatement, Map<VariableExpression, List<ClassNode>>> blockStatements2Types = new IdentityHashMap<BlockStatement, Map<VariableExpression, List<ClassNode>>>();
+
+
/**
* Stores information which is only valid in the "if" branch of an if-then-else statement. This is used when the if
* condition expression makes use of an instanceof check
http://git-wip-us.apache.org/repos/asf/groovy/blob/0e737d7b/src/test/groovy/bugs/Groovy8523Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8523Bug.groovy b/src/test/groovy/bugs/Groovy8523Bug.groovy
new file mode 100755
index 0000000..ac59854
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8523Bug.groovy
@@ -0,0 +1,111 @@
+/*
+ * 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 Groovy8523Bug extends GroovyTestCase {
+ void testInstanceofNot1() {
+ assertScript '''
+ import groovy.transform.CompileStatic
+ @CompileStatic
+ class Test1 {
+ static int checkRes = 0
+
+ static void f1(Object var1) {
+ if(!(var1 instanceof Runnable)){
+ checkRes = 3
+ return;
+ }
+ f2(var1)
+ }
+
+ static void f2(Runnable var2) {
+ checkRes = 4
+ }
+ }
+
+ Runnable r = {}
+ Test1.f1(r)
+ assert Test1.checkRes == 4;
+ '''
+ }
+
+
+ void testInstanceofNot2() {
+ assertScript '''
+ import groovy.transform.CompileStatic
+ @CompileStatic
+ class Test1 {
+ static int checkRes = 0
+
+ static void f1(Object var1) {
+ if(!(var1 instanceof Runnable)){
+ checkRes = 3
+ return;
+ }
+ if(!(var1 instanceof List)){
+ checkRes = 5
+ return;
+ }
+ f2(var1)
+ }
+
+ static void f2(Runnable var2) {
+ checkRes = 4
+ }
+ }
+
+ Runnable r = {}
+ Test1.f1(r)
+ assert Test1.checkRes == 5;
+ '''
+ }
+
+
+ void testInstanceofNot3() {
+ assertScript '''
+ import groovy.transform.CompileStatic
+ @CompileStatic
+ class Test1 {
+ static int checkRes = 0
+
+ static void f1(Object var1) {
+ if(!(var1 instanceof Runnable)){
+ checkRes = 3
+ return;
+ }
+ if(!(var1 instanceof Thread)){
+ checkRes = 5
+ return;
+ }
+ f2(var1)
+ }
+
+ static void f2(Runnable var2) {
+ checkRes = 4
+ }
+ }
+
+ Runnable r = {}
+ Test1.f1(r)
+ assert Test1.checkRes == 5;
+ '''
+ }
+
+
+}