RFR: 8306901: Macro offset_of confuses Eclipse CDT

[Julian Waters]

On Thu, 27 Apr 2023 10:00:57 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> > > Also I wonder whether the ATTRIBUTE_ALIGNED(16) would be more correctly `alignas(klass)` but that is pre-existing.
>>> > 
>>> > 
>>> > Probably :) At least it would be cleaner in my eyes than the hard coded 16. With this pr I'd like to address just the CDT issue though.
>>> 
>>> I believe the trick may not work if the type were artificially aligned to something larger than16, e.g. struct XX alignas(64). I guess that depends on what the compiler does when casting a pointer with a smaller alignment to something with a larger alignemnt.
>> 
>> Maybe. I'm not familiar enough with `alignas` right now. Also I would have to 'meditate' a bit about [the alignas-part in the hotspot-style guide](https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md#alignas).
>> Right now I wouldn't expect extended alignment of a type to affect member offsets. But that might be naive.
>
>> > > > Also I wonder whether the ATTRIBUTE_ALIGNED(16) would be more correctly `alignas(klass)` but that is pre-existing.
>> > > 
>> > > 
>> > > Probably :) At least it would be cleaner in my eyes than the hard coded 16. With this pr I'd like to address just the CDT issue though.
>> > 
>> > 
>> > I believe the trick may not work if the type were artificially aligned to something larger than16, e.g. struct XX alignas(64). I guess that depends on what the compiler does when casting a pointer with a smaller alignment to something with a larger alignemnt.
>> 
>> Maybe. I'm not familiar enough with `alignas` right now. Also I would have to 'meditate' a bit about [the alignas-part in the hotspot-style guide](https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md#alignas). Right now I wouldn't expect extended alignment of a type to affect member offsets. But that might be naive.
> 
> Checked with GCC and clang, it seems to work. But I think its still UB (otherwise we could just omit the ALIGN specifier completely).

@tstuefe Currently there is a change planned to replace the macro in some place with alignas (see #11431) but I haven't had time to get round to finishing it just yet

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

PR Comment: https://git.openjdk.org/jdk/pull/13668#issuecomment-1527547745