RFR: 8337597: Macros should be defined in the global namespace in src/hotspot/share/memory/metaspace

Kim Barrett kbarrett at openjdk.org
Mon Aug 19 17:26:48 UTC 2024


On Fri, 16 Aug 2024 08:40:17 GMT, Casper Norrbin <duke at openjdk.org> wrote:

> Hi everyone,
> 
> Metaspace currently contains macros that appear to be scoped within the `metaspace` namespace, but in reality, their scope is global. To avoid confusion, these macros should be moved to the global namespace.
> 
> Additionally, there are instances where macros are defined within functions for localized use. Moving these macros outside the functions would introduce unnecessary complexity. Instead, I’ve opted to `#undef` these macros immediately after their use, ensuring they remain accessible only within their respective functions.
> 
> Thanks!

I think the sonarcloud warnings driving some of this are silly.

> Metaspace currently contains macros that appear to be scoped within the `metaspace` namespace, but in reality, their scope is global. To avoid confusion, these macros should be moved to the global namespace.

I see no confusion; macros are well known to not be namespace scoped. I would
prefer to keep the namespace opening at the top of the file's code.  Looking
at the JBS issue, I see this is driven by a sonarcloud warning that I think is
just bogus.  But I'll defer to @tstuefe (as the primary maintainer of
metaspace) if he prefers this style.

> Additionally, there are instances where macros are defined within functions for localized use. Moving these macros outside the functions would introduce unnecessary complexity. Instead, I’ve opted to `#undef` these macros immediately after their use, ensuring they remain accessible only within their respective functions.

The #undef of local macros is reasonable hygiene, though a bit pedantic in the
cases being changed here.

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20610#pullrequestreview-2246084713
PR Comment: https://git.openjdk.org/jdk/pull/20610#issuecomment-2297065100


More information about the hotspot-runtime-dev mailing list