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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to