[foreign-abi] RFR: Rename SystemABI to ForeignLinker, and move C support to a separate class.

Maurizio Cimadamore mcimadamore at openjdk.java.net
Mon May 18 12:02:54 UTC 2020


On Mon, 18 May 2020 11:58:00 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Looks good - there are few things that need more thinking:
>> * the name `C` seems thin. It doesn't describe very well what the class is for. Maybe `CSupport` or something like that
>>   might be more expressive
>> * if we have already C somewhere in the class name, there's a question as to whether constants should drop their C-ness
>>   (e.g. `C.C_BOOL` looks odd).
>> * I wonder if, now that we have a dedicated C class, helpers functions to read/write strings shouldn't just go in there
>>   (e.g. take Cstring and expand its static helpers onto the new class).
>
>> if we have already C somewhere in the class name, there's a question as to whether constants should drop their C-ness
>> (e.g. `C.C_BOOL` looks odd).
>> Thought about this; C.C_BOOL and C.Win64.C_BOOL become C.BOOL,
>> C.Win64.BOOL. That seems fine. Then there's also static import variants;
>> C_BOOL, Win64.C_BOOL that go to BOOL and Win64.BOOL. I feel like for the
>> C_BOOL -> BOOL case some information is lost, and that's the most common
>> case we have it seems. But, maybe it's clear enough from the context
>> that a C layout is being used? I think the main risk is confusing with
>> Java carrier types or layouts (since the names are similar. Feeling a
>> little on the fence about it, so I held off on the first revision.
> 
> I was about to mention when I wrote my message that, one counter argument to my proposal was that, if the code used a
> static import at the top (and you have few examples in your patch) then the code would just use `BOOL`, which might
> also be a bit thin.  In the end, perhaps this is tied with the support class called just `C`; e.g. `C.C_Bool` looks a
> tad weird, `C<something>.C_BOOL` (e.g. `CSupport.C_BOOL`) perhaps not as much.

> the name `C` seems thin. It doesn't describe very well what the class is for. Maybe `CSupport` or something like that
> might be more expressive
> Yeah, that's a good suggestion. Will apply.

The *Support* suffix is something that we use sometimes in the JDK (e.g. `StreamSupport`). If you think that `C` in
`CSupport` is not much, I thought we could tweak to `ClangSupport` or something like that but that will likely lead to
confusion (as it parses as `clang`).

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/174


More information about the panama-dev mailing list