This is an archive of the discontinued LLVM Phabricator instance.

Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl
ClosedPublic

Authored by gribozavr on May 21 2019, 4:40 AM.

Event Timeline

gribozavr created this revision.May 21 2019, 4:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 4:40 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ilya-biryukov accepted this revision.May 22 2019, 12:00 AM

LGTM

clang/include/clang/AST/DeclBase.h
374

NIT: move constructors and assignments won't be generated if copy is deleted, so this is redundant.
However, feel free to keep it if you want to be more explicit.

clang/include/clang/AST/Stmt.h
1057

NIT: Move the deleted declarations to the start of the class?
Or move the deleted declarations in other classes to the first public section?

(For consistency)

This revision is now accepted and ready to land.May 22 2019, 12:00 AM
gribozavr marked 4 inline comments as done.May 22 2019, 1:04 AM
gribozavr added inline comments.
clang/include/clang/AST/DeclBase.h
374

I feel like it is better to have the API explicitly spelled out.

clang/include/clang/AST/Stmt.h
1057

I put these new declarations close to the "primary" constructor.

LG either way, the comments were NITs.

clang/include/clang/AST/Stmt.h
1057

Why not move declarations inside Stmt to the primary constructor too?

gribozavr updated this revision to Diff 200684.May 22 2019, 2:56 AM
gribozavr marked 2 inline comments as done.

Addressed review comments

gribozavr marked 2 inline comments as done.May 22 2019, 2:56 AM
gribozavr added inline comments.
clang/include/clang/AST/Stmt.h
1057

Done.

This revision was automatically updated to reflect the committed changes.
gribozavr marked an inline comment as done.