[foreign] RFR 8218742: Refine Scope API

[Maurizio Cimadamore]

Hi, here's v2:

http://cr.openjdk.java.net/~mcimadamore/panama/8218742_v2/

I addressed most of the comments, see below

On 11/02/2019 13:46, Jorn Vernee wrote:
> Hi Maurizio,
>
> The patch does not apply cleanly after JDK-8218669 [1] was merged. I'm 
> getting some merge failures in References.java
>
> About ScopeImpl;
>
>   - The parent scope field is called `owner`, but the accessor is 
> called `parent()`, I'd say go with either "parent" or "owner" as a 
> name for both.
fixed
>
>   - Also, There are a bunch of casts to ScopeImpl, mostly in the 
> implementation of ScopeImpl::merge. Should the field's type be changed 
> to ScopeImpl instead, and have 1 cast in the constructor?
fixed
>
> - Changes in jextract/staticForwarder/StaticForwarderTest.java don't 
> seem to be needed?
Not sure what you mean - the static forwarded class is a place where we 
can have an accessor for the library scope - that's what the changes do 
(and the test checks that such accessor works).
>
> - Also, Scope inference is in the proposal, but it doesn't seem to be 
> implemented with this patch. I guess that will come later?

In this revision I've added inference - in the sense that when calling 
Reference.OfPointer::get we know infer the scope of the extracted 
pointer to be the same as that of the pointer we're using for dereference.

I believe there's some more work to do, especially for native functions 
- ideally you'd like things coming out of a native function to use the 
library scope - but this is problematic because (1) when we create the 
invokers MH we do not necessarily have a library scope available (this 
is the same issue as with globals, which is made more complex by the 
fact that method handles are created by SystemABI and that API does not 
depend on scopes - nor we want it to, I believe). Another issue is that 
for (2) callback arguments (e.g. think of a callback accepting a 
pointer) we'd like again for the scope to be inferred as the library 
scope that's making the callback call - but I'm not sure that's even 
possible, since we jump into the callback from C code, which knows very 
little about scopes.

The current patch uses an unchecked scope in both cases.

P.S.

I'm not 100% sure that the new meaning of library scope - as a scope 
associated with a specific _instance_ of a library (as opposed to class) 
is more useful or harmful - it certainly makes a lot of the code 
generation harder because now all handles we use might, in principle, 
depend on scope which is stored in an instance field of the generated class.

Maurizio


>
> Rest looks good,
> Jorn
>
> [1] : https://bugs.openjdk.java.net/projects/JDK/issues/JDK-8218669
>
> Maurizio Cimadamore schreef op 2019-02-11 14:03:
>> Hi,
>> this is an official RFR for the changes discussed in the writeup in
>> [1] and in the subsequent thread [2].
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8218742/
>>
>> I've updated the examples document to reflect the API changes, as well
>> as polished the code, fixup javadocs etc.
>>
>> The tests pass on all platforms.
>>
>> Cheers
>> Maurizio
>>
>> [1] - http://cr.openjdk.java.net/~mcimadamore/panama/scopes.html
>> [2] -
>> https://mail.openjdk.java.net/pipermail/panama-dev/2019-January/003926.html 
>>