[NFC][Clang] Fix Static Code Analysis Concerns with copy without assign

[Manna]

Summary

This patch adds missing assignment operator to the class which has user-defined copy constructor.

[aaronpuchert]

Can only comment on ThreadSafetyTIL.h. Thanks for addressing this, I agree that these expressions should not be assignable.

clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h

323–324

We can probably even drop the comment. Types in an inheritance hierarchy tend to be not assignable, and I don't see why it would ever be needed for what amounts to be AST nodes.

387–389

My understanding is that deleting copy assignment on the base class automatically deletes it for the derived classes, so I'd appreciate if we could drop this and the following additions. That's just noise in my view.

Also, most of these classes don't seem to have a user-declared copy constructor, or am I missing something?

[Manna]

I have addressed review comments.

[Manna]

clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h

323–324

Thank you @aaronpuchert for reviews and feedbacks.

I have dropped the comment.

387–389

Yup, Sounds reasonable to me. Removed

[Manna]

I have updated patch to resolve build errors.

[aaronpuchert]

Changes to ThreadSafetyTIL.h look good to me, thanks!

[Manna]

In D150931#4358903, @aaronpuchert wrote:

Changes to ThreadSafetyTIL.h look good to me, thanks!

Thank you @aaronpuchert for reviews!

[Manna]

ping @tahonermann

[tahonermann]

These changes look fine to me.

[Manna]

Thank you @tahonermann and @aaronpuchert for reviews and comments!

clang/include/clang/AST/TemplateBase.h

public:
 
   /// Construct an integral constant template argument with the same
   /// value as Other but a different type.
   TemplateArgument(const TemplateArgument &Other, QualType Type) {
     Integer = Other.Integer;
     Integer.Type = Type.getAsOpaquePtr();
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  TemplateArgument &operator=(const TemplateArgument &) = delete;
+
   /// Construct a template argument that is a template.
   ///
   /// This form of template argument is generally used for template template
   /// parameters. However, the template name could be a dependent template
   /// name that ends up being instantiated to a function template whose address
   /// is taken.
   ///
   /// \param Name The template name.

clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h

public:
 
   /// Set the basic block and instruction ID for this expression.
   void setID(BasicBlock *B, unsigned id) { Block = B; SExprID = id; }
 
 protected:
   SExpr(TIL_Opcode Op) : Opcode(Op) {}
   SExpr(const SExpr &E) : Opcode(E.Opcode), Flags(E.Flags) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  SExpr &operator=(const SExpr &) = delete;
+
   const TIL_Opcode Opcode;
   unsigned char Reserved = 0;
   unsigned short Flags = 0;
   unsigned SExprID = 0;
   BasicBlock *Block = nullptr;
 };
 
 // Contains various helper functions for SExprs.
