aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaOverload.cpp:5638 ImplicitConversionSequence ICS = - CCE == Sema::CCEK_ExplicitBool - ? TryContextuallyConvertToBool(S, From) - : TryCopyInitialization(S, From, T, - /*SuppressUserConversions=*/false, - /*InOverloadResolution=*/false, - /*AllowObjCWritebackConversion=*/false, - /*AllowExplicit=*/false); + TryCopyInitialization(S, From, T, + /*SuppressUserConversions=*/false, ---------------- This function is checking converted constant expressions which was not touched by p1401r5, and it looks like this will break anything attempting to to contextual conversion constant expressions of type bool per http://eel.is/c++draft/expr.const#10 because it's removing the attempt to contextually convert to bool here. ================ Comment at: clang/lib/Sema/SemaOverload.cpp:5638 ImplicitConversionSequence ICS = - CCE == Sema::CCEK_ExplicitBool - ? TryContextuallyConvertToBool(S, From) - : TryCopyInitialization(S, From, T, - /*SuppressUserConversions=*/false, - /*InOverloadResolution=*/false, - /*AllowObjCWritebackConversion=*/false, - /*AllowExplicit=*/false); + TryCopyInitialization(S, From, T, + /*SuppressUserConversions=*/false, ---------------- aaron.ballman wrote: > This function is checking converted constant expressions which was not > touched by p1401r5, and it looks like this will break anything attempting to > to contextual conversion constant expressions of type bool per > http://eel.is/c++draft/expr.const#10 because it's removing the attempt to > contextually convert to bool here. Also, I don't think P1401 was applied as a DR (but we should double check!) so I'd expect some checks for the language standard to gate the behavior. If P1401 was applied as a DR, then we should add some tests to clang/test/CXX with the proper DR markings so that our DR status page gets updated. ================ Comment at: clang/test/SemaCXX/cxx2a-explicit-bool.cpp:731 + +namespace P1401 { + ---------------- Curiously, these tests already have this behavior without this patch applied: https://godbolt.org/z/13PojdWEe Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106216/new/ https://reviews.llvm.org/D106216 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
