Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8295024

Cyclic constructor error is non-deterministic and inconsistent

    XMLWordPrintable

Details

    • b20
    • generic
    • generic

    Description

      ADDITIONAL SYSTEM INFORMATION :
      openjdk version "18.0.2.1" 2022-08-18
      OpenJDK Runtime Environment Homebrew (build 18.0.2.1+0)
      OpenJDK 64-Bit Server VM Homebrew (build 18.0.2.1+0, mixed mode, sharing)



      A DESCRIPTION OF THE PROBLEM :
      The "recursive constructor invocation" compiler error message is generated when there exist any constructors which (indirectly) invoke themselves.

      However, in the case of mutual recursion between two or more constructors, the source code location associated with the error message is generated non-deterministically in two different ways:
      1. The choice of which constructor
      2. The choice of constructor definition vs. constructor invocation

      For example, consider this source file:

          public class Cyclic {
              public Cyclic(int x) {
                  this((float)x);
              }
              public Cyclic(float x) {
                  this((int)x);
              }
          }

      There are two possible error messages that javac will generate:

      Error #1:

      Cyclic.java:2: error: recursive constructor invocation
          public Cyclic(int x) {
                 ^

      Error #2:

      Cyclic.java:3: error: recursive constructor invocation
              this((float)x);
              ^

      Note that in error #1, the Cyclic(int) constructor is the "culprit" and its declaration is identified, whereas in error #2 the Cyclic(float) constructor is the "culprit" but its invocation is highlighted.

      Why is this a problem? Unnecessary non-determinism is, in general, always a problem to avoid if possible. For example, people may have automated scripts that run and expect certain output in certain situations, etc.

      To give a more concrete example that is closer to home, consider that it's currently not even possible to add a "golden output" negative unit test to OpenJDK for this compiler error - because the output is non-deterministic!

      Another problem is that highlighting the constructor declaration (as in the case of Error #1) is less useful than highlighting the point of constructor invocation, because it's a less precise pinpointing of where the problem occurs.

      Looking at the code, it's clear the two sources of inconsistency have two different causes:

      1. The choice of which constructor

      This is caused by the method Check.checkCyclicConstructors() using a HashMap. The keys to the map are Symbol objects, and Symbol does not override hashCode(), so the map is using System.identityHashCode() values which are non-deterministic.

      Changing the HashMap to a LinkedHashMap is the easy fix here.

      2. The choice of constructor definition vs. constructor invocation

      The problem here is that the method TreeInfo.diagnosticPositionFor() is used to identify the error location by searching the AST for a match to the Symbol of the (first) constructor involved in the recursion. However, the Symbol will match the AST at both the constructor's definition and at any point of its invocation. Whichever happens to occur first in the source code is the one that's found and reported.

      To fix this we could e.g. add a variation on TreeInfo.diagnosticPositionFor() that accepts a Predicate<JCTree> and then use it to filter out the JCMethodDecl's so the declarations would be skipped and therefore the invocations would always be found and reported.


      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Compile and run "Source code for an executable test case".

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      The same error repeated 10 times, e.g.:

      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error

      ACTUAL -
      Some random combination of three different possible errors, e.g.:

      /Cyclic.java:2: error: recursive constructor invocation
              public Cyclic(int x) {
                     ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:2: error: recursive constructor invocation
              public Cyclic(int x) {
                     ^
      1 error
      /Cyclic.java:6: error: recursive constructor invocation
                  this((long)x);
                  ^
      1 error
      /Cyclic.java:6: error: recursive constructor invocation
                  this((long)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:2: error: recursive constructor invocation
              public Cyclic(int x) {
                     ^
      1 error
      /Cyclic.java:6: error: recursive constructor invocation
                  this((long)x);
                  ^
      1 error
      /Cyclic.java:3: error: recursive constructor invocation
                  this((float)x);
                  ^
      1 error
      /Cyclic.java:2: error: recursive constructor invocation
              public Cyclic(int x) {
                     ^
      1 error


      ---------- BEGIN SOURCE ----------
          import java.net.*;
          import java.util.*;
          import javax.tools.*;
          public class CyclicErrorsNonDeterministic {
          
              static final String SOURCE = """
                      public class Cyclic {
                          public Cyclic(int x) {
                              this((float)x);
                          }
                          public Cyclic(float x) {
                              this((long)x);
                          }
                          public Cyclic(long x) {
                              this((double)x);
                          }
                          public Cyclic(double x) {
                              this((int)x);
                          }
                      }
                  """;
              static final SimpleJavaFileObject FILE = new SimpleJavaFileObject(
                URI.create("string:///Cyclic.java"), JavaFileObject.Kind.SOURCE) {
                  @Override
                  public String getCharContent(boolean ignoreEncodingErrors) {
                      return SOURCE;
                  }
              };
          
              public static void main(String[] args) throws Exception {
                  JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
                  for (int i = 0; i < 10; i++)
                      compiler.getTask(null, null, null, null, null, Collections.singleton(FILE)).call();
              }
          }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      None known.

      FREQUENCY : always


      Attachments

        Issue Links

          Activity

            People

              acobbs Archie Cobbs
              webbuggrp Webbug Group
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: