[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

Hubert Tong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 19 22:19:20 PDT 2022


hubert.reinterpretcast added inline comments.


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:17-22
+constexpr int g() { // expected-error {{constexpr function never produces a constant expression}}
+  goto test;        // expected-note {{subexpression not valid in a constant expression}} \
+           // expected-warning {{use of this statement in a constexpr function is incompatible with C++ standards before C++2b}}
+test:
+  return 0;
+}
----------------
Still trying to make the choice of tests consistent between the different cases (static local variable, goto statement, etc.) and consistent within files...

The test that is here (function will unconditionally evaluate) seems to belong in `constant-expression-cxx2b.cpp`... or at least that is where the equivalent tests for the static local variable case, etc. is (and correctly, I think).

The test that should be here is the "conditionally will evaluate" case where the function is not called. That is, this file has tests that check that the warnings are produced purely from the definition being present.


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:32-34
+constexpr void non_literal() { // expected-error {{constexpr function never produces a constant expression}}
+  NonLiteral n;                // expected-note {{non-literal type 'NonLiteral' cannot be used in a constant expression}}
+}
----------------
For the reason above, I think this should be in `constant-expression-cxx2b.cpp` instead.


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:1-3
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++14-extensions -Werror=c++20-extensions -Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++14 -DCXX14 -Werror=c++20-extensions -Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++20 -DCXX14 -DCXX20 -Werror=c++2b-extensions %s
----------------
hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > The use `-Werror` hides potential issues where the error is categorically produced (instead of under the warning group).
> > > 
> > > Considering that there is a relevant design consistency issue of exactly that sort right now that this test could have highlighted (but didn't), a change to stop using `-Werror` may be prudent.
> > This seems inconsistent with how that file is currently structured though
> I meant to change the entire file to check for warnings instead of errors. I guess that really should be a separate patch.
I guess the change will cause the "never produces a constant expression" warning unless if that is suppressed with `-Wno-invalid-constexpr`.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:95
+} // namespace eval_goto
+
+int test_in_lambdas() {
----------------
I see no tests in the entire patch where a labelled statement is evaluated successfully in a constant expression evaluation. This file will be where such a test should be.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:96-123
+int test_in_lambdas() {
+  auto a = [] constexpr {    // expected-error{{constexpr function never produces a constant expression}}
+    static const int m = 32; // expected-note {{control flows through the declaration of a static variable}} \
+                                 // expected-warning {{incompatible with C++ standards before C++2b}}
+    return m;
+  };
+
----------------
I think it would be more meaningful to have the explicitly `constexpr` lambda tests in the file (see comment on the later code below) that also runs under C++20 mode. The tests should be structured to demonstrate that the explicitly `constexpr` lambdas are usable in constant expression evaluation even in older modes.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:125-154
+int test_lambdas_implicitly_constexpr() {
+  auto a = [] {
+    static const int m = 32;
+    return m;
+  };
+
+  auto b = [](int n) {
----------------
It is quite important that all of the cases (label, `goto`, `static`, `thread_local`, and non-literal) is tested in lambdas not explicitly `constexpr` and run in C++20 mode too. The focus should be that:

  - the cases that are implicitly constexpr only in C++2b are not usable in constant expression evaluation in C++20 mode, and
  - they //are// usable in constant expression evaluation in C++2b mode.

I suggest a separate file for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400



More information about the cfe-commits mailing list