[ADT] Skip destroying trivially destructible values in DenseMap

[kazutakahirata]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/106934.diff

1 Files Affected:

• (modified) llvm/include/llvm/ADT/DenseMap.h (+5)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index f71cd5b4771b75..68f3f1defc2bd0 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -392,6 +392,11 @@ class DenseMapBase : public DebugEpochBase {
   DenseMapBase() = default;
 
   void destroyAll() {
+    // Don't bother destroying trivially destructible stuff.
+    if constexpr (std::is_trivially_destructible_v<KeyT> &&
+                  std::is_trivially_destructible_v<ValueT>)
+      return;
+
     if (getNumBuckets() == 0) // Nothing to do.
       return;
 

[kuhar]

This looks fine in isolation, but I wonder if we do have a test that makes sure that non-trivial destructors get run?

[kazutakahirata]

This looks fine in isolation, but I wonder if we do have a test that makes sure that non-trivial destructors get run?

Let me come up with some tests. Thanks!

[MaskRay]

Test?

[dwblaikie]

Test?

Not sure this is testable? If they're trivially destructible, it wouldn't be possible to observe the difference between destroying them and not destroying them, right?

[kuhar]

Test?

Not sure this is testable? If they're trivially destructible, it wouldn't be possible to observe the difference between destroying them and not destroying them, right?

We could have the test for the non-trivially destructible case to make sure we don't unintentionally skip destructors. If we already do, it would be good to point out in the PR description.

[dwblaikie]

Test?

Not sure this is testable? If they're trivially destructible, it wouldn't be possible to observe the difference between destroying them and not destroying them, right?

We could have the test for the non-trivially destructible case to make sure we don't unintentionally skip destructors. If we already do, it would be good to point out in the PR description.

Sure, that'd be nice - good call.