RFR(T): 8233530: gcc 5.4 build warning -Wc++14-compat after JDK-8233359

[Thomas Stüfe]

Hi David,

sorry for that. I had marked this as trivial, understood your initial
answer as valid Review and pushed this after getting a second review from
Goetz.

But no problem, I can do a follow up patch. What are the remaining
criticisms? Removal of the _GNUG_ define?

Best Regards, Thomas


On Wed, Nov 6, 2019 at 2:09 PM David Holmes <david.holmes at oracle.com> wrote:

> I was surprised to see this got pushed before Kim had had a chance to
> respond and when there was apparently still an open question to me. :(
>
> And now we see it breaks gcc 4.8.5.
>
> David
>
> On 6/11/2019 3:37 pm, Kim Barrett wrote:
> >> On Nov 5, 2019, at 8:44 PM, David Holmes <david.holmes at oracle.com>
> wrote:
> >>
> >> On 6/11/2019 11:38 am, Kim Barrett wrote:
> >>>> On Nov 5, 2019, at 2:06 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
> >>>>
> >>>> Hi all,
> >>>>
> >>>> may I please have reviews for this small build fix:
> >>>>
> >>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8233530
> >>>> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8233530-wc14-compat/webrev.00/webrev/
> >>>> Prior discussion:
> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-November/036726.html
> >>>>
> >>>> Thank you,
> >>>>
> >>>> Thomas
> >>> The "#ifdef __GNUG__" should not be needed or used.
> >>> There are a dozen existing uses of PRAGMA_DIAG_PUSH.
> >>> While there aren't currently any other direct uses of
> >>> PRAGMA_DISABLE_GCC_WARNING, there are indirect uses via other
> >>> PRAGMA_DISABLE_xxx macros.
> >>> None of those have __GNUX__ protections (for any X).
> >>
> >> Well AFAICS they reside in a gcc specific file:
> compilerWarnings_gcc.hpp. But we also take steps in compilerWarnings.hpp to
> accommodate their use in source code when other compilers are used.
> >>
> >> #ifndef PRAGMA_DISABLE_GCC_WARNING
> >> #define PRAGMA_DISABLE_GCC_WARNING(name)
> >> #endif
> >>
> >> so I agree we should not need any guard to make this gcc only - unless
> we really do want to control the version.
> >
> > If we need to control the version it’s applied to, we can use
> PRAGMA_STRINGOP_TRUNCATION_IGNORED as a model.
> > Or just version-conditionalize at the one place it’s needed, for now.
> If more -Wc++14-compat issues come up later
> > then we can introduce a new macro.
> >
> >
>