Variable(SExpr *D, const ValueDecl *Cvd = nullptr)
     Flags = VK_Let;
   }
 
   Variable(const Variable &Vd, SExpr *D)  // rewrite constructor
       : SExpr(Vd), Name(Vd.Name), Definition(D), Cvdecl(Vd.Cvdecl) {
     Flags = Vd.kind();
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Variable &operator=(const Variable &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Variable; }
 
   /// Return the kind of variable (let, function param, or self)
   VariableKind kind() const { return static_cast<VariableKind>(Flags); }
 
   /// Return the name of the variable, if any.
   StringRef name() const { return Name; }
 
Function(Variable *Vd, SExpr *Bd)
     Vd->setKind(Variable::VK_Fun);
   }
 
   Function(const Function &F, Variable *Vd, SExpr *Bd) // rewrite constructor
       : SExpr(F), VarDecl(Vd), Body(Bd) {
     Vd->setKind(Variable::VK_Fun);
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Function &operator=(const Function &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Function; }
 
   Variable *variableDecl()  { return VarDecl; }
   const Variable *variableDecl() const { return VarDecl; }
 
   SExpr *body() { return Body; }
   const SExpr *body() const { return Body; }
 
public:
 
   SFunction(const SFunction &F, Variable *Vd, SExpr *B) // rewrite constructor
       : SExpr(F), VarDecl(Vd), Body(B) {
     assert(Vd->Definition == nullptr);
     Vd->setKind(Variable::VK_SFun);
     Vd->Definition = this;
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  SFunction &operator=(const SFunction &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_SFunction; }
 
   Variable *variableDecl() { return VarDecl; }
   const Variable *variableDecl() const { return VarDecl; }
 
   SExpr *body() { return Body; }
   const SExpr *body() const { return Body; }
 

 
 /// A block of code -- e.g. the body of a function.
 class Code : public SExpr {
 public:
   Code(SExpr *T, SExpr *B) : SExpr(COP_Code), ReturnType(T), Body(B) {}
   Code(const Code &C, SExpr *T, SExpr *B) // rewrite constructor
       : SExpr(C), ReturnType(T), Body(B) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Code &operator=(const Code &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Code; }
 
   SExpr *returnType() { return ReturnType; }
   const SExpr *returnType() const { return ReturnType; }
 
   SExpr *body() { return Body; }
   const SExpr *body() const { return Body; }
 

 
 /// A typed, writable location in memory
 class Field : public SExpr {
 public:
   Field(SExpr *R, SExpr *B) : SExpr(COP_Field), Range(R), Body(B) {}
   Field(const Field &C, SExpr *R, SExpr *B) // rewrite constructor
       : SExpr(C), Range(R), Body(B) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Field &operator=(const Field &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Field; }
 
   SExpr *range() { return Range; }
   const SExpr *range() const { return Range; }
 
   SExpr *body() { return Body; }
   const SExpr *body() const { return Body; }
 

 /// Once all parameters have been applied, Call can be used to invoke the
 /// function.
 class Apply : public SExpr {
 public:
   Apply(SExpr *F, SExpr *A) : SExpr(COP_Apply), Fun(F), Arg(A) {}
   Apply(const Apply &A, SExpr *F, SExpr *Ar)  // rewrite constructor
       : SExpr(A), Fun(F), Arg(Ar) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Apply &operator=(const Apply &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Apply; }
 
   SExpr *fun() { return Fun; }
   const SExpr *fun() const { return Fun; }
 
   SExpr *arg() { return Arg; }
   const SExpr *arg() const { return Arg; }
 

 
 /// Apply a self-argument to a self-applicable function.
 class SApply : public SExpr {
 public:
   SApply(SExpr *Sf, SExpr *A = nullptr) : SExpr(COP_SApply), Sfun(Sf), Arg(A) {}
   SApply(SApply &A, SExpr *Sf, SExpr *Ar = nullptr) // rewrite constructor
       : SExpr(A), Sfun(Sf), Arg(Ar) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  SApply &operator=(const SApply &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_SApply; }
 
   SExpr *sfun() { return Sfun; }
   const SExpr *sfun() const { return Sfun; }
 
   SExpr *arg() { return Arg ? Arg : Sfun; }
   const SExpr *arg() const { return Arg ? Arg : Sfun; }
 

 
 /// Call a function (after all arguments have been applied).
 class Call : public SExpr {
 public:
   Call(SExpr *T, const CallExpr *Ce = nullptr)
       : SExpr(COP_Call), Target(T), Cexpr(Ce) {}
   Call(const Call &C, SExpr *T) : SExpr(C), Target(T), Cexpr(C.Cexpr) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Call &operator=(const Call &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Call; }
 
   SExpr *target() { return Target; }
   const SExpr *target() const { return Target; }
 
   const CallExpr *clangCallExpr() const { return Cexpr; }
 
   template <class V>
public:
   enum AllocKind {
     AK_Stack,
     AK_Heap
   };
 
   Alloc(SExpr *D, AllocKind K) : SExpr(COP_Alloc), Dtype(D) { Flags = K; }
   Alloc(const Alloc &A, SExpr *Dt) : SExpr(A), Dtype(Dt) { Flags = A.kind(); }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Alloc &operator=(const Alloc &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Call; }
 
   AllocKind kind() const { return static_cast<AllocKind>(Flags); }
 
   SExpr *dataType() { return Dtype; }
   const SExpr *dataType() const { return Dtype; }
 
   template <class V>

 };
 
 /// Load a value from memory.
 class Load : public SExpr {
 public:
   Load(SExpr *P) : SExpr(COP_Load), Ptr(P) {}
   Load(const Load &L, SExpr *P) : SExpr(L), Ptr(P) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Load &operator=(const Load &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Load; }
 
   SExpr *pointer() { return Ptr; }
   const SExpr *pointer() const { return Ptr; }
 
   template <class V>
   typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx) {
     auto Np = Vs.traverse(Ptr, Vs.subExprCtx(Ctx));

 
 /// Store a value to memory.
 /// The destination is a pointer to a field, the source is the value to store.
 class Store : public SExpr {
 public:
   Store(SExpr *P, SExpr *V) : SExpr(COP_Store), Dest(P), Source(V) {}
   Store(const Store &S, SExpr *P, SExpr *V) : SExpr(S), Dest(P), Source(V) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Store &operator=(const Store &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Store; }
 
   SExpr *destination() { return Dest; }  // Address to store to
   const SExpr *destination() const { return Dest; }
 
   SExpr *source() { return Source; }     // Value to store
   const SExpr *source() const { return Source; }
 

 /// If p is a reference to an array, then p[i] is a reference to the i'th
 /// element of the array.
 class ArrayIndex : public SExpr {
 public:
   ArrayIndex(SExpr *A, SExpr *N) : SExpr(COP_ArrayIndex), Array(A), Index(N) {}
   ArrayIndex(const ArrayIndex &E, SExpr *A, SExpr *N)
       : SExpr(E), Array(A), Index(N) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  ArrayIndex &operator=(const ArrayIndex &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_ArrayIndex; }
 
   SExpr *array() { return Array; }
   const SExpr *array() const { return Array; }
 
   SExpr *index() { return Index; }
   const SExpr *index() const { return Index; }
 

 /// If p is a reference to an array, then p + n, where n is an integer, is
 /// a reference to a subarray.
 class ArrayAdd : public SExpr {
 public:
   ArrayAdd(SExpr *A, SExpr *N) : SExpr(COP_ArrayAdd), Array(A), Index(N) {}
   ArrayAdd(const ArrayAdd &E, SExpr *A, SExpr *N)
       : SExpr(E), Array(A), Index(N) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  ArrayAdd &operator=(const ArrayAdd &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_ArrayAdd; }
 
   SExpr *array() { return Array; }
   const SExpr *array() const { return Array; }
 
   SExpr *index() { return Index; }
   const SExpr *index() const { return Index; }
 

 class UnaryOp : public SExpr {
 public:
   UnaryOp(TIL_UnaryOpcode Op, SExpr *E) : SExpr(COP_UnaryOp), Expr0(E) {
     Flags = Op;
   }
 
   UnaryOp(const UnaryOp &U, SExpr *E) : SExpr(U), Expr0(E) { Flags = U.Flags; }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  UnaryOp &operator=(const UnaryOp &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_UnaryOp; }
 
   TIL_UnaryOpcode unaryOpcode() const {
     return static_cast<TIL_UnaryOpcode>(Flags);
   }
 
   SExpr *expr() { return Expr0; }
   const SExpr *expr() const { return Expr0; }
BinaryOp(TIL_BinaryOpcode Op, SExpr *E0, SExpr *E1)
     Flags = Op;
   }
 
   BinaryOp(const BinaryOp &B, SExpr *E0, SExpr *E1)
       : SExpr(B), Expr0(E0), Expr1(E1) {
     Flags = B.Flags;
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  BinaryOp &operator=(const BinaryOp &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_BinaryOp; }
 
   TIL_BinaryOpcode binaryOpcode() const {
     return static_cast<TIL_BinaryOpcode>(Flags);
   }
 
   SExpr *expr0() { return Expr0; }
   const SExpr *expr0() const { return Expr0; }

 /// Cast expressions.
 /// Cast expressions are essentially unary operations, but we treat them
 /// as a distinct AST node because they only change the type of the result.
 class Cast : public SExpr {
 public:
   Cast(TIL_CastOpcode Op, SExpr *E) : SExpr(COP_Cast), Expr0(E) { Flags = Op; }
   Cast(const Cast &C, SExpr *E) : SExpr(C), Expr0(E) { Flags = C.Flags; }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Cast &operator=(const Cast &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Cast; }
 
   TIL_CastOpcode castOpcode() const {
     return static_cast<TIL_CastOpcode>(Flags);
   }
 
   SExpr *expr() { return Expr0; }
   const SExpr *expr() const { return Expr0; }
enum Status {
     PH_SingleVal,    // Phi node has one distinct value, and can be eliminated
     PH_Incomplete    // Phi node is incomplete
   };
 
   Phi() : SExpr(COP_Phi) {}
   Phi(MemRegionRef A, unsigned Nvals) : SExpr(COP_Phi), Values(A, Nvals)  {}
   Phi(const Phi &P, ValArray &&Vs) : SExpr(P), Values(std::move(Vs)) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Phi &operator=(const Phi &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Phi; }
 
   const ValArray &values() const { return Values; }
   ValArray &values() { return Values; }
 
   Status status() const { return static_cast<Status>(Flags); }
   void setStatus(Status s) { Flags = s; }
 

 /// of the call.
 class Goto : public Terminator {
 public:
   Goto(BasicBlock *B, unsigned I)
       : Terminator(COP_Goto), TargetBlock(B), Index(I) {}
   Goto(const Goto &G, BasicBlock *B, unsigned I)
       : Terminator(COP_Goto), TargetBlock(B), Index(I) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Goto &operator=(const Goto &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Goto; }
 
   const BasicBlock *targetBlock() const { return TargetBlock; }
   BasicBlock *targetBlock() { return TargetBlock; }
 
   /// Returns the index into the
   unsigned index() const { return Index; }
 
public:
   }
 
   Branch(const Branch &Br, SExpr *C, BasicBlock *T, BasicBlock *E)
       : Terminator(Br), Condition(C) {
     Branches[0] = T;
     Branches[1] = E;
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Branch &operator=(const Branch &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Branch; }
 
   const SExpr *condition() const { return Condition; }
   SExpr *condition() { return Condition; }
 
   const BasicBlock *thenBlock() const { return Branches[0]; }
   BasicBlock *thenBlock() { return Branches[0]; }
 

 
 /// Return from the enclosing function, passing the return value to the caller.
 /// Only the exit block should end with a return statement.
 class Return : public Terminator {
 public:
   Return(SExpr* Rval) : Terminator(COP_Return), Retval(Rval) {}
   Return(const Return &R, SExpr* Rval) : Terminator(R), Retval(Rval) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Return &operator=(const Return &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Return; }
 
   /// Return an empty list.
   ArrayRef<BasicBlock *> successors() { return std::nullopt; }
 
   SExpr *returnValue() { return Retval; }
   const SExpr *returnValue() const { return Retval; }
 
public:
 
   explicit BasicBlock(MemRegionRef A)
       : SExpr(COP_BasicBlock), Arena(A), BlockID(0), Visited(false) {}
   BasicBlock(BasicBlock &B, MemRegionRef A, InstrArray &&As, InstrArray &&Is,
              Terminator *T)
       : SExpr(COP_BasicBlock), Arena(A), BlockID(0), Visited(false),
         Args(std::move(As)), Instrs(std::move(Is)), TermInstr(T) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  BasicBlock &operator=(const BasicBlock &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_BasicBlock; }
 
   /// Returns the block ID.  Every block has a unique ID in the CFG.
   int blockID() const { return BlockID; }
 
   /// Returns the number of predecessors.
   size_t numPredecessors() const { return Predecessors.size(); }
   size_t numSuccessors() const { return successors().size(); }
SCFG(MemRegionRef A, unsigned Nblocks)
     add(Exit);
   }
 
   SCFG(const SCFG &Cfg, BlockArray &&Ba) // steals memory from Ba
       : SExpr(COP_SCFG), Arena(Cfg.Arena), Blocks(std::move(Ba)) {
     // TODO: set entry and exit!
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  SCFG &operator=(const SCFG &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_SCFG; }
 
   /// Return true if this CFG is valid.
   bool valid() const { return Entry && Exit && Blocks.size() > 0; }
 
   /// Return true if this CFG has been normalized.
   /// After normalization, blocks are in topological order, and block and
   /// instruction IDs have been assigned.

 /// This is a pseduo-term; it will be lowered to a branch in a CFG.
 class IfThenElse : public SExpr {
 public:
   IfThenElse(SExpr *C, SExpr *T, SExpr *E)
       : SExpr(COP_IfThenElse), Condition(C), ThenExpr(T), ElseExpr(E) {}
   IfThenElse(const IfThenElse &I, SExpr *C, SExpr *T, SExpr *E)
       : SExpr(I), Condition(C), ThenExpr(T), ElseExpr(E) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  IfThenElse &operator=(const IfThenElse &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_IfThenElse; }
 
   SExpr *condition() { return Condition; }   // Address to store to
   const SExpr *condition() const { return Condition; }
 
   SExpr *thenExpr() { return ThenExpr; }     // Value to store
   const SExpr *thenExpr() const { return ThenExpr; }
 
public:
   Let(Variable *Vd, SExpr *Bd) : SExpr(COP_Let), VarDecl(Vd), Body(Bd) {
     Vd->setKind(Variable::VK_Let);
   }
 
   Let(const Let &L, Variable *Vd, SExpr *Bd) : SExpr(L), VarDecl(Vd), Body(Bd) {
     Vd->setKind(Variable::VK_Let);
   }
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Let &operator=(const Let &) = delete;
+
   static bool classof(const SExpr *E) { return E->opcode() == COP_Let; }
 
   Variable *variableDecl()  { return VarDecl; }
   const Variable *variableDecl() const { return VarDecl; }
 
   SExpr *body() { return Body; }
   const SExpr *body() const { return Body; }
 

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

 class CallEvent;
 
 template<typename T = CallEvent>
 class CallEventRef : public IntrusiveRefCntPtr<const T> {
 public:
   CallEventRef(const T *Call) : IntrusiveRefCntPtr<const T>(Call) {}
   CallEventRef(const CallEventRef &Orig) : IntrusiveRefCntPtr<const T>(Orig) {}
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  CallEventRef &operator=(const CallEventRef &) = delete;
+
   CallEventRef<T> cloneWithState(ProgramStateRef State) const {
     return this->get()->template cloneWithState<T>(State);
   }
 
   // Allow implicit conversions to a superclass type, since CallEventRef
   // behaves like a pointer-to-const.
   template <typename SuperT>
   operator CallEventRef<SuperT> () const {