RFR: 8295024: Cyclic constructor error is non-deterministic and inconsistent

[Vicente Romero]

On Wed, 12 Oct 2022 14:22:40 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

> This patch addresses two related issues that cause javac's reporting of recursive constructor errors to be non-deterministic.
> 
> The first issue is caused by the use of a `HashMap` when finding cycles in the graph of constructor invocations. This causes non-determinism in which constructor is identified as the "culprit" for the purposes of error reporting. The obvious fix here is to use a `LinkedHashMap` instead.
> 
> The second problem is caused by how the error's source code location is found via `TreeInfo.diagnosticPositionFor()`. That method searches the AST for a node matching the constructor's `Symbol`, but both the constructor declaration and all of its invocations will match that `Symbol`, and so whichever of those happens first in the source code wins.
> 
> The fix adds an optional filter parameter to `TreeInfo.diagnosticPositionFor()`, and then uses it to match only the constructor invocation. Reporting the invocation instead of the declaration is preferred because it provides more specific information about exactly how the recursion occurs. Put another way, when identifying a cycle in a graph, reporting an edge is more helpful than reporting a node.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java line 744:

> 742:         final Predicate<? super JCTree> filter;
> 743: 
> 744:         DeclScanner(final Symbol sym, Predicate<? super JCTree> filter) {

same here, better to keep the original declaration and add an overloaded one with the new argument, the old method can invoke the new one

-------------

PR: https://git.openjdk.org/jdk/pull/10679