hubert.reinterpretcast added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1904-1906
+ if (!SemaRef.LangOpts.CPlusPlus2b &&
+ CheckLiteralType(SemaRef, Kind, VD->getLocation(), VD->getType(),
diag::err_constexpr_local_var_non_literal_type,
----------------
This seems to unconditionally error in pre-C++2b modes. For consistency, this
should be a `-Wc++2b-extensions` warning.
================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp:1
-// RUN: %clang_cc1 -std=c++2a -verify %s
----------------
aaron.ballman wrote:
> 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.
@cor3ntin, I don't think Aaron's comment has been addressed. I think keeping
`-std=c++2a` and adding `-Wc++2b-extensions` would be appropriate. I know there
are tests elsewhere that also focus on paragraph 3, but this test has coverage
specifically for destructors.
================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:38
+ if (!b)
+ NonLiteral n;
+}
----------------
For consistency, this should warn (under `-Wpre-c++2b-compat`).
================
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
----------------
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.
================
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}}
----------------
aaron.ballman wrote:
> I think we should keep this test coverage by adding
> `-Werror=c++2b-extensions` to the RUN line for C++2b.
@aaron.ballman, the tests have been restored; however, they test the extension
behaviour now. The `-Werror=c++2b-extensions` version of the tests are covered
by the tests elsewhere that focus on [dcl.constexpr] paragraph 3.
================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:3-13
+constexpr int f(int n) { // expected-error {{constexpr function never
produces a constant expression}}
+ static const int m = n; // expected-warning {{definition of a static
variable in a constexpr function is incompatible with C++ standards before
C++2b}} \
+ // expected-note {{control flows through the
declaration of a static variable}}
+ return m;
+}
+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}} \
----------------
Suggest to remove the `return m;` versions of the tests. They are redundant for
the purpose of verifying that "control flows through [ ... ]" is enough for the
function to never produce a constant expression.
================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:30
+ return m;
+}
+
----------------
Call `j`. Just make it go though the early return.
================
Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:38
+ return m;
+}
+
----------------
Same comment as for `j` above (but for `k`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111400/new/
https://reviews.llvm.org/D111400
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
