[foreign] RFR 8216268: duplicate declaration handler has to be a tree visitor

[Sundararajan Athijegannathan]

Thanks for the review.

Yes - we do change the way duplicates are detected and handled - in a 
tree phase rather than late in codegen. I didn't see any errors in tests 
as well as with samples. But earlier there was a overwrite warning (for 
duplicate decl. from the same file) - but that does not occur anymore:

File test.h

struct Foo;
struct Foo;

Thanks,
-Sundar

On 09/01/19, 5:44 PM, Maurizio Cimadamore wrote:
> Great work.
>
> I like it that:
>
> 1) It moves duplicate detection logic out of TypedefHandler (which is 
> there for another reason)
> 2) and also out of ASMCodeFactory
>
> One element of risk that I note here, is that the new code isn't 
> exactly a functional replacement for the old one, in that some of the 
> checks (e.g. those in ASMCodeFactory) used to be on a per-header 
> basis, while with your new visitor pass, everything is applied on the 
> header trees produced by the parser. I can imagine situations in which 
> the old code did not detect conflict (e.g. A$foo vs. B$foo) whereas 
> the new approach detects the conflict and generates only one class.
>
> I think the new approach is probably for the best and more in sync 
> with where we need to be longer term anyway. But just want to double 
> check on that.
>
> Other than that, +1
>
> Maurizio
>
> On 09/01/2019 05:18, Sundararajan Athijegannathan wrote:
>> Please review.
>>
>> Updated webrev: 
>> https://cr.openjdk.java.net/~sundar/8216268/webrev.01/index.html
>>
>> PS. Updated it for the recent class loading changes in the test 
>> framework.
>>
>> -Sundar
>>
>> On 07/01/19, 7:09 PM, Sundararajan Athijegannathan wrote:
>>> Please review.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8216268
>>> Webrev: 
>>> https://cr.openjdk.java.net/~sundar/8216268/webrev.00/index.html
>>>
>>> Thanks
>>> -Sundar