[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 6 11:31:01 PDT 2021


Quuxplusone added a comment.

There's unlikely to be any further substantive comments from me, and this basically LGTM (or, in the places it looks bad, it's just the pre-existing awfulness of how the Clang test suite is organized).
I think you need to either get someone's attention to approve this, or approve it yourself. :)



================
Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:393-406
 // Proposed DR: copy-elision doesn't trigger lifetime extension.
-// expected-warning at +1 2{{temporary whose address is used as value of local variable 'b5' will be destroyed at the end of the full-expression}}
+// cxx11-warning at +1 2{{temporary whose address is used as value of local variable 'b5' will be destroyed at the end of the full-expression}}
 constexpr B b5 = B{ {0}, {0} }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
 
 namespace NestedNonStatic {
   // Proposed DR: for a reference constant expression to refer to a static
   // storage duration temporary, that temporary must itself be initialized
----------------
I wonder if you ought to just leave this test alone. It's got `cxx11` in the name of the test, and it does a lot of things that are deprecated in 20/2b which you're having to work around.
No strong opinion, but mainly because I haven't looked closely.
If your upcoming NRVO patches actually have some effect on constexpr evaluation, then perhaps it's time to add a //separate// `constant-expression-cxx20.cpp`. Or, split out the NRVO-specific parts of this one into a smaller simpler `constexpr-copy-elision.cpp` which runs in all modes, but leave the bulk of this file alone and C++11-only. WDYT?

Ooh, same comment about `constant-expression-cxx(1y->14).cpp`. I do see that as evidence in favor of adding a `constant-expression-cxx20.cpp`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99225/new/

https://reviews.llvm.org/D99225



More information about the cfe-commits mailing list