[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-11-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG703038b35a86: [Sema] Fix volatile check when testing if a return object can be implicitly… (authored by nullptr.cpp, committed by arthur.j.odwyer).

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-11-10 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp added a comment. Thanks for all your suggestions! But I don't have commit access, can anyone help me commit this with "Yang Fan " ? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. In D88295#2365560 , @Quuxplusone wrote: >> So, how about we add another `CES` flag > > As the original author of `CES_AsIfByStdMove`, I am opposed to

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-31 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp updated this revision to Diff 302073. nullptr.cpp added a comment. move test to `class/class.init/class.copy.elision/p1.cpp` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295 Files:

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D88295#2365474 , @rsmith wrote: > ... where `X` has a volatile copy constructor and a volatile move > constructor, I think we should produce the warning suggesting use of > `std::move`. If I read this correctly, we'd

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > So, how about we add another `CES` flag As the original author of `CES_AsIfByStdMove`, I am opposed to any attempt to complicate this patch. My medium-term goal, now that P1155 has been adopted, is to //eliminate// the complexity

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D88295#2365474 , @rsmith wrote: > The other is whether implicitly move in a `co_return` statement. In that > case, I think the use of `CES_AsIfByStdMove` is a mistake, and we should be > using `CES_Default` there. I did

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D88295#2365312 , @Quuxplusone wrote: > I believe the organization of that directory implies that the path should be > `class/class.init/class.copy.elision/`, not `special/class.copy/`. > Otherwise LGTM, and I like this new test

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. I believe the organization of that directory implies that the path should be `class/class.init/class.copy.elision/`, not `special/class.copy/`. Otherwise LGTM, and I like this new test better than the old one! I still can't help re actually landing this patch, as I

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp updated this revision to Diff 301822. nullptr.cpp added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295 Files: clang/lib/Sema/SemaStmt.cpp

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-29 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp added a comment. In D88295#2358845 , @aaronpuchert wrote: > Could you perhaps integrate this into the existing test > `clang/test/CXX/special/class.copy/implicit-move.cpp` instead? > Whenever you have something that closely corresponds to

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-29 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp updated this revision to Diff 301810. nullptr.cpp added a comment. make test more clearly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295 Files: clang/lib/Sema/SemaStmt.cpp

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/implicitly-movable.cpp:14 + +private: + A(const A &); aaronpuchert wrote: > Quuxplusone wrote: > > aaronpuchert wrote: > > > Is this testing what we want it to test? The private functions are

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/implicitly-movable.cpp:14 + +private: + A(const A &); aaronpuchert wrote: > Is this testing what we want it to test? The private functions are just not > part of the overload set, right? > > I

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/implicitly-movable.cpp:14 + +private: + A(const A &); aaronpuchert wrote: > Is this testing what we want it to test? The private functions are just not > part of the overload set, right? > > I

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Can't really add anything to the discussion between @Quuxplusone and the author, just a few comments about the test. Comment at: clang/test/SemaCXX/implicitly-movable.cpp:1 +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify %s

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-28 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp added a comment. Ping I don't have commit access, can anyone help me commit this with "Yang Fan " ? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-15 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp added a comment. Ping! I don't have commit access, can anyone help me commit this with "Yang Fan " ? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-09-26 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp added a comment. - C++11: > [class.copy]p32: > When **the criteria for elision of a copy operation **are met or would be met > save for the fact that the source object is a function parameter, and the > object to be copied is designated by an lvalue, overload resolution to select

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-09-26 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp updated this revision to Diff 294485. nullptr.cpp added a comment. Add test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88295/new/ https://reviews.llvm.org/D88295 Files: clang/lib/Sema/SemaStmt.cpp

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-09-25 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. (This patch was split out from D88220 at my request.) @nullptr.cpp, please add the regression test from https://godbolt.org/z/5EfK99 . After a test is added, this patch LGTM (but will need approval also from someone else). Your

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-09-25 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp created this revision. nullptr.cpp added reviewers: Quuxplusone, rsmith, erik.pilkington. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. nullptr.cpp requested review of this revision. In C++11 standard, to become implicitly movable, the expression in