[NFC][Clang][Sema] Apply Rule of Three to various classes

[shafik]

Static analysis flagged a case where we had defined the destructor but not copy ctor or copy assignment, which violates the Rule of Three. I applied this to all the cases I found. With the exception to destructors with empty bodies. These should be defaulted but that should be done seperately.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

Static analysis flagged a case where we had defined the destructor but not copy ctor or copy assignment, which violates the Rule of Three. I applied this to all the cases I found. With the exception to destructors with empty bodies. These should be defaulted but that should be done seperately.

---

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

1 Files Affected:

• (modified) clang/include/clang/Sema/Sema.h (+47)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c9ad6860dc625..bebbe87e70c20 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1294,6 +1294,8 @@ class Sema final : public SemaBase {
     }
 
     ~CompoundScopeRAII() { S.ActOnFinishOfCompoundStmt(); }
+    CompoundScopeRAII(const CompoundScopeRAII &) = delete;
+    CompoundScopeRAII &operator=(const CompoundScopeRAII &) = delete;
 
   private:
     Sema &S;
@@ -2065,6 +2067,9 @@ class Sema final : public SemaBase {
   public:
     PragmaStackSentinelRAII(Sema &S, StringRef SlotLabel, bool ShouldAct);
     ~PragmaStackSentinelRAII();
+    PragmaStackSentinelRAII(const PragmaStackSentinelRAII &) = delete;
+    PragmaStackSentinelRAII &
+    operator=(const PragmaStackSentinelRAII &) = delete;
 
   private:
     Sema &S;
@@ -3501,6 +3506,8 @@ class Sema final : public SemaBase {
     }
 
     ~ContextRAII() { pop(); }
+    ContextRAII(const ContextRAII &) = delete;
+    ContextRAII &operator=(const ContextRAII &) = delete;
   };
 
   void DiagnoseInvalidJumps(Stmt *Body);
@@ -8449,6 +8456,8 @@ class Sema final : public SemaBase {
                      bool Enabled = true);
 
     ~CXXThisScopeRAII();
+    CXXThisScopeRAII(const CXXThisScopeRAII &) = delete;
+    CXXThisScopeRAII &operator=(const CXXThisScopeRAII &) = delete;
   };
 
   /// Make sure the value of 'this' is actually available in the current
@@ -10037,6 +10046,8 @@ class Sema final : public SemaBase {
       S.DeferDiags = SavedDeferDiags || DeferDiags;
     }
     ~DeferDiagsRAII() { S.DeferDiags = SavedDeferDiags; }
+    DeferDiagsRAII(const DeferDiagsRAII &) = delete;
+    DeferDiagsRAII &operator=(const DeferDiagsRAII &) = delete;
   };
 
   /// Flag indicating if Sema is building a recovery call expression.
@@ -11322,6 +11333,8 @@ class Sema final : public SemaBase {
       S.FpPragmaStack.Stack.clear();
     }
     ~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }
+    FpPragmaStackSaveRAII(const FpPragmaStackSaveRAII &) = delete;
+    FpPragmaStackSaveRAII &operator=(const FpPragmaStackSaveRAII &) = delete;
 
   private:
     Sema &S;
@@ -12421,6 +12434,8 @@ class Sema final : public SemaBase {
   protected:
     Sema &S;
     ~SFINAEContextBase() { S.CurrentSFINAEContext = Prev; }
+    SFINAEContextBase(const SFINAEContextBase &) = delete;
+    SFINAEContextBase &operator=(const SFINAEContextBase &) = delete;
 
   private:
     SFINAETrap *Prev;
@@ -12492,6 +12507,9 @@ class Sema final : public SemaBase {
     ~TentativeAnalysisScope() {
       SemaRef.DisableTypoCorrection = PrevDisableTypoCorrection;
     }
+
+    TentativeAnalysisScope(const TentativeAnalysisScope &) = delete;
+    TentativeAnalysisScope &operator=(const TentativeAnalysisScope &) = delete;
   };
 
   /// For each declaration that involved template argument deduction, the
@@ -13065,6 +13083,9 @@ class Sema final : public SemaBase {
       }
     }
 
