cor3ntin added inline comments.
================
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">;
----------------
mizvekov wrote:
> Looks a bit easier to parse the english there.
I would rather not change that, to remain consistent with existing diagnostics
involving `constexpr if`
But I agree it might be good to change them all
================
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
----------------
mizvekov wrote:
> 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.
Thanks, much better, I did not know it was an option
================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:43
namespace ccce {
+struct S {
+} s;
----------------
mizvekov wrote:
> This is not consistently indented.
Unfortunately, it was put there by clang-format, should I move it manually?
================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:71-72
+
+ constexpr S constexprS;
+ if constexpr (s) {
+ }
----------------
mizvekov wrote:
> Hmm what is this testing exactly? That `constexprS` is not getting confused
> with `s`?
Indeed, will fix
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
