On Wed, 7 Jan 2026 17:34:11 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:

...

Please review this change to fix JfrSet to avoid triggering -Wzero-as-null-pointer-constant warnings when that warning is enabled.

The old code uses an entry value with representation 0 to indicate the entry doesn't have a value. It compares an entry value against literal 0 to check for that. If the key type is a pointer type, this involves an implicit 0 => null pointer constant conversion, so we get a warning when that warning is enabled.

Instead we initialize entry values to a value-initialized key, and compare against a value-initialized key. This changes the (currently undocumented) requirements on the key type. The key type is no longer required to be trivially constructible (to permit memset-based initialization), but is now required to be value-initializable. That's currently a wash, since all of the in-use key types are fundamental types (traceid (u8) and Klass*).

Testing: mach5 tier1-3 (tier3 is where most jfr tests are run)

Will review this later Kim, sorry for the delay (26 stuff).

Thanks for reviewing @mgronlun ------------- PR Comment: https://git.openjdk.org/jdk/pull/29022#issuecomment-3756446182

On Thu, 15 Jan 2026 19:13:47 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:

...

Will review this later Kim, sorry for the delay (26 stuff).

Thanks for reviewing @mgronlun

@kimbarrett don't you also need to adjust this for completeness:

``` void clear() { memset(_table, 0, _table_size * sizeof(K)); } ```

Drat! Missed that. https://bugs.openjdk.org/browse/JDK-8375544 ------------- PR Comment: https://git.openjdk.org/jdk/pull/29022#issuecomment-3760548998

On Wed, 14 Jan 2026 15:11:59 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:

...

Please review this change to fix JfrSet to avoid triggering -Wzero-as-null-pointer-constant warnings when that warning is enabled.

The old code uses an entry value with representation 0 to indicate the entry doesn't have a value. It compares an entry value against literal 0 to check for that. If the key type is a pointer type, this involves an implicit 0 => null pointer constant conversion, so we get a warning when that warning is enabled.

Instead we initialize entry values to a value-initialized key, and compare against a value-initialized key. This changes the (currently undocumented) requirements on the key type. The key type is no longer required to be trivially constructible (to permit memset-based initialization), but is now required to be value-initializable. That's currently a wash, since all of the in-use key types are fundamental types (traceid (u8) and Klass*).

Testing: mach5 tier1-3 (tier3 is where most jfr tests are run)

src/hotspot/share/jfr/utilities/jfrSet.hpp line 72:

...

70: } 71: for (unsigned i = 0; i < table_size; ++i) { 72: ::new (&table[i]) K{};

Is this the new (placement pun intended) way to do a placement new, using the outer scope operator ::? Or is it because we don't know what Hotspot type this is?

It's the same old way one should always do it. If one wants global placement new, one should say so. An unqualified `new` expression does a class-based lookup of `operator new`, so if the class has one (and lots of ours do), that will be used. We don't want that here, regardless of the type of `K`. As it happens, for all current uses `K` is a fundamental type, so it doesn't matter. But it's clearer and future proof to be explicit.

src/hotspot/share/jfr/utilities/jfrSet.hpp line 142:

...

140: for (unsigned i = 0; i < old_table_size; ++i) { 141: const K k = old_table[i]; 142: if (k != K{}) {

Are these K{}'s compile constant expressions?

For the types currently used, yes. This is a "value-initialized" (C++17 11.6/8) temporary. For fundamental types, that's a zero-initialized temporary. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29022#discussion_r2695057339 PR Review Comment: https://git.openjdk.org/jdk/pull/29022#discussion_r2695088728