[PATCH] D106216: Disallow narrowing conversions to bool in explicit specififers.

[Aaron Ballman via Phabricator via cfe-commits]

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