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

Thomas Stuefe stuefe at openjdk.org
Tue Aug 20 06:07:53 UTC 2024


On Mon, 19 Aug 2024 17:22:13 GMT, Kim Barrett <kbarrett 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!
>
>> 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.

I agree with @kimbarrett, there is nothing to solve here. Macros are not affected by namespaces. And I prefer opening the namespaces at the start of the file, similar to include guards, since otherwise additions can creep in that are placed outside the namespace too.

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

PR Comment: https://git.openjdk.org/jdk/pull/20610#issuecomment-2298036574


More information about the hotspot-runtime-dev mailing list