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

Soumi Manna via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 19 16:09:39 PDT 2023


Manna marked 2 inline comments as done.
Manna added inline comments.


================
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;
----------------
aaronpuchert wrote:
> 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.
Thank you @aaronpuchert for reviews and feedbacks. 

I have dropped the comment.


================
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;
----------------
aaronpuchert wrote:
> 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?
Yup, Sounds reasonable to me. Removed


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

https://reviews.llvm.org/D150931



More information about the cfe-commits mailing list