[llvm] aee6b8e - [ADT] Explicitly delete copy/move constructors and operator= in IntervalMap

[Krzysztof Parzyszek via llvm-commits]

Author: Krzysztof Parzyszek
Date: 2022-05-26T07:58:18-07:00
New Revision: aee6b8efd09c014348c96b81d64961f8e7a4ab6a

URL: https://github.com/llvm/llvm-project/commit/aee6b8efd09c014348c96b81d64961f8e7a4ab6a
DIFF: https://github.com/llvm/llvm-project/commit/aee6b8efd09c014348c96b81d64961f8e7a4ab6a.diff

LOG: [ADT] Explicitly delete copy/move constructors and operator= in IntervalMap

The default implementations will perform a shallow copy instead of a deep
copy, causing some internal data structures to be shared between different
objects. Disable these operations so they don't get accidentally used.

Differential Revision: https://reviews.llvm.org/D126401

Added: 
    

Modified: 
    llvm/include/llvm/ADT/IntervalMap.h
    llvm/unittests/ADT/IntervalMapTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/IntervalMap.h b/llvm/include/llvm/ADT/IntervalMap.h
index 3c65a108dcd8..57f02df252c0 100644
--- a/llvm/include/llvm/ADT/IntervalMap.h
+++ b/llvm/include/llvm/ADT/IntervalMap.h
@@ -1042,6 +1042,17 @@ class IntervalMap {
     new(&rootLeaf()) RootLeaf();
   }
 
+  // The default copy/move constructors and assignment operators would perform
+  // a shallow copy, leading to an incorrect internal state. To prevent
+  // accidental use, explicitly delete these operators.
+  // If necessary, implement them to perform a deep copy.
+  IntervalMap(const IntervalMap &Other) = delete;
+  IntervalMap(IntervalMap &&Other) = delete;
+  // Note: these are already implicitly deleted, because RootLeaf (union
+  // member) has a non-trivial assignment operator (because of std::pair).
+  IntervalMap &operator=(const IntervalMap &Other) = delete;
+  IntervalMap &operator=(IntervalMap &&Other) = delete;
+
   ~IntervalMap() {
     clear();
     rootLeaf().~RootLeaf();

diff  --git a/llvm/unittests/ADT/IntervalMapTest.cpp b/llvm/unittests/ADT/IntervalMapTest.cpp
index 65a9064cbc3b..07cda05f4f59 100644
--- a/llvm/unittests/ADT/IntervalMapTest.cpp
+++ b/llvm/unittests/ADT/IntervalMapTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/ADT/IntervalMap.h"
 #include "gtest/gtest.h"
+#include <type_traits>
 
 using namespace llvm;
 
@@ -17,6 +18,15 @@ typedef IntervalMap<unsigned, unsigned, 4> UUMap;
 typedef IntervalMap<unsigned, unsigned, 4,
                     IntervalMapHalfOpenInfo<unsigned>> UUHalfOpenMap;
 
+static_assert(!std::is_copy_constructible<UUMap>::value,
+              "IntervalMap copy constructor should be deleted");
+static_assert(!std::is_move_constructible<UUMap>::value,
+              "IntervalMap move constructor should be deleted");
+static_assert(!std::is_copy_assignable<UUMap>::value,
+              "IntervalMap copy assignment should be deleted");
+static_assert(!std::is_move_assignable<UUMap>::value,
+              "IntervalMap move assignment should be deleted");
+
 // Empty map tests
 TEST(IntervalMapTest, EmptyMap) {
   UUMap::Allocator allocator;