Document constructor call divergence from RI as deliberate.

Test: n/a (comments-only)
Bug: 20269058
Bug: 183319013
Bug: 183485797
Change-Id: I21a49c74fe3b85bfd9cccbca6ab3a3416388b9bc

2 files changed, 18 insertions, 12 deletions

diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 1a6d7d265b..3118d88d3d 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -2887,15 +2887,6 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g
           break;
         }
 
-        /* must be in same class or in superclass */
-        // const RegType& this_super_klass = this_type.GetSuperClass(&reg_types_);
-        // TODO: re-enable constructor type verification
-        // if (this_super_klass.IsConflict()) {
-          // Unknown super class, fail so we re-check at runtime.
-          // Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "super class unknown for '" << this_type << "'";
-          // break;
-        // }
-
         /* arg must be an uninitialized reference */
         if (!this_type.IsUninitializedTypes()) {
           Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Expected initialization on uninitialized reference "
@@ -2903,6 +2894,16 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g
           break;
         }
 
+        // Note: According to JLS, constructors are never inherited. Therefore the target
+        // constructor should be defined exactly by the `this_type`, or by the direct
+        // superclass in the case of a constructor calling the superclass constructor.
+        // However, ART had this check commented out for a very long time and this has
+        // allowed bytecode optimizers such as R8 to inline constructors, often calling
+        // `j.l.Object.<init>` directly without any intermediate constructor. Since this
+        // optimization allows eliminating constructor methods, this often results in a
+        // significant dex size reduction. Therefore it is undesirable to reinstate this
+        // check and ART deliberately remains permissive here and diverges from the RI.
+
         /*
          * Replace the uninitialized reference with an initialized one. We need to do this for all
          * registers that have the same object instance in them, not just the "this" register.
diff --git a/test/162-method-resolution/src/Main.java b/test/162-method-resolution/src/Main.java
index 4b8ee920b7..08d11f5dd8 100644
--- a/test/162-method-resolution/src/Main.java
+++ b/test/162-method-resolution/src/Main.java
@@ -424,8 +424,13 @@ public class Main {
      *     invoke-direct Test11Derived.<init>(Ljava/lang/String;)V from Test11User in first dex
      *         TODO b/183485797 This should throw a NSME (constructors are never inherited, JLS 8.8)
      *                          but actually calls the superclass constructor.
-     *         expected: Throws NoSuchMethodError
-     *         actual: Successful construction of a Test11Derived instance.
+     *         expected: Successful construction of a Test11Derived instance.
+     * According to JLS, constructors are never inherited, so we should throw NoSuchMethodError and
+     * the RI does exactly that. However, ART has been permissive and allowed calling a superclass
+     * constructor directly for a long time and bytecode optimizers such as R8 are now using this
+     * to significantly reduce the dex file size. It is undesirable to implement strict checks now
+     * due to app compatibility issues and dex file size impact. Therefore ART deliberately
+     * diverges from the RI in this case and accepts the call to the superclass constructor.
      *
      * Files:
      *   src/Test11Base.java          - defines Test11Base with <init>(Ljava/lang/String;)V
@@ -434,7 +439,7 @@ public class Main {
      */
     private static void test11() throws Exception {
         if (usingRI) {
-            // For RI, just print the expected output to hide the divergence for now.
+            // For RI, just print the expected output to hide the deliberate divergence.
             System.out.println("Calling Test11User.test():\n" +
                                "Test11Base.<init>(\"Test\")");
         } else {