commit 83cdc55e364cb63c451ad5f3ddbdcdb3b8cbe296
author Jinseong Jeon <jsjeon@google.com> Wed Sep 04 10:55:37 2019 -0700
committer Jinseong Jeon <jsjeon@google.com> Wed Sep 04 18:20:00 2019 +0000
tree 8457d3cc4c2c3078f58d09e1dc1b859eec8171f4
parent bc0ffdabdc6572b6550e809812652085d3e2fa07

Invert check in the throwing condition of field instruction for readability.

Also update comments regarding lookup failure in D8.

Bug: 118602538
Change-Id: Ia0a0334726efefac46060091dbd470cb73d550d0

src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java

diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
index c9545e1..e041d5a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -72,9 +72,9 @@
       if (appView.enableWholeProgramOptimizations()) {
         return AbstractError.specific(appView.dexItemFactory().noSuchFieldErrorType);
       } else {
-        // In D8, resolution failure does not necessarily mean the absence of the field. It could be
-        // ICCE or IAE if the current field instruction is referring to incompatible or invisible
-        // field in a super type, respectively.
+        // In D8, the field lookup can only consult the context definition. Nothing can be concluded
+        // from a lookup failure. For example, it could be ICCE or IAE if the current field access
+        // is referring to incompatible or invisible field in a super type, respectively.
         return AbstractError.top();
       }
     }
@@ -103,23 +103,24 @@
     }
     // For D8, reaching here means the field is in the same context, hence the class is guaranteed
     // to be initialized already.
-    if (appView.enableWholeProgramOptimizations()) {
-      boolean mayTriggerClassInitialization = isStaticGet() || isStaticPut();
-      if (mayTriggerClassInitialization) {
-        // Only check for <clinit> side effects if there is no -assumenosideeffects rule.
-        if (appView.appInfo().hasLiveness()) {
-          AppInfoWithLiveness appInfoWithLiveness = appView.appInfo().withLiveness();
-          if (appInfoWithLiveness.noSideEffects.containsKey(resolvedField.field)) {
-            return AbstractError.bottom();
-          }
+    if (!appView.enableWholeProgramOptimizations()) {
+      return AbstractError.bottom();
+    }
+    boolean mayTriggerClassInitialization = isStaticGet() || isStaticPut();
+    if (mayTriggerClassInitialization) {
+      // Only check for <clinit> side effects if there is no -assumenosideeffects rule.
+      if (appView.appInfo().hasLiveness()) {
+        AppInfoWithLiveness appInfoWithLiveness = appView.appInfo().withLiveness();
+        if (appInfoWithLiveness.noSideEffects.containsKey(resolvedField.field)) {
+          return AbstractError.bottom();
         }
-        // May trigger <clinit> that may have side effects.
-        if (field.holder.classInitializationMayHaveSideEffects(
-            appView,
-            // Types that are a super type of `context` are guaranteed to be initialized already.
-            type -> appView.isSubtype(context, type).isTrue())) {
-          return AbstractError.top();
-        }
+      }
+      // May trigger <clinit> that may have side effects.
+      if (field.holder.classInitializationMayHaveSideEffects(
+          appView,
+          // Types that are a super type of `context` are guaranteed to be initialized already.
+          type -> appView.isSubtype(context, type).isTrue())) {
+        return AbstractError.top();
       }
     }
     return AbstractError.bottom();