[foreign] RFR duplicates in named layouts have to be reported as error

[Sundararajan Athijegannathan]

Looks good.

Minor: the new test has copyright year -> 2016

-Sundar

On 11/06/18, 8:09 PM, Maurizio Cimadamore wrote:
> To be clear, here's what I'm proposing as a way to limit complexity:
>
> http://cr.openjdk.java.net/~mcimadamore/panama/resolution_context_v2/
>
> I've added a bunch of 'resolveLayout/Function' methods in Util (but 
> they could also go in LayoutResolver...) - the binder calls them when 
> needed. ABI is now free of unresolved thingies.
>
> Maurizio
>
>
> On 11/06/18 14:31, Maurizio Cimadamore wrote:
>> In hindsight, I believe the very first changeset submitted in this 
>> thread was the correct one. Avoiding changes in the binder internal 
>> by pushing more complexity onto the parser/unresolved layout API was 
>> a siren song, which ultimately leads to a bad complexity ratio for 
>> clients of the layout API.
>>
>> If the binder impl is more complex by having to deal with unresolved 
>> layouts explicitly, well, that's not optimal (because it's a source 
>> of potential bugs), but it's still way better than having _all_ 
>> external clients having to care about resolution context.
>>
>> Perhaps, one improvement we could concretely make in this area is to 
>> have an Util::resolve(Layout) which takes a partial layout and gives 
>> you an instantiated layout. That way, we could force resolution in 
>> places where we need to - not just on single unresolved nodes, but on 
>> entire layout trees.
>>
>> I think this could help improving code uniformity and also reducing 
>> impact of the changes - e.g. with this I think we could avoid sending 
>> unresolved layouts into the ABI in the first place?
>>
>> Maurizio
>>
>>
>> On 05/06/18 09:55, Sundararajan Athijegannathan wrote:
>>> As I understand the main change (from what I had earlier) is to use 
>>> the most enclosing class as context for layout resolution - 
>>> regardless of whether it has NativeHeader annotation or not. Perhaps 
>>> the tests can avoid NativeHeader annotation now.
>>>
>>> For eg. these
>>>
>>>  test/jdk/java/nicl/abi/sysv/x64/CallingSequenceBuilderTest.java
>>>  test/jdk/java/nicl/types/StructTest.java
>>>
>>>  do not need NativeHeader annotation anymore.
>>>
>>> Rest is fine.
>>>
>>> -Sundar
>>>
>>> On 05/06/18, 2:14 PM, Maurizio Cimadamore wrote:
>>>> Sorry - this is it:
>>>>
>>>> http://cr.openjdk.java.net/~mcimadamore/panama/resolution_context/
>>>>
>>>> Maurizio
>>>>
>>>>
>>>> On 04/06/18 03:24, Sundararajan Athijegannathan wrote:
>>>>>
>>>>> Sorry I was on vacation.  The link you provided seems to be the 
>>>>> initial named-layout implementation webrev. Will you please 
>>>>> provide the correct link?
>>>>>
>>>>> -Sundar
>>>>>
>>>>> On 28/05/18, 6:04 PM, Maurizio Cimadamore wrote:
>>>>>> Sorry, wrong webrev link, here's the correct one:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~mcimadamore/panama/layout_resolver/
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 28/05/18 13:32, Maurizio Cimadamore wrote:
>>>>>>> Here's a stab at the current approach; I started with your patch 
>>>>>>> and tweaked in places.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~mcimadamore/panama/panama-binder-v3.html 
>>>>>>>
>>>>>>>
>>>>>>> I think the result is good; the problems you mention are 
>>>>>>> resolved, and all use cases are supported.
>>>>>>>
>>>>>>> Let me know what you think.
>>>>>>>
>>>>>>> Maurizio
>>>>>>>
>>>>>>>
>>>>>>> On 28/05/18 11:02, Maurizio Cimadamore wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 28/05/18 09:37, Maurizio Cimadamore wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 28/05/18 07:39, Sundararajan Athijegannathan wrote:
>>>>>>>>>> Isolated Structs will work if there are no name definitions 
>>>>>>>>>> or references in its layout. If there are Unresolved name 
>>>>>>>>>> references in its layout, then there has to be a 'context' in 
>>>>>>>>>> which the names are defined/resolved. The context cannot be 
>>>>>>>>>> entire Java process as it currently is! Two issues with the 
>>>>>>>>>> current code: 
>>>>>>>>> I agree with all you say. The only thing I'm disagreeing with 
>>>>>>>>> is the fact that the context has to be the @NativeHeader 
>>>>>>>>> annotation, as that suggests an header-centric nature that was 
>>>>>>>>> never intended to be part of the design.
>>>>>>>>>
>>>>>>>>> Other, more explicit ways to define the context include an 
>>>>>>>>> explicit annotation which list all the classes where dependent 
>>>>>>>>> layout can be found. I think I'd like something like this 
>>>>>>>>> better. Example (names TBD):
>>>>>>>>>
>>>>>>>>> @NativeStruct("[$(b)](a)")
>>>>>>>>> @Friends(B.class)
>>>>>>>>> interface A {
>>>>>>>>>     B b();
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> @NativeStruct("[$(a)](b)")
>>>>>>>>> @Friends(A.class)
>>>>>>>>> interface B {
>>>>>>>>>     A a();
>>>>>>>>> }
>>>>>>>> Now that I have expressed this more explicitly, another idea 
>>>>>>>> emerged - what if we assumed that, if no annotation is 
>>>>>>>> provided, the context is the outermost class? That will work 
>>>>>>>> neutrally with both struct and header interfaces...
>>>>>>>>
>>>>>>>> Maurizio
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Maurizio
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
>