Panama source code fails to compile

Ty Young youngty1997 at gmail.com
Mon Jan 4 19:38:57 UTC 2021 

On 1/4/21 12:56 PM, Maurizio Cimadamore wrote:
>
> On 04/01/2021 16:58, Ty Young wrote:
>>
>> On 1/4/21 5:23 AM, Maurizio Cimadamore wrote:
>>> As usual, I find the tone of some of these emails way over the top.
>>>
>>> We do have CI, both internally and externally. I don't think the 
>>> GitHub test was triggered during the merge, so the external CI did 
>>> not run in this case. Our internal CI ran as usual and did not 
>>> report any error - likely, as Remi said, because of a difference in 
>>> the compiler being used.
>>
>>
>> FWIW the changes were never merged into foreign-jextract(master) as 
>> far as I can tell, only foreign-memaccess+abi.
>>
>>
>> The point I was making about the CLI is so people outside of Oracle 
>> know what's working or isn't. This time around it was a bad merge 
>> that introduced a duplicate function, which is the most trivial of 
>> trivial fixes, but I've in the past seen wrong/non-existant 
>> constructors called in Java code before too, in which case waiting 
>> and letting JDK developers fix it is best I think.
>
> In general, Skara allows for some CI results to be published alongside 
> PRs - see for instance:
>
> https://github.com/openjdk/jdk/pull/1611
>
> You can see at the bottom that GiitHub reports that some build/test 
> have failed.
>
> On top of my head I don't know if (a) this is enabled for the Panama 
> repo and (b) if it is enabled for merge changes.


I noticed that too when I was messing around with Github actions on my 
fork and realized that it existed. Bit strange it isn't enabled.


>
>
>>
>> Those builds are generally too old and the changes from one to the 
>> next can be massive. For the longest time it pointed to the old 
>> Pointer API despite it being long abandoned. Things only happen to be 
>> better now because Panama hasn't received major changes in months.
>
> The latest build was released just weeks ago (12/10).
>
> We plan to release more frequently now that we finished hammering the 
> implementation and that all jextract samples have been ported. In 
> other words, there was a reason as to why no new build has been 
> published in such a long time. But we'll do better.


Right, I've always assumed those were more "milestone" releases. Thanks.


>
>>
>>
>> There are more IDEs out there than Intellij. I use Netbeans which, 
>> while increasingly buggy as time goes on, does mostly work and was 
>> not affected by the changes you mention. It handles JDK builds 
>> built-fron-source surprisingly well actually, so long as you use 
>> those builds as your boot JDK and Dr. Deprecator decided to take a 
>> vacation for the JDK development cycle.
>
> Sure, I'm aware of Netbeans :-)
>
> That said, IJ support is so broken right now that I don't think it 
> would be a great idea releasing a binary now when we can release a new 
> one in 2-3 weeks and make sure it works properly with IJ too.
>
>>
>>
>>>
>>> As for the state of the API, as the API is incubating we consider it 
>>> to be our best shot _so far_, but things might still change given 
>>> targeted feedback. Of course, the more we are iterating, the less 
>>> substantial API changes you are seeing.
>>
>>
>> I did a casual look over some of the code and there are a number of 
>> mostly minor cleanups and API modifications that could be made IMO, 
>> although only one so far that I've seen could probably be called major:
>>
>>
>> - AbstractLayout could be modified to use generics so that all layout 
>> types no longer need to override standard MemoryLayout withers. The 
>> Javadoc bug that prevented this from being done has long since been 
>> fixed(JDK-8224052) in JDK 14. I've made the changes already here:
>>
>>
> Thanks for pointing that out - I think I tried that at some point but 
> it was still not working as I expected, but worth giving another try.


It displays the "inherited from MemoryLayout" message and copies over 
the javadoc fine as far as I can tell. I wasn't sure specifically what 
the issue was. Netbeans doesn't like the Javadoc of the with* methods, 
but Netbeans is also buggy in general and no Javadoc is set either for 
the JDK.


>>
>>
>> - Padding layout should probably be a subclass of ValueLayout and 
>> just override methods as needed. Having every memory layout have a 
>> isPadding() method doesn't make much sense. The PaddingLayout class 
>> can then be moved to an internal package as there is no way to create 
>> an instance outside of ofPaddingBits as an API user. The same could 
>> be done for AbstractLayout.
> Are you talking about API or impl? Impl-wise, we could resort to 
> overriding for isPadding, but if PaddingLayout is to be impl-private, 
> then user has no way to do instanceof PaddingLayout, so you need a 
> predicate?


