[PATCH] D150931: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

Aaron Puchert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 19 13:52:49 PDT 2023


aaronpuchert added a comment.

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



================
Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:323-324
 
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  SExpr &operator=(const SExpr &) = delete;
----------------
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.


================
Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:387-389
+  // The copy assignment operator is defined as deleted pending further
+  // motivation.
+  Variable &operator=(const Variable &) = delete;
----------------
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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150931/new/

https://reviews.llvm.org/D150931



More information about the cfe-commits mailing list