RFR: 8257037: No javac warning when calling deprecated constructor with diamond [v2]

[Maurizio Cimadamore]

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