[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

aaron.ballman added a comment. In D99005#2860494 , @mizvekov wrote: >> I'd prefer to keep existing code working without requiring users to enable a >> flag, if at all possible (esp because I don't expect the flag to be needed >> forever). That said, I st

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. @aaron.ballman I created a DR for the option of temporarily disabling the effects of this change under "-fms-compatibility". https://reviews.llvm.org/D105518 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https:

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. In D99005#2860598 , @Quuxplusone wrote: > I have no strong/well-informed opinions here. I have an idea to offer, //if// > there is precedent for it. My idea is, keep it as an error if the rvalue > resolution finds nothing, but (

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone added a comment. > I'd like to give some time for other stakeholders to give their opinion if > this is not too urgent, specially @Quuxplusone and @rsmith. I have no strong/well-informed opinions here. I have an idea to offer, //if// there is precedent for it. My idea is, keep it as

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. In D99005#2860021 , @aaron.ballman wrote: > I would argue it gives the committee valuable implementation experience > feedback: the change breaks at least one popular system header. But we already knew that :-) Reverting the ch

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

aaron.ballman added a comment. In D99005#2859886 , @mizvekov wrote: > In D99005#2859476 , @aaron.ballman > wrote: > >> https://godbolt.org/z/dvEbv7GKo >> >> I'm not certain if this is as expected of an issue, thoug

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. In D99005#2859476 , @aaron.ballman wrote: > https://godbolt.org/z/dvEbv7GKo > > I'm not certain if this is as expected of an issue, though. In the original > example, `C` carried state that was set up after initialization but wa

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

aaron.ballman added a comment. In D99005#2843201 , @mizvekov wrote: > Thank you again @sberg. > > I have talked to the MSVC STL maintainers. They would be open to a pull > request for fixing this. > The repo can be found at https://github.com/microsoft/ST

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

aaron.ballman added a comment. In D99005#2844365 , @mizvekov wrote: > But is this example a reduction from a real world code base? > The committee wants feedback and we are interested how hard you believe this > change affects you. This is an example tha

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

zahiraam added a comment. In D99005#2844365 , @mizvekov wrote: > In D99005#2844332 , @zahiraam wrote: > >> This change has made this snippet fail. >> https://godbolt.org/z/3ehK784hY Pass >> https://godbolt.org/z/9q4

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. In D99005#2844332 , @zahiraam wrote: > This change has made this snippet fail. > https://godbolt.org/z/3ehK784hY Pass > https://godbolt.org/z/9q48WvsP7 fails. Hello! That is expected breakage from the changes proposed in P2266 <

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

zahiraam added a comment. This change has made this snippet fail. https://godbolt.org/z/3ehK784hY Pass https://godbolt.org/z/9q48WvsP7 fails. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. In D99005#2843201 , @mizvekov wrote: > I have talked to the MSVC STL maintainers. They would be open to a pull > request for fixing this. > The repo can be found at https://github.com/microsoft/STL > Be my guest if you want to submi

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. Thank you again @sberg. I have talked to the MSVC STL maintainers. They would be open to a pull request for fixing this. The repo can be found at https://github.com/microsoft/STL Be my guest if you want to submit that fix yourself. If not, I can probably do it in the ne

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. And, again for the record, with a build of LibreOffice with clang-cl (and `-Xclang -std=c++2b`) on Windows, at least against the C++ standard library from Visual Studio 2019 version 16.20.2, I ran into two issues in the standard library itself, when using `std::getline` a

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGced6b204d18e: [clang] Implement P2266 Simpler implicit move (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 352985. mizvekov added a comment. fix ctidy warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 352821. mizvekov added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp clang/lib/Sema

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 352818. mizvekov added a comment. - Change block capture NRVO semantics to match P2266 in C++2b mode. - Update cxx_status. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/n

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. For the record, the one other breakage of a LibreOffice build with this is the handful of places that needed https://git.libreoffice.org/core/+/433ab39b2175bdadb4916373cd2dc8e1aabc08a5%5E%21 "Adapt implicit OString return value construction to C++23 P2266R1": In a nutsh

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone added a comment. @sberg: Thanks for the example! @mizvekov's comments are correct, but I wanted to put some more comments here for posterity. (I'll also try to find a place for this real-world example in the next revision of p2266

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. In D99005#2818606 , @sberg wrote: > (In a build prior to > https://reviews.llvm.org/rGc60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 "Revert > '[clang] NRVO: Improvements and handling of more cases.'") I see the > following (reduced

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. (In a build prior to https://reviews.llvm.org/rGc60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 "Revert '[clang] NRVO: Improvements and handling of more cases.'") I see the following (reduced from https://git.libreoffice.org/core/+/649313625b94e6b879848fc19b607b74375100bf/o3tl/

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf2063178218: [clang] Implement P2266 Simpler implicit move (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcbd0054b9eb1: [clang] Implement P2266 Simpler implicit move (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a comment. Actually we can land it now. @rsmith gave me the go ahead in private. The windows pipeline passes. The latest debian build failure is some random fortran thing, so we are good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. IMHO we should just land this already. (It's been sitting without major update since mid-April.) It is intended to affect only `-std=c++2b` mode, and implements a paper (full disclo

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 351274. mizvekov added a comment. rerun pipeline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaCorouti

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 338262. mizvekov added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaCoroutine.cpp

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 337009. mizvekov added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaCoroutine.cpp

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 335604. mizvekov added a comment. - test/SemaCXX/coroutine-rvo: Make MoveOnly trivial, since the temporary is not created anymore and there is no special reason to test a non-trivial type here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 335542. mizvekov added a comment. - Rebased on top of D99696 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1059 +else if (auto *F = dyn_cast(DC)) + goto unimplemented; // FIXME: get the return type here somehow... +else rjmccall wrote: > mizvekov wrote: > > rjmccall

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

rjmccall added a subscriber: fhahn. rjmccall added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1059 +else if (auto *F = dyn_cast(DC)) + goto unimplemented; // FIXME: get the return type here somehow... +else mizvekov w

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1059 +else if (auto *F = dyn_cast(DC)) + goto unimplemented; // FIXME: get the return type here somehow... +else rjmccall wrote: > mizvekov wrote: > > @rjmccal

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 333925. mizvekov added a comment. Small update making some invariants more clear, with some simplifications. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: c

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1059 +else if (auto *F = dyn_cast(DC)) + goto unimplemented; // FIXME: get the return type here somehow... +else mizvekov wrote: > @rjmccall Hello! Do you have

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 333756. mizvekov added a comment. some further simplifications Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sem

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 333749. mizvekov added a comment. small fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp clang/

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov added a subscriber: rjmccall. mizvekov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1059 +else if (auto *F = dyn_cast(DC)) + goto unimplemented; // FIXME: get the return type here somehow... +else @rjmcca

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

mizvekov updated this revision to Diff 333745. mizvekov added a comment. So it turns out there was a pre-existing bug where ObjC++ blocks with dependent return type were not getting copy elision. My latest refactoring accidentally fixed this in the return statement sema action, but unfortunately