Bit of both. ofValueBits can be modified to return a ValueLayout since, 
under this change, PaddingLayout extends ValueLayout and "is-a" 
ValueLayout. isPadding() will then be made exclusive to ValueLayout and 
would be how you determine if a ValueLayout is padding. If that method 
exists, I don't see a reason why exposing PaddingLayout as part of 
public API and letting a user do instanceof is needed as isPadding() 
exists. isPadding() should maybe use getClass() instead of instanceof, 
don't think it's a big deal but it's slightly faster IIRC. There isn't 
any reason to expose PaddingLayout or AbstractLayout in public API AFAIK.


>>
>>
>> - GroupLayout should probably be split into StructLayout and 
>> UnionLayout. This is the biggest change, but it simplifies the code, 
>> provides better clarity on what's actually being represented, and 
>> allows for future struct/union specific operations to be added 
>> without adding a bunch of if(!isUnion) throw new 
>> UnsupportedOperationException(); type code.
>
> Less sure about this.
>
> I think pattern matching will also help a great deal with this, at 
> some point.


My understanding is that you need a type to do pattern matching? The 
Kind enum used internally for isStruct() and isUnion() is private.


If the issue is unifying the types then GroupLayout could maybe be 
repurposed so that it simply exposes memberLayouts() and other shared code?


