RFR: 8295024: Cyclic constructor error is non-deterministic and inconsistent
Vicente Romero
vromero at openjdk.org
Fri Oct 14 13:54:32 UTC 2022
On Thu, 13 Oct 2022 20:31:55 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.
>
> Thanks for the helpful comments. They should be addressed in [b7482f9](https://github.com/openjdk/jdk/pull/10679/commits/b7482f949b95b1cff30b7b75f0e9d9a23773b71a) and the patch has gotten cleaner as a result.
@archiecobbs please try integrating again, I just added the `sponsor` command. Thanks for fixing this!
-------------
PR: https://git.openjdk.org/jdk/pull/10679
More information about the compiler-dev
mailing list