[PATCH] D105127: Implement P1401R5

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith 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:
> 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.


================
Comment at: clang/www/cxx_status.html:1299
       <td><a href="https://wg21.link/P1401R5">P1401R5</a></td>
-      <td class="none" align="center">No</td>
+      <td class="full" align="center">Clang 13</td>
     </tr>
----------------
cor3ntin wrote:
> rsmith wrote:
> > This should be class `unreleased` (yellow) for now so that people can easily tell what's in the most recent Clang release versus what's implemented but not released; we convert all the `class="unreleased"` to `class="full"` when we cut a release.
> Would you prefer I mark it partial for the explicit bool case?
If you're not planning on working on `explicit(bool)` yourself, then I think marking it as "partial" might be useful as a reminder that we should go back and look at that paper again. Otherwise, I have no strong preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127