On Thu, 4 May 2023 09:50:23 GMT, Axel Boldt-Christmas <abold...@openjdk.org>
wrote:
>>> I'm getting build warnings on all linux platforms with gcc-11.3.0:
>>>
>>> ```
>>> src/hotspot/share/gc/z/zDriver.cpp:84:13: error: In the GNU C Library,
>>> "minor" is defined
>>> by <sys/sysmacros.h>. For historical compatibility, it is
>>> currently defined by <sys/types.h> as well, but we plan to
>>> remove this soon. To use "minor", include <sys/sysmacros.h>
>>> directly. If you did not intend to use a system-defined macro
>>> "minor", you should undefine it after including <sys/types.h>. [-Werror]
>>> 84 | ZDriverMinor* ZDriver::minor() {
>>> ```
>>
>> That's unfortunate as minor and major are quite central to Generational ZGC
>> and having to rename those functions will make the code look worse. I wonder
>> if we should undef minor and major where needed.
>
>> I'm getting build warnings on all linux platforms with gcc-11.3.0:
>>
>> ```
>> src/hotspot/share/gc/z/zDriver.cpp:84:13: error: In the GNU C Library,
>> "minor" is defined
>> by <sys/sysmacros.h>. For historical compatibility, it is
>> currently defined by <sys/types.h> as well, but we plan to
>> remove this soon. To use "minor", include <sys/sysmacros.h>
>> directly. If you did not intend to use a system-defined macro
>> "minor", you should undefine it after including <sys/types.h>. [-Werror]
>> 84 | ZDriverMinor* ZDriver::minor() {
>> ```
>
> @TheRealMDoerr I cannot reproduce this with gcc but can see the issue with
> clangd.
> Can you check if this patch solves the issue you are seeing?
>
> diff --git a/src/hotspot/share/gc/z/zDriver.hpp
> b/src/hotspot/share/gc/z/zDriver.hpp
> index 640ea6575ef..7fa650b1fa1 100644
> --- a/src/hotspot/share/gc/z/zDriver.hpp
> +++ b/src/hotspot/share/gc/z/zDriver.hpp
> @@ -29,6 +29,14 @@
> #include "gc/z/zThread.hpp"
> #include "gc/z/zTracer.hpp"
>
> +#ifdef minor
> +#undef minor
> +#endif
> +
> +#ifdef major
> +#undef major
> +#endif
> +
> class VM_ZOperation;
> class ZDriverMinor;
> class ZDriverMajor;
> @xmas92: Thanks for your quick solution. Your patch solves the problem. If
> you want to integrate it, please also add a comment why this is needed.
Thanks for testing it. Will do.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13771#issuecomment-1534586643
