[PATCH] D111400: [Clang] Implement P2242R3

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:5128-5130
+          if (!CheckLocalVariableDeclaration(Info, VD)) {
+            return ESR_Failed;
+          }
----------------



================
Comment at: clang/lib/AST/ExprConstant.cpp:5174-5177
+      const VarDecl *VD = dyn_cast_or_null<VarDecl>(D);
+      if (VD && !CheckLocalVariableDeclaration(Info, VD)) {
+        return ESR_Failed;
+      }
----------------



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2130
+  case Stmt::GotoStmtClass:
+    if (!Cxx2bLoc.isValid())
+      Cxx2bLoc = S->getBeginLoc();
----------------



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2132-2136
+    for (Stmt *SubStmt : S->children())
+      if (SubStmt &&
+          !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmts,
+                                      Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
         return false;
----------------



================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp:1
-// RUN: %clang_cc1 -std=c++2a -verify %s
 
----------------
I think we want to keep testing the C++20 behavior and want new tests for the C++2b behavior. We still need to be sure we're correct in C++20 mode given the differences (even in extensions because `-pedantic-errors` is a thing). So I'd recommend splitting this test into two or using an additional RUN line.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx14.cpp:47
 static_assert(g(2) == 42, "");
-constexpr int h(int n) {
-  static const int m = n; // expected-error {{static variable not permitted in a constexpr function}}
----------------
I think we should keep this test coverage by adding `-Werror=c++2b-extensions` to the RUN line for C++2b.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx14.cpp:61
+} // expected-warning {{non-void}} \
+  //expected-note 2{{control reached end of constexpr function}}
+
----------------



================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:8-9
+}
+constexpr int g(int n) {        //  expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-warning {{definition of a thread_local variable in a constexpr function is incompatible with C++ standards before C++2b}} \
+                                // expected-note {{control flows through the declaration of a thread_local variable}}
----------------



================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:60-61
+
+constexpr int ke = k_evaluated(1); //expected-error {{constexpr variable 'ke' must be initialized by a constant expression}} \
+                                   //expected-note {{in call}}
+
----------------
This error seems suspect to me. If we made flowing through a thread_local into an extension (line 54), then this code should be accepted. However, I think we're getting the error because the constant expression evaluator produces the note on line 55 and that usually will cause the evaluator to claim it wasn't a constant expression.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:77
+int a = f(0);
+constexpr int b = f(0); //expected-error {{must be initialized by a constant expression}} \
+                        // expected-note {{in call to 'f(0)'}}
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400