RFR: 8257037: No javac warning when calling deprecated constructor with diamond [v2]
Maurizio Cimadamore
mcimadamore at openjdk.java.net
Wed Dec 2 12:17:55 UTC 2020
On Tue, 1 Dec 2020 21:08:15 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> Thank you for the comment and suggestion.
>>
>> I had seen and paid attention to the following code in `Attr.java` when being solved this bug.
>>
>> // (for constructors (but not for constructor references), the error
>> // was given when the constructor was resolved)
>>
>> if (sym.name != names.init || tree.hasTag(REFERENCE)) {
>> chk.checkDeprecated(tree.pos(), env.info.scope.owner, sym);
>> chk.checkSunAPI(tree.pos(), sym);
>> chk.checkProfile(tree.pos(), sym);
>> chk.checkPreview(tree.pos(), sym);
>> }
>>
>> These code confused me. Because the comments `(for constructors, the error was given when the constructor was resolved)` and the conditional expression `sym.name != names.init` were submitted at 2007(submit log: Initial load). And nobody raises suggestion to it over these years though this code snippet had been revised many times.
>>
>> In order not to generate unknown issues, I chose not to revise these code when being solved this bug.
>>
>> @mcimadamore I test your patch locally by using the following command. All the tests passed.
>>
>> make test TEST="jtreg:langtools/tools/javac" CONF=linux-x86_64-server-release
>>
>> Your patch is a better solution if no any unknown regression occurs. But I have no idea about the unknown regressions now.
>> I would like to accept your suggestion and revise my code. **And we should think more situations which could generate regressions to avoid unknown issues occur.**
>>
>> ----
>> Finally, I have a little question.
>> Is the code `newConstr.setAttributes(sym);` necessary in `Resolve.findDiamond`?
>> I delete it in your patch, and test locally by using the following command. All the tests passed.
>>
>> make test TEST="jtreg:langtools/tools/javac" CONF=linux-x86_64-server-release
>
>> Is the code `newConstr.setAttributes(sym);` necessary in `Resolve.findDiamond`?
>
> Sorry, that was a leftover from a previous experiment. That's not necessary. We'll run some more tests on our side and I'll let you know.
> @mcimadamore Should I add you as a contributor of this patch? Does it need another reviewer to review if I add you as a contributor?
As you prefer - I don't mind either way. I don't think adding me as a contributor should prevent the integration, or me being able to sponsor, but I'm not 100% sure.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1490
More information about the compiler-dev
mailing list