mizvekov added a comment.
Thanks!
Just some nits and some minor points on the tests.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491
+def err_constexpr_if_condition_expression_is_not_constant : Error<
+ "constexpr if condition is not a constant expression convertible to bool">;
def err_static_assert_failed : Error<"static_assert failed%select{ %1|}0">;
----------------
Looks a bit easier to parse the english there.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3922
//
// FIXME: Return this value to the caller so they don't need to recompute it.
+
----------------
This FIXME got pushed out of the thing it used to refer to.
================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:1-4
// RUN: %clang_cc1 -std=c++1z -verify %s
// RUN: %clang_cc1 -std=c++1z -verify %s -DUNDEFINED
+// RUN: %clang_cc1 -std=c++2b -verify %s
+// RUN: %clang_cc1 -std=c++2b -verify %s -DUNDEFINED
----------------
I think attaching the expected diagnostics to the lines that generate them
looks clearer and is easier to maintain.
The problem is that it seems you are having to work around that some of these
errors are conditional on the std version, and it would indeed be quite a bite
noisier with the preprocessor there, but there is a simple solution: with the
suggested edit above, you can do things like:
````
// cxx17-error {{value of type 'ccce::S' is not implicitly convertible to
'bool'}}
// cxx2b-error {{value of type 'ccce::S' is not contextually convertible to
'bool'}}
````
But you can still do `expected-note {{cannot be used in a constant
expression}}` in order to expect diagnostics for all versions.
================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:43
namespace ccce {
+struct S {
+} s;
----------------
This is not consistently indented.
================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:71-72
+
+ constexpr S constexprS;
+ if constexpr (s) {
+ }
----------------
Hmm what is this testing exactly? That `constexprS` is not getting confused
with `s`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105127/new/
https://reviews.llvm.org/D105127
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
