From 4ab74cdc124af6b4f57c2d2d09548e01d64a1f34 Mon Sep 17 00:00:00 2001 From: Jeffrey Yasskin Date: Fri, 23 Oct 2009 20:54:00 +0000 Subject: [PATCH] Fix stylistic and documentation problems in ValueMap found by Nick Lewycky and Evan Cheng. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@84967 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/DenseMap.h | 4 +-- include/llvm/ADT/ValueMap.h | 52 ++++++++++++++++++++-------------- unittests/ADT/ValueMapTest.cpp | 6 ++-- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/include/llvm/ADT/DenseMap.h b/include/llvm/ADT/DenseMap.h index 0ed2d5a2525..52354b7f222 100644 --- a/include/llvm/ADT/DenseMap.h +++ b/include/llvm/ADT/DenseMap.h @@ -454,12 +454,12 @@ public: return Ptr != RHS.Ptr; } - inline DenseMapIterator& operator++() { // Preincrement + inline DenseMapIterator& operator++() { // Preincrement ++Ptr; AdvancePastEmptyBuckets(); return *this; } - DenseMapIterator operator++(int) { // Postincrement + DenseMapIterator operator++(int) { // Postincrement DenseMapIterator tmp = *this; ++*this; return tmp; } diff --git a/include/llvm/ADT/ValueMap.h b/include/llvm/ADT/ValueMap.h index 14f21001df7..6d394fd2404 100644 --- a/include/llvm/ADT/ValueMap.h +++ b/include/llvm/ADT/ValueMap.h @@ -7,7 +7,19 @@ // //===----------------------------------------------------------------------===// // -// This file defines the ValueMap class. +// This file defines the ValueMap class. ValueMap maps Value* or any subclass +// to an arbitrary other type. It provides the DenseMap interface but updates +// itself to remain safe when keys are RAUWed or deleted. By default, when a +// key is RAUWed from V1 to V2, the old mapping V1->target is removed, and a new +// mapping V2->target is added. If V2 already existed, its old target is +// overwritten. When a key is deleted, its mapping is removed. +// +// You can override a ValueMap's Config parameter to control exactly what +// happens on RAUW and destruction and to get called back on each event. It's +// legal to call back into the ValueMap from a Config's callbacks. Config +// parameters should inherit from ValueMapConfig to get default +// implementations of all the methods ValueMap uses. See ValueMapConfig for +// documentation of the functions you can override. // //===----------------------------------------------------------------------===// @@ -31,6 +43,9 @@ class ValueMapIterator; template class ValueMapConstIterator; +/// This class defines the default behavior for configurable aspects of +/// ValueMap<>. User Configs should inherit from this class to be as compatible +/// as possible with future versions of ValueMap. template struct ValueMapConfig { /// If FollowRAUW is true, the ValueMap will update mappings on RAUW. If it's @@ -46,27 +61,17 @@ struct ValueMapConfig { template static void onRAUW(const ExtraDataT &Data, KeyT Old, KeyT New) {} template - static void onDeleted(const ExtraDataT &Data, KeyT Old) {} + static void onDelete(const ExtraDataT &Data, KeyT Old) {} /// Returns a mutex that should be acquired around any changes to the map. /// This is only acquired from the CallbackVH (and held around calls to onRAUW - /// and onDeleted) and not inside other ValueMap methods. NULL means that no + /// and onDelete) and not inside other ValueMap methods. NULL means that no /// mutex is necessary. template static sys::Mutex *getMutex(const ExtraDataT &Data) { return NULL; } }; -/// ValueMap maps Value* or any subclass to an arbitrary other -/// type. It provides the DenseMap interface. When the key values are -/// deleted or RAUWed, ValueMap relies on the Config to decide what to -/// do. Config parameters should inherit from ValueMapConfig to -/// get default implementations of all the methods ValueMap uses. -/// -/// By default, when a key is RAUWed from V1 to V2, the old mapping -/// V1->target is removed, and a new mapping V2->target is added. If -/// V2 already existed, its old target is overwritten. When a key is -/// deleted, its mapping is removed. You can override Config to get -/// called back on each event. +/// See the file comment. template, typename ValueInfoT = DenseMapInfo > class ValueMap { @@ -177,6 +182,9 @@ public: } private: + // Takes a key being looked up in the map and wraps it into a + // ValueMapCallbackVH, the actual key type of the map. We use a helper + // function because ValueMapCVH is constructed with a second parameter. ValueMapCVH Wrap(KeyT key) const { // The only way the resulting CallbackVH could try to modify *this (making // the const_cast incorrect) is if it gets inserted into the map. But then @@ -186,6 +194,8 @@ private: } }; +// This CallbackVH updates its ValueMap when the contained Value changes, +// according to the user's preferences expressed through the Config object. template class ValueMapCallbackVH : public CallbackVH { friend class ValueMap; @@ -208,7 +218,7 @@ public: sys::Mutex *M = Config::getMutex(Copy.Map->Data); if (M) M->acquire(); - Config::onDeleted(Copy.Map->Data, Copy.Unwrap()); // May destroy *this. + Config::onDelete(Copy.Map->Data, Copy.Unwrap()); // May destroy *this. Copy.Map->Map.erase(Copy); // Definitely destroys *this. if (M) M->release(); @@ -279,7 +289,7 @@ public: struct ValueTypeProxy { const KeyT first; ValueT& second; - ValueTypeProxy *operator->() { return this; } + ValueTypeProxy *operator->() { return this; } operator std::pair() const { return std::make_pair(first, second); } @@ -301,11 +311,11 @@ public: return I != RHS.I; } - inline ValueMapIterator& operator++() { // Preincrement + inline ValueMapIterator& operator++() { // Preincrement ++I; return *this; } - ValueMapIterator operator++(int) { // Postincrement + ValueMapIterator operator++(int) { // Postincrement ValueMapIterator tmp = *this; ++*this; return tmp; } }; @@ -329,7 +339,7 @@ public: struct ValueTypeProxy { const KeyT first; const ValueT& second; - ValueTypeProxy *operator->() { return this; } + ValueTypeProxy *operator->() { return this; } operator std::pair() const { return std::make_pair(first, second); } @@ -351,11 +361,11 @@ public: return I != RHS.I; } - inline ValueMapConstIterator& operator++() { // Preincrement + inline ValueMapConstIterator& operator++() { // Preincrement ++I; return *this; } - ValueMapConstIterator operator++(int) { // Postincrement + ValueMapConstIterator operator++(int) { // Postincrement ValueMapConstIterator tmp = *this; ++*this; return tmp; } }; diff --git a/unittests/ADT/ValueMapTest.cpp b/unittests/ADT/ValueMapTest.cpp index 9de340c3f35..e4eec75ede9 100644 --- a/unittests/ADT/ValueMapTest.cpp +++ b/unittests/ADT/ValueMapTest.cpp @@ -187,7 +187,7 @@ struct LockMutex : ValueMapConfig { *Data.CalledRAUW = true; EXPECT_FALSE(Data.M->tryacquire()) << "Mutex should already be locked."; } - static void onDeleted(const ExtraData &Data, KeyT Old) { + static void onDelete(const ExtraData &Data, KeyT Old) { *Data.CalledDeleted = true; EXPECT_FALSE(Data.M->tryacquire()) << "Mutex should already be locked."; } @@ -238,7 +238,7 @@ struct CountOps : ValueMapConfig { static void onRAUW(const ExtraData &Data, KeyT Old, KeyT New) { ++*Data.RAUWs; } - static void onDeleted(const ExtraData &Data, KeyT Old) { + static void onDelete(const ExtraData &Data, KeyT Old) { ++*Data.Deletions; } }; @@ -270,7 +270,7 @@ struct ModifyingConfig : ValueMapConfig { static void onRAUW(ExtraData Map, KeyT Old, KeyT New) { (*Map)->erase(Old); } - static void onDeleted(ExtraData Map, KeyT Old) { + static void onDelete(ExtraData Map, KeyT Old) { (*Map)->erase(Old); } }; -- 2.34.1