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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
