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

[Maurizio Cimadamore]

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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>