+    RecursiveInstGuard(const RecursiveInstGuard &) = delete;
+    RecursiveInstGuard &operator=(const RecursiveInstGuard &) = delete;
+
     operator bool() const { return Key.getOpaqueValue() == nullptr; }
 
   private:
@@ -13538,6 +13559,10 @@ class Sema final : public SemaBase {
       S.PopExpressionEvaluationContext();
       S.PopFunctionScopeInfo();
     }
+
+    SynthesizedFunctionScope(const SynthesizedFunctionScope &) = delete;
+    SynthesizedFunctionScope &
+    operator=(const SynthesizedFunctionScope &) = delete;
   };
 
   /// List of active code synthesis contexts.
@@ -13615,6 +13640,8 @@ class Sema final : public SemaBase {
           OldSubstIndex(std::exchange(Self.ArgPackSubstIndex, NewSubstIndex)) {}
 
     ~ArgPackSubstIndexRAII() { Self.ArgPackSubstIndex = OldSubstIndex; }
+    ArgPackSubstIndexRAII(const ArgPackSubstIndexRAII &) = delete;
+    ArgPackSubstIndexRAII &operator=(const ArgPackSubstIndexRAII &) = delete;
   };
 
   bool pushCodeSynthesisContext(CodeSynthesisContext Ctx);
@@ -13745,6 +13772,8 @@ class Sema final : public SemaBase {
       TI.setEvaluateConstraints(false);
     }
     ~ConstraintEvalRAII() { TI.setEvaluateConstraints(OldValue); }
+    ConstraintEvalRAII(const ConstraintEvalRAII &) = delete;
+    ConstraintEvalRAII &operator=(const ConstraintEvalRAII &) = delete;
   };
 
   // Must be used instead of SubstExpr at 'constraint checking' time.
@@ -13993,6 +14022,10 @@ class Sema final : public SemaBase {
           S.PendingLocalImplicitInstantiations);
     }
 
+    LocalEagerInstantiationScope(const LocalEagerInstantiationScope &) = delete;
+    LocalEagerInstantiationScope &
+    operator=(const LocalEagerInstantiationScope &) = delete;
+
   private:
     Sema &S;
     bool AtEndOfTU;
@@ -14066,6 +14099,11 @@ class Sema final : public SemaBase {
       }
     }
 
+    GlobalEagerInstantiationScope(const GlobalEagerInstantiationScope &) =
+        delete;
+    GlobalEagerInstantiationScope &
+    operator=(const GlobalEagerInstantiationScope &) = delete;
+
   private:
     Sema &S;
     bool Enabled;
@@ -14301,6 +14339,11 @@ class Sema final : public SemaBase {
       swapSavedState();
     }
 
+    SavePendingParsedClassStateRAII(const SavePendingParsedClassStateRAII &) =
+        delete;
+    SavePendingParsedClassStateRAII &
+    operator=(const SavePendingParsedClassStateRAII &) = delete;
+
   private:
     Sema &S;
     decltype(DelayedOverridingExceptionSpecChecks)
@@ -14800,6 +14843,10 @@ class Sema final : public SemaBase {
     ~SatisfactionStackResetRAII() {
       SemaRef.SwapSatisfactionStack(BackupSatisfactionStack);
     }
+
+    SatisfactionStackResetRAII(const SatisfactionStackResetRAII &) = delete;
+    SatisfactionStackResetRAII &
+    operator=(const SatisfactionStackResetRAII &) = delete;
   };
 
   void SwapSatisfactionStack(

[AaronBallman]

The changes LGTM as far as they go, but we don't have any coding standard requirement for a rule of three (five, zero, etc) and these changes are a noop. If we're going to have tools which enforce a rule like this, should we change the coding standard to mention following the rule?

[shafik]

The changes LGTM as far as they go, but we don't have any coding standard requirement for a rule of three (five, zero, etc) and these changes are a noop. If we're going to have tools which enforce a rule like this, should we change the coding standard to mention following the rule?

Yeah that might be a good idea. It is a pretty common sense rule, if you have a resource/count/flag/etc you are specially managing on construction/destruction you should either implement copy or explicitly delete them.