On Thu, 24 Sep 2020 21:28:01 GMT, Paul Hohensee <p...@openjdk.org> wrote:
> Please review this small patch to enable the OSX build using Xcode 12.0. > > Thanks, > Paul No, don't do this. In the original, double is used to obtain the desired alignmnent. Changing the element type to short reduces the alignment requirement for the variable. initialize_shared_locs will then adjust the alignment, potentially shrinking the effective array size. So this is a real change that I think shouldn't be made. I think changing the declaration for locs_buf to any of the following gives equivalent behavior to the current code. I don't know whether any of them will trigger the new clang warning though. I don't have access to that version right now, and the documentation for the warning is useless (like so much of clang's warning options documentation). (1) alignas(double) char locs_buf[20 * sizeof(double)]; (but alignas is not yet in the "permitted features" list in the Style Guide: https://bugs.openjdk.java.net/browse/JDK-8252584) (2) union { char locs_buf[20 * sizeof(double)]; double align; }; (3) std::aligned_storage_t<20 * sizeof(double)> locs_buf; and change (relocInfo*)locs_buf => (relocInfo*)&locs_buf and #include <type_traits> This has the benefit of being explicit that we're just stack allocating a block of storage. I'll make a wild guess that (1) and (2) will still warn, though char arrays are somewhat special in the language so maybe they won't. I'm pretty sure (3) should do the trick. ------------- Changes requested by kbarrett (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/348
