mizvekov added inline comments.

================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3930
+  ExprResult E = PerformContextuallyConvertToBool(CondExpr);
+  if (!IsConstexpr || CondExpr->isValueDependent())
+    return E;
----------------
Minor nit: I think it would be a tiny bit clearer if you factored out this 
condition, since you repeat it:
```
bool IsConstexprNonValueDependent = IsConstexpr  && 
!CondExpr->isValueDependent();
if (!LangOpts.CPlusPlus2b && IsConstexprNonValueDependent) {
....
if (!IsConstexprNonValueDependent)
```


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3935
+  E = VerifyIntegerConstantExpression(
+      E.get(), &Cond,
+      diag::err_constexpr_if_condition_expression_is_not_constant);
----------------
The value returned from `PerformContextuallyConvertToBool` can have a possibly 
failed state (We follow this convention for FooResult named types).
But here there is a `get` on it on a path where there could be an error.
Can you handle this, and also add test case to cover it?


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

Reply via email to