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