From 774b584f425df71418bf439cc1f11886f1f4f30f Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 3 Aug 2015 22:30:24 +0000 Subject: [PATCH] -Wdeprecated-clean: Fix cases of violating the rule of 5 in ways that are deprecated in C++11 Various value handles needed to be copy constructible and copy assignable (mostly for their use in DenseMap). But to avoid an API that might allow accidental slicing, make these members protected in the base class and make derived classes final (the special members become implicitly public there - but disallowing further derived classes that might be sliced to the intermediate type). Might be worth having a warning a bit like -Wnon-virtual-dtor that catches public move/copy assign/ctors in classes with virtual functions. (suppressable in the same way - by making them protected in the base, and making the derived classes final) Could be fancier and only diagnose them when they're actually called, potentially. Also allow a few default implementations where custom implementations (especially with non-standard return types) were implemented. llvm-svn: 243909 --- llvm/include/llvm/Analysis/AliasSetTracker.h | 2 +- llvm/include/llvm/Analysis/AssumptionCache.h | 2 +- llvm/include/llvm/Analysis/IVUsers.h | 2 +- llvm/include/llvm/Analysis/ScalarEvolution.h | 2 +- .../llvm/Analysis/ScalarEvolutionExpressions.h | 2 +- llvm/include/llvm/IR/ValueHandle.h | 30 +++++++++++----------- llvm/include/llvm/IR/ValueMap.h | 4 +-- llvm/lib/Analysis/CFLAliasAnalysis.cpp | 4 +-- llvm/lib/Analysis/LazyValueInfo.cpp | 2 +- llvm/lib/CodeGen/MachineModuleInfo.cpp | 2 +- llvm/lib/ExecutionEngine/ExecutionEngine.cpp | 2 +- llvm/unittests/IR/ValueHandleTest.cpp | 12 ++++----- 12 files changed, 32 insertions(+), 34 deletions(-) diff --git a/llvm/include/llvm/Analysis/AliasSetTracker.h b/llvm/include/llvm/Analysis/AliasSetTracker.h index 881699d..9829e7f 100644 --- a/llvm/include/llvm/Analysis/AliasSetTracker.h +++ b/llvm/include/llvm/Analysis/AliasSetTracker.h @@ -286,7 +286,7 @@ inline raw_ostream& operator<<(raw_ostream &OS, const AliasSet &AS) { class AliasSetTracker { /// CallbackVH - A CallbackVH to arrange for AliasSetTracker to be /// notified whenever a Value is deleted. - class ASTCallbackVH : public CallbackVH { + class ASTCallbackVH final : public CallbackVH { AliasSetTracker *AST; void deleted() override; void allUsesReplacedWith(Value *) override; diff --git a/llvm/include/llvm/Analysis/AssumptionCache.h b/llvm/include/llvm/Analysis/AssumptionCache.h index 9de8ed5..b903f96 100644 --- a/llvm/include/llvm/Analysis/AssumptionCache.h +++ b/llvm/include/llvm/Analysis/AssumptionCache.h @@ -140,7 +140,7 @@ public: class AssumptionCacheTracker : public ImmutablePass { /// A callback value handle applied to function objects, which we use to /// delete our cache of intrinsics for a function when it is deleted. - class FunctionCallbackVH : public CallbackVH { + class FunctionCallbackVH final : public CallbackVH { AssumptionCacheTracker *ACT; void deleted() override; diff --git a/llvm/include/llvm/Analysis/IVUsers.h b/llvm/include/llvm/Analysis/IVUsers.h index 00dbcbd..37d0149 100644 --- a/llvm/include/llvm/Analysis/IVUsers.h +++ b/llvm/include/llvm/Analysis/IVUsers.h @@ -34,7 +34,7 @@ class DataLayout; /// The Expr member keeps track of the expression, User is the actual user /// instruction of the operand, and 'OperandValToReplace' is the operand of /// the User that is the use. -class IVStrideUse : public CallbackVH, public ilist_node { +class IVStrideUse final : public CallbackVH, public ilist_node { friend class IVUsers; public: IVStrideUse(IVUsers *P, Instruction* U, Value *O) diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h index c45fdb2..772028c 100644 --- a/llvm/include/llvm/Analysis/ScalarEvolution.h +++ b/llvm/include/llvm/Analysis/ScalarEvolution.h @@ -210,7 +210,7 @@ namespace llvm { private: /// SCEVCallbackVH - A CallbackVH to arrange for ScalarEvolution to be /// notified whenever a Value is deleted. - class SCEVCallbackVH : public CallbackVH { + class SCEVCallbackVH final : public CallbackVH { ScalarEvolution *SE; void deleted() override; void allUsesReplacedWith(Value *New) override; diff --git a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h index da24de2..d5a3fc4 100644 --- a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h +++ b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h @@ -404,7 +404,7 @@ namespace llvm { /// value, and only represent it as its LLVM Value. This is the "bottom" /// value for the analysis. /// - class SCEVUnknown : public SCEV, private CallbackVH { + class SCEVUnknown final : public SCEV, private CallbackVH { friend class ScalarEvolution; // Implement CallbackVH. diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h index 53fa80a..d5ef6c5 100644 --- a/llvm/include/llvm/IR/ValueHandle.h +++ b/llvm/include/llvm/IR/ValueHandle.h @@ -52,13 +52,21 @@ protected: Weak }; + ValueHandleBase(const ValueHandleBase &RHS) + : ValueHandleBase(RHS.PrevPair.getInt(), RHS) {} + + ValueHandleBase(HandleBaseKind Kind, const ValueHandleBase &RHS) + : PrevPair(nullptr, Kind), Next(nullptr), V(RHS.V) { + if (isValid(V)) + AddToExistingUseList(RHS.getPrevPtr()); + } + private: PointerIntPair PrevPair; ValueHandleBase *Next; Value* V; - ValueHandleBase(const ValueHandleBase&) = delete; public: explicit ValueHandleBase(HandleBaseKind Kind) : PrevPair(nullptr, Kind), Next(nullptr), V(nullptr) {} @@ -67,11 +75,7 @@ public: if (isValid(V)) AddToUseList(); } - ValueHandleBase(HandleBaseKind Kind, const ValueHandleBase &RHS) - : PrevPair(nullptr, Kind), Next(nullptr), V(RHS.V) { - if (isValid(V)) - AddToExistingUseList(RHS.getPrevPtr()); - } + ~ValueHandleBase() { if (isValid(V)) RemoveFromUseList(); @@ -145,6 +149,8 @@ public: WeakVH(const WeakVH &RHS) : ValueHandleBase(Weak, RHS) {} + WeakVH &operator=(const WeakVH &RHS) = default; + Value *operator=(Value *RHS) { return ValueHandleBase::operator=(RHS); } @@ -314,7 +320,6 @@ class TrackingVH : public ValueHandleBase { public: TrackingVH() : ValueHandleBase(Tracking) {} TrackingVH(ValueTy *P) : ValueHandleBase(Tracking, GetAsValue(P)) {} - TrackingVH(const TrackingVH &RHS) : ValueHandleBase(Tracking, RHS) {} operator ValueTy*() const { return getValPtr(); @@ -324,10 +329,6 @@ public: setValPtr(RHS); return getValPtr(); } - ValueTy *operator=(const TrackingVH &RHS) { - setValPtr(RHS.getValPtr()); - return getValPtr(); - } ValueTy *operator->() const { return getValPtr(); } ValueTy &operator*() const { return *getValPtr(); } @@ -344,10 +345,9 @@ public: class CallbackVH : public ValueHandleBase { virtual void anchor(); protected: - CallbackVH(const CallbackVH &RHS) - : ValueHandleBase(Callback, RHS) {} - - virtual ~CallbackVH() {} + ~CallbackVH() = default; + CallbackVH(const CallbackVH &) = default; + CallbackVH &operator=(const CallbackVH &) = default; void setValPtr(Value *P) { ValueHandleBase::operator=(P); diff --git a/llvm/include/llvm/IR/ValueMap.h b/llvm/include/llvm/IR/ValueMap.h index 4d00b63..ad518ac 100644 --- a/llvm/include/llvm/IR/ValueMap.h +++ b/llvm/include/llvm/IR/ValueMap.h @@ -214,8 +214,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 { +template +class ValueMapCallbackVH final : public CallbackVH { friend class ValueMap; friend struct DenseMapInfo; typedef ValueMap ValueMapT; diff --git a/llvm/lib/Analysis/CFLAliasAnalysis.cpp b/llvm/lib/Analysis/CFLAliasAnalysis.cpp index fe1c088..b76cbb4 100644 --- a/llvm/lib/Analysis/CFLAliasAnalysis.cpp +++ b/llvm/lib/Analysis/CFLAliasAnalysis.cpp @@ -153,15 +153,13 @@ struct FunctionInfo { struct CFLAliasAnalysis; -struct FunctionHandle : public CallbackVH { +struct FunctionHandle final : public CallbackVH { FunctionHandle(Function *Fn, CFLAliasAnalysis *CFLAA) : CallbackVH(Fn), CFLAA(CFLAA) { assert(Fn != nullptr); assert(CFLAA != nullptr); } - ~FunctionHandle() override {} - void deleted() override { removeSelfFromCache(); } void allUsesReplacedWith(Value *) override { removeSelfFromCache(); } diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp index 77e8535..f70833b 100644 --- a/llvm/lib/Analysis/LazyValueInfo.cpp +++ b/llvm/lib/Analysis/LazyValueInfo.cpp @@ -295,7 +295,7 @@ raw_ostream &operator<<(raw_ostream &OS, const LVILatticeVal &Val) { namespace { /// A callback value handle updates the cache when values are erased. class LazyValueInfoCache; - struct LVIValueHandle : public CallbackVH { + struct LVIValueHandle final : public CallbackVH { LazyValueInfoCache *Parent; LVIValueHandle(Value *V, LazyValueInfoCache *P) diff --git a/llvm/lib/CodeGen/MachineModuleInfo.cpp b/llvm/lib/CodeGen/MachineModuleInfo.cpp index 6a20624..4d5bd83 100644 --- a/llvm/lib/CodeGen/MachineModuleInfo.cpp +++ b/llvm/lib/CodeGen/MachineModuleInfo.cpp @@ -35,7 +35,7 @@ char MachineModuleInfo::ID = 0; MachineModuleInfoImpl::~MachineModuleInfoImpl() {} namespace llvm { -class MMIAddrLabelMapCallbackPtr : CallbackVH { +class MMIAddrLabelMapCallbackPtr final : CallbackVH { MMIAddrLabelMap *Map; public: MMIAddrLabelMapCallbackPtr() : Map(nullptr) {} diff --git a/llvm/lib/ExecutionEngine/ExecutionEngine.cpp b/llvm/lib/ExecutionEngine/ExecutionEngine.cpp index 77ea674..136a2d5 100644 --- a/llvm/lib/ExecutionEngine/ExecutionEngine.cpp +++ b/llvm/lib/ExecutionEngine/ExecutionEngine.cpp @@ -95,7 +95,7 @@ ExecutionEngine::~ExecutionEngine() { namespace { /// \brief Helper class which uses a value handler to automatically deletes the /// memory block when the GlobalVariable is destroyed. -class GVMemoryBlock : public CallbackVH { +class GVMemoryBlock final : public CallbackVH { GVMemoryBlock(const GlobalVariable *GV) : CallbackVH(const_cast(GV)) {} diff --git a/llvm/unittests/IR/ValueHandleTest.cpp b/llvm/unittests/IR/ValueHandleTest.cpp index e6eb7cb..e1d598b 100644 --- a/llvm/unittests/IR/ValueHandleTest.cpp +++ b/llvm/unittests/IR/ValueHandleTest.cpp @@ -29,7 +29,7 @@ protected: } }; -class ConcreteCallbackVH : public CallbackVH { +class ConcreteCallbackVH final : public CallbackVH { public: ConcreteCallbackVH(Value *V) : CallbackVH(V) {} }; @@ -235,7 +235,7 @@ TEST_F(ValueHandle, CallbackVH_Comparisons) { } TEST_F(ValueHandle, CallbackVH_CallbackOnDeletion) { - class RecordingVH : public CallbackVH { + class RecordingVH final : public CallbackVH { public: int DeletedCalls; int AURWCalls; @@ -261,7 +261,7 @@ TEST_F(ValueHandle, CallbackVH_CallbackOnDeletion) { } TEST_F(ValueHandle, CallbackVH_CallbackOnRAUW) { - class RecordingVH : public CallbackVH { + class RecordingVH final : public CallbackVH { public: int DeletedCalls; Value *AURWArgument; @@ -291,7 +291,7 @@ TEST_F(ValueHandle, CallbackVH_CallbackOnRAUW) { } TEST_F(ValueHandle, CallbackVH_DeletionCanRAUW) { - class RecoveringVH : public CallbackVH { + class RecoveringVH final : public CallbackVH { public: int DeletedCalls; Value *AURWArgument; @@ -339,7 +339,7 @@ TEST_F(ValueHandle, DestroyingOtherVHOnSameValueDoesntBreakIteration) { // arrangement of other VHs so that the bad behavior would be // triggered in whichever order callbacks run. - class DestroyingVH : public CallbackVH { + class DestroyingVH final : public CallbackVH { public: std::unique_ptr ToClear[2]; DestroyingVH(Value *V) { @@ -384,7 +384,7 @@ TEST_F(ValueHandle, AssertingVHCheckedLast) { // Value deletion, the CallbackVH should get a chance to do so // before the AssertingVHs assert. - class ClearingVH : public CallbackVH { + class ClearingVH final : public CallbackVH { public: AssertingVH *ToClear[2]; ClearingVH(Value *V, -- 2.7.4