Correctly add a nil_chk on the outer expression of a ClassInstanceCre…

…ation. Change on 2016/09/14 by kstanger <kstanger@google.com> ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=133200600
google · Sep 16, 2016 · ed37efd · ed37efd
1 parent 384bece
commit ed37efd
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 13 deletions.
diff --git a/translator/src/main/java/com/google/devtools/j2objc/pipeline/TranslationProcessor.java b/translator/src/main/java/com/google/devtools/j2objc/pipeline/TranslationProcessor.java
@@ -196,6 +196,15 @@ public static void applyMutations(
     new InitializationNormalizer().run(unit);
     ticker.tick("InitializationNormalizer");
 
+    // Adds nil_chk calls wherever an expression is dereferenced.
+    // After: InnerClassExtractor - Cannot handle local classes.
+    // After: InitializationNormalizer
+    // Before: OuterReferenceFixer - Must resolve before outer references are substituted.
+    // Before: LabelRewriter - Control flow analysis requires original Java
+    //   labels.
+    new NilCheckResolver().run(unit);
+    ticker.tick("NilCheckResolver");
+
     // Fix references to outer scope and captured variables.
     new OuterReferenceFixer(outerResolver).run(unit);
     ticker.tick("OuterReferenceFixer");
@@ -206,14 +215,6 @@ public static void applyMutations(
       ticker.tick("UnsequencedExpressionRewriter");
     }
 
-    // Adds nil_chk calls wherever an expression is dereferenced.
-    // After: InnerClassExtractor - Cannot handle local classes.
-    // After: InitializationNormalizer
-    // Before: LabelRewriter - Control flow analysis requires original Java
-    //   labels.
-    new NilCheckResolver().run(unit);
-    ticker.tick("NilCheckResolver");
-
     // Rewrites labeled break and continue statements.
     new LabelRewriter().run(unit);
     ticker.tick("LabelRewriter");

diff --git a/translator/src/main/java/com/google/devtools/j2objc/translate/NilCheckResolver.java b/translator/src/main/java/com/google/devtools/j2objc/translate/NilCheckResolver.java
@@ -375,10 +375,6 @@ private boolean isBoxingMethod(IMethodBinding method) {
   private boolean needsNilCheck(Expression e) {
     IVariableBinding sym = TreeUtil.getVariableBinding(e);
     if (sym != null) {
-      // Outer class references should always be non-nil.
-      if (sym.getName().startsWith("this$") || sym.getName().equals("outer$")) {
-        return false;
-      }
       return BindingUtil.isVolatile(sym) || !isSafeVar(sym);
     }
     IMethodBinding method = TreeUtil.getMethodBinding(e);
@@ -457,9 +453,20 @@ public void endVisit(SuperMethodInvocation node) {
   }
 
   @Override
-  public void endVisit(ClassInstanceCreation node) {
+  public boolean visit(ClassInstanceCreation node) {
+    Expression outerTarget = node.getExpression();
+    if (outerTarget != null) {
+      outerTarget.accept(this);
+      addNilCheck(outerTarget);
+    }
+    for (Expression arg : node.getArguments()) {
+      arg.accept(this);
+    }
+    // Don't need to visit AnonymousClassDeclaration child because it's removed by
+    // AnonymousClassConverter.
     removeNonFinalFields();
     handleThrows();
+    return false;
   }
 
   @Override

diff --git a/translator/src/test/java/com/google/devtools/j2objc/translate/NilCheckResolverTest.java b/translator/src/test/java/com/google/devtools/j2objc/translate/NilCheckResolverTest.java
@@ -333,4 +333,12 @@ public void testLabeledBlock() throws IOException {
         "break_test_label: ;",
         "[nil_chk(o) description];");
   }
+
+  public void testOuterExpressionOfClassInstanceCreation() throws IOException {
+    String translation = translateSourceFile(
+        "class Test { class Inner {} public Inner test(Test t) { return t.new Inner(); } }",
+        "Test", "Test.m");
+    // "t" needs a nil_chk.
+    assertTranslation(translation, "return create_Test_Inner_initWithTest_(nil_chk(t));");
+  }
 }