>
>>
>>
>> This is just from me skimming the surface API code after submitting 
>> the OCA, there are probably more changes although I doubt there is 
>> anything major.
> Thanks for the comment
>
> Maurizio
>>
>>
>>>
>>> Cheers
>>> Maurizio
>>>
>>>
>>> On 29/12/2020 21:12, Ty Young wrote:
>>>> Yes, mostly. Things happen, especially on holidays when everyone 
>>>> has things to do and places to be. I was going to wait until after 
>>>> the new year to bring it up, but I had seen JDK developers replying 
>>>> to things so I figured they were back.
>>>>
>>>>
>>>> A proper CI could have at least warned that there is an issue, just 
>>>> like other previous issues. Having newer JDK builds would be a 
>>>> benefit to projects like Panama as it reduces the hurdle to try out 
>>>> recent versions, especially those on Windows. Again, probably 
>>>> screaming into the void on the CI thing.
>>>>
>>>>
>>>> On 12/29/20 5:21 AM, Samuel Audet wrote:
>>>>> Hi,
>>>>>
>>>>> I think the point is that this could have been prevented with 
>>>>> proper CI. Since the repos are now on GitHub, OpenJDK could easily 
>>>>> benefit
>>>>> from GitHub Actions...
>>>>>
>>>>> Samuel
>>>>>
>>>>> On Tue, Dec 29, 2020, 16:39 David Holmes <david.holmes at oracle.com 
>>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>>
>>>>>     On 29/12/2020 1:06 pm, Ty Young wrote:
>>>>>     >
>>>>>     > On 12/28/20 3:18 PM, Remi Forax wrote:
>>>>>     >> ----- Mail original -----
>>>>>     >>> De: "Ty Young" <youngty1997 at gmail.com
>>>>>     <mailto:youngty1997 at gmail.com>>
>>>>>     >>> À: "panama-dev at openjdk.java.net
>>>>>     <mailto:panama-dev at openjdk.java.net>'"
>>>>>     <panama-dev at openjdk.java.net 
>>>>> <mailto:panama-dev at openjdk.java.net>>
>>>>>     >>> Envoyé: Lundi 28 Décembre 2020 22:06:58
>>>>>     >>> Objet: Panama source code fails to compile
>>>>>     >>> Just an FYI, the Panama source code have been broken since 
>>>>> before
>>>>>     >>> Christmas with this error:
>>>>>     >>>
>>>>>     >>>
>>>>>     >>> panama-foreign/src/hotspot/share/oops/methodData.cpp:1616:6:
>>>>>     error:
>>>>>     >>> redefinition of 'static bool
>>>>>     MethodData::profile_memory_access(const
>>>>>     >>> methodHandle&, int)'
>>>>>     >>>   1616 | bool MethodData::profile_memory_access(const
>>>>>     methodHandle& m,
>>>>>     >>> int bci) {
>>>>>     >>>        |      ^~~~~~~~~~
>>>>>     >>>
>>>>> /run/media/ty/Windows_Linux_Shared/OpenJDK/panama-foreign/src/hotspot/share/oops/methodData.cpp:1604:6: 
>>>>>
>>>>>
>>>>>     >>>
>>>>>     >>> note: 'static bool MethodData::profile_memory_access(const
>>>>>     >>> methodHandle&, int)' previously defined here
>>>>>     >>>   1604 | bool MethodData::profile_memory_access(const
>>>>>     methodHandle& m,
>>>>>     >>> int bci) {
>>>>>     >>>        |
>>>>>     >>>
>>>>>     >>>
>>>>>     >>> I tried building using the usual `bash configure
>>>>>     >>> --disable-warnings-as-errors` and `make images` on (Arch)
>>>>>     Linux. I am
>>>>>     >>> able to compile the standard JDK from source just fine.
>>>>>     >>>
>>>>>     >>>
>>>>>     >>> I know I'm probably (metaphorically) screaming into the void
>>>>>     here but...
>>>>>     >>> people often have data caps and JDK source code is *A 
>>>>> LOT*. Not to
>>>>>     >>> mention the CPU and memory requirements needed to compile 
>>>>> it in a
>>>>>     >>> reasonable amount of time, which are massive. I suggested
>>>>>     months ago on
>>>>>     >>> the skara-dev list on having build succeeded/failed badges 
>>>>> and/or
>>>>>     >>> automatic JDK builds but apparently no one is interested
>>>>>     because it
>>>>>     >>> hasn't been added. Is making sure your code compiles & runs
>>>>>     before you
>>>>>     >>> push it not a requirement for JDK developers or something? I
>>>>>     understand
>>>>>     >>> it was Christmas but this has happened a few times now.
>>>>>     >> Usually for this kind of error, it's because you are compiling
>>>>>     with a
>>>>>     >> c++ compiler more recent than the one used by the CI,
>>>>>     >> so the bug flies under the radar.
>>>>>     >
>>>>>     >
>>>>>     > There is clearly a duplicate declaration:
>>>>>     >
>>>>>     >
>>>>>     >
>>>>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blob/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1604__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8LnniiGbY$ 
>>>>>
>>>>> <https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blob/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1604__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8LnniiGbY$ 
>>>>> >
>>>>>
>>>>>     >
>>>>>     >
>>>>>     >
>>>>>     > and
>>>>>     >
>>>>>     >
>>>>>     >
>>>>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blob/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1616__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8Ldyj5wAg$ 
>>>>>
>>>>> <https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blob/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1616__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8Ldyj5wAg$ 
>>>>> >
>>>>>
>>>>>     >
>>>>>     >
>>>>>     >
>>>>>     > are the same. Git blame shows that the duplicate was added 17
>>>>>     days ago:
>>>>>     >
>>>>>     >
>>>>>     >
>>>>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blame/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1616__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8L7aTmLhQ$ 
>>>>>
>>>>> <https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/blame/5a8d47fd226708e46f56ec82294e9c786f6850c1/src/hotspot/share/oops/methodData.cpp*L1616__;Iw!!GqivPVa7Brio!ILC6p5xgmmz-ejxPQX_xGplLVDDWejZBFdTxXI-t04hinw_VlVp01-MoqnQmCM8L7aTmLhQ$ 
>>>>> >
>>>>>
>>>>>
>>>>>     I think I see what happened.
>>>>>
>>>>>     JDK-8257692 was fixed in the panama-foreign repo in 
>>>>> foreign-jextract
>>>>>     branch.
>>>>>
>>>>>     JDK-8258242 was fixed in the mainline JDK and contained some of
>>>>>     the same
>>>>>     code changes.
>>>>>
>>>>>     A few days before Christmas there was an auto-merge between
>>>>>     panama-foreign and mainline JDK, and that is when the 
>>>>> duplicate code
>>>>>     chunk has arisen.
>>>>>
>>>>>     David
>>>>>     -----
>>>>>
>>>>>     >
>>>>>     >
>>>>>     > and removing the duplicate allows the JDK to build correctly.
>>>>>     >
>>>>>     >
>>>>>     > So, no, this has nothing to do with the distro or the 
>>>>> compiler I
>>>>>     use.
>>>>>     > I've already been through this blame game nonsense on swing-dev
>>>>>     with JDK
>>>>>     > developers when they broke literally every Swing application
>>>>>     that used
>>>>>     > GTK L&F on Arch Linux, lets not do it again, please.
>>>>>     >
>>>>>     >
>>>>>     >>
>>>>>     >>>
>>>>>     >>> Just a little annoyed, sorry. I try to always use the latest
>>>>>     builds so I
>>>>>     >>> don't have to constantly make large code changes, although
>>>>>     there hasn't
>>>>>     >>> been many changes lately, which is odd. Is Panama's API 
>>>>> nearly
>>>>>     complete?
>>>>>     >>> What's the state of Panama at this point?
>>>>>     >> Rémi
>>>>>

More information about the panama-dev mailing list