[PATCH] D111400: [Clang] Implement P2242R3

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:16
+}
+
+constexpr int f(int x) {
----------------
Add a `NonLiteral` case and a case with a labelled statement and no `goto`?


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:37-38
+int x = f(0);
+constexpr int y = f(0); //expected-error {{must be initialized by a constant expression}} \
+                        // expected-note {{in call to 'f(0)'}}
+
----------------
I'm mildly confused over why the error case for the constexpr evaluation appears in this file (which is about the structure of the body) and not in `constant-expression-cxx2b.cpp` (which is about constant expression evaluation).


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:194-201
+  static constexpr int n = 123;
+#ifndef CXX2b
+  // expected-error at -2 {{definition of a static variable in a constexpr function is a C++2b extension}}
+#endif
+#if !defined(CXX14)
+  // expected-error at -5 {{variable declaration in a constexpr function is a C++14 extension}}
+#endif
----------------
The return statement needs to happen before the declaration of the static variable.


================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:353
+#ifndef CXX14
+  // expected-error at -5 {{C++14}}
+#endif
----------------
The abbreviated message snippet might be easy to understand if someone was looking at the diff from this patch, but it would likely be less so if they were just looking at the code later.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:3-7
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; //  expected-note {{declared here}}  \
+                          //  expected-warning {{definition of a static variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;               //  expected-note {{initializer of 'm' is not a constant expression}}
+}
----------------
Same comment as the `thread_local` case below. Also, the expected error is missing even if the function is called in a context requiring constant evaluation.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:8-12
+constexpr int g(int n) {        //  expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-note {{declared here}} \
+                                //  expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}}
+  return m;                     //  expected-note {{initializer of 'm' is not a constant expression}}
+}
----------------
The implementation generates an error because the `return` is `m` and not something like `&m - &m`.

The wording has:

  - a control flow that passes through a declaration of a variable with static or thread storage duration

as one of the things that cannot be evaluated for a constant expression.

The patch does not generate an error on the `&m - &m` version of the test.


================
Comment at: clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp:118-122
-auto NCL = [](int i) { static int j; return j; }; //expected-note{{declared here}}
-constexpr int (*fp5)(int) = NCL;
-constexpr int I =  //expected-error{{must be initialized by a constant expression}}
-                  fp5(5); //expected-note{{non-constexpr function}} 
-
----------------
Patch does not change the behaviour of this test anymore (and this file does not have a C++2b `RUN` line). Should keep the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400