[PATCH] D105127: Implement P1401R5

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 5 inline comments as done.
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">;
----------------
aaron.ballman wrote:
> rsmith wrote:
> > mizvekov wrote:
> > > cor3ntin wrote:
> > > > 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
> > > I see, yeah agreed.
> > Would it be reasonable to drop the "convertible to bool" part here? We know the problem is that the (converted) expression is not a constant expression, not that the expression can't be converted to bool, because we handle the conversion to bool separately before we get to this diagnostic; I think the diagnostic would be clearer if it didn't mention the conversion.
> +1 -- I think the "convertible to bool" may trip some users up on how to correct the issue.
Sorry, I missed that feedback earlier, fixed now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127