[cfe-dev] Implementing the proposed resolution of C++ CWG 1423
dyp
dyp-cpp at gmx.net
Wed May 27 17:30:46 PDT 2015
Dear clang developers,
I've been trying to implement the resolution of C++ CWG 1423 in clang++
which restricts conversions from nullptr_t to bool to
direct-initialization. This is my first attempt at contributing to
clang, and as far as I can tell, it's the only standard conversion that
is not an implicit conversion.
(I couldn't find any bug report nor mailing on this, and I wanted to
contribute to clang for some time now, so I just went ahead and
implemented it. Even if my solution is too crappy to be accepted, it was
a nice exercise and not too hard I guess ;)
While I was successful in adding the restriction, there remain some
issues with my fix, and I'm not very comfortable with my
"solution" for list-initialization. Therefore, I'd appreciate any
feedback and suggestions how to introduce the restrictions. I hope my
request is appropriate for this mailing list.
Specifically, I have the following questions:
1. Changes of public member functions
I've modified the signature of two public member functions of the Sema
class by adding an additional parameter. How problematic is this change?
(API etc)
2. OpenMP support
I've added an additional parameter to the first two overloads of
Sema::PerformImplicitConversion, that specifies whether or not we're
performing a direct initialization. This function is also used by
SemaOpenMP.cpp
I have never worked with OpenMP, so I don't really know if the usages in
SemaOpenMP.cpp occur within a direct-initialization context.
3. Diagnostics
How important are good diagnostic messages for this new error case?
Should better/specific diagnostics be included in the patch, or can they
be improved in a later patch?
4. Test cases
Since CWG 1423 is classified as a DR, and g++ implements it also in
C++11 mode, I did not include any language standard switches for this
fix. However, this leads to some of the existing tests now failing. I
have modified them, and added further test cases for this special
implicit conversion. Unfortunately, this means that there's no separate
test for this fix. Is there a better solution?
5. Support for list-initialization
I couldn't figure out how to properly add a distinction between
direct-list-initialization and copy-list-initialization for CWG 1423.
The TryListInitialization function relies on the InitListChecker class
for all scalar initialization. As the comment some lines above points
out, the InitListChecker class always performs copy-initialization, and
it seemed to me to be deeply ingrained in its algorithms. There already
are several special cases in TryListInitialization that handle
direct-list-initialization where the destination type is a record type.
I've tried to add scalar initialization to one case that uses
InitializationSequence::InitializeFrom (which finally dispatches to
Sema::TryImplicitConversion which I've already modified for
direct-nonlist-initialization), but this makes some tests fail
apparently due to a lack of support of initialization of vector types,
different diagnostics and compound literals.
It seems this part could benefit from some refactoring, although I'm not
sure what would be a good approach.
As a quick and dirty fix, I've routed only the nullptr_t -> bool
conversion through this special case, which is of course only a
workaround to prevent the aforementioned issues. It does work, but it
feels like a really dirty hack.
6. A naming issue
It occurred to me when writing this email that my extension of
Sema::TryImplicitConversion might be inappropriate:
Direct-initialization is not an implicit conversion as defined in
C++14[conv]p3.
Sema::TryContextuallyConvertToBool also uses TryImplicitConversion, even
though that isn't strictly an implicit conversion either. (Probably just
a bike-shedding issue.)
Thank you very much and kind regards,
dyp
-------------- next part --------------
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 238381)
+++ include/clang/Sema/Sema.h (working copy)
@@ -2079,7 +2079,8 @@
bool AllowExplicit,
bool InOverloadResolution,
bool CStyle,
- bool AllowObjCWritebackConversion);
+ bool AllowObjCWritebackConversion,
+ bool IsDirectInit);
bool IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType);
bool IsFloatingPointPromotion(QualType FromType, QualType ToType);
@@ -8044,10 +8045,12 @@
ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
AssignmentAction Action,
- bool AllowExplicit = false);
+ bool AllowExplicit = false,
+ bool IsDirectInit = false);
ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
AssignmentAction Action,
bool AllowExplicit,
+ bool IsDirectInit,
ImplicitConversionSequence& ICS);
ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
const ImplicitConversionSequence& ICS,
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 238381)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -7113,7 +7113,7 @@
ExprResult Res;
if (Diagnose) {
Res = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
- AA_Assigning);
+ AA_Assigning, /*IsDirectInit=*/false);
} else {
ImplicitConversionSequence ICS =
TryImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
@@ -7121,7 +7121,8 @@
/*AllowExplicit=*/false,
/*InOverloadResolution=*/false,
/*CStyle=*/false,
- /*AllowObjCWritebackConversion=*/false);
+ /*AllowObjCWritebackConversion=*/false,
+ /*IsDirectInit=*/false);
if (ICS.isFailure())
return Incompatible;
Res = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp (revision 238381)
+++ lib/Sema/SemaExprCXX.cpp (working copy)
@@ -1367,7 +1367,7 @@
assert(Context.getTargetInfo().getIntWidth() && "Builtin type of size 0?");
ConvertedSize = PerformImplicitConversion(ArraySize, Context.getSizeType(),
- AA_Converting);
+ AA_Converting, /*IsDirectInit=*/false);
if (!ConvertedSize.isInvalid() &&
ArraySize->getType()->getAs<RecordType>())
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp (revision 238381)
+++ lib/Sema/SemaInit.cpp (working copy)
@@ -3602,33 +3602,40 @@
}
if (S.getLangOpts().CPlusPlus && !DestType->isAggregateType() &&
- InitList->getNumInits() == 1 &&
- InitList->getInit(0)->getType()->isRecordType()) {
- // - Otherwise, if the initializer list has a single element of type E
- // [...references are handled above...], the object or reference is
- // initialized from that element (by copy-initialization for
- // copy-list-initialization, or by direct-initialization for
- // direct-list-initialization); if a narrowing conversion is required
- // to convert the element to T, the program is ill-formed.
- //
- // Per core-24034, this is direct-initialization if we were performing
- // direct-list-initialization and copy-initialization otherwise.
- // We can't use InitListChecker for this, because it always performs
- // copy-initialization. This only matters if we might use an 'explicit'
- // conversion operator, so we only need to handle the cases where the source
- // is of record type.
- InitializationKind SubKind =
- Kind.getKind() == InitializationKind::IK_DirectList
- ? InitializationKind::CreateDirect(Kind.getLocation(),
- InitList->getLBraceLoc(),
- InitList->getRBraceLoc())
- : Kind;
- Expr *SubInit[1] = { InitList->getInit(0) };
- Sequence.InitializeFrom(S, Entity, SubKind, SubInit,
- /*TopLevelOfInitList*/true);
- if (Sequence)
- Sequence.RewrapReferenceInitList(Entity.getType(), InitList);
- return;
+ InitList->getNumInits() == 1) {
+ QualType InitType = InitList->getInit(0)->getType();
+ // CWG 1423
+ const Type* InitTypeUQ = InitType.getTypePtrOrNull();
+ const bool IsNullptrToBool = DestType->isBooleanType() &&
+ InitTypeUQ && InitTypeUQ->isNullPtrType();
+ if(InitType->isRecordType() || IsNullptrToBool)
+ {
+ // - Otherwise, if the initializer list has a single element of type E
+ // [...references are handled above...], the object or reference is
+ // initialized from that element (by copy-initialization for
+ // copy-list-initialization, or by direct-initialization for
+ // direct-list-initialization); if a narrowing conversion is required
+ // to convert the element to T, the program is ill-formed.
+ //
+ // Per core-24034, this is direct-initialization if we were performing
+ // direct-list-initialization and copy-initialization otherwise.
+ // We can't use InitListChecker for this, because it always performs
+ // copy-initialization. This only matters if we might use an 'explicit'
+ // conversion operator, so we only need to handle the cases where the source
+ // is of record type.
+ InitializationKind SubKind =
+ Kind.getKind() == InitializationKind::IK_DirectList
+ ? InitializationKind::CreateDirect(Kind.getLocation(),
+ InitList->getLBraceLoc(),
+ InitList->getRBraceLoc())
+ : Kind;
+ Expr *SubInit[1] = { InitList->getInit(0) };
+ Sequence.InitializeFrom(S, Entity, SubKind, SubInit,
+ /*TopLevelOfInitList*/true);
+ if (Sequence)
+ Sequence.RewrapReferenceInitList(Entity.getType(), InitList);
+ return;
+ }
}
InitListChecker CheckInitList(S, Entity, InitList,
@@ -4101,7 +4108,8 @@
/*AllowExplicit=*/false,
/*FIXME:InOverloadResolution=*/false,
/*CStyle=*/Kind.isCStyleOrFunctionalCast(),
- /*AllowObjCWritebackConversion=*/false);
+ /*AllowObjCWritebackConversion=*/false,
+ /*IsDirectInit=*/false);
if (ICS.isBad()) {
// FIXME: Use the conversion function set stored in ICS to turn
@@ -4884,6 +4892,8 @@
// initializer expression to the cv-unqualified version of the
// destination type; no user-defined conversions are considered.
+ const bool IsDirectInit = Kind.getKind() == InitializationKind::IK_Direct ||
+ Kind.getKind() == InitializationKind::IK_DirectList;
ImplicitConversionSequence ICS
= S.TryImplicitConversion(Initializer, DestType,
/*SuppressUserConversions*/true,
@@ -4890,7 +4900,8 @@
/*AllowExplicitConversions*/ false,
/*InOverloadResolution*/ false,
/*CStyle=*/Kind.isCStyleOrFunctionalCast(),
- allowObjCWritebackConversion);
+ allowObjCWritebackConversion,
+ IsDirectInit);
if (ICS.isStandard() &&
ICS.Standard.Second == ICK_Writeback_Conversion) {
Index: lib/Sema/SemaOpenMP.cpp
===================================================================
--- lib/Sema/SemaOpenMP.cpp (revision 238381)
+++ lib/Sema/SemaOpenMP.cpp (working copy)
@@ -2463,7 +2463,8 @@
QualType NewType = C.getIntTypeForBitwidth(
NewSize, Type->hasSignedIntegerRepresentation());
Diff = SemaRef.PerformImplicitConversion(Diff.get(), NewType,
- Sema::AA_Converting, true);
+ Sema::AA_Converting,
+ /*AllowExplicit=*/true);
if (!Diff.isUsable())
return nullptr;
}
@@ -2692,7 +2693,8 @@
return ExprError();
Update = SemaRef.PerformImplicitConversion(
- Update.get(), VarRef.get()->getType(), Sema::AA_Converting, true);
+ Update.get(), VarRef.get()->getType(), Sema::AA_Converting,
+ /*AllowExplict=*/true);
if (!Update.isUsable())
return ExprError();
@@ -2714,7 +2716,7 @@
// OK to convert to signed, because new type has more bits than old.
QualType NewType = C.getIntTypeForBitwidth(Bits, /* Signed */ true);
return SemaRef.PerformImplicitConversion(E, NewType, Sema::AA_Converting,
- true);
+ /*AllowExplicit=*/true);
}
/// \brief Check if the given expression \a E is a constant integer that fits
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp (revision 238381)
+++ lib/Sema/SemaOverload.cpp (working copy)
@@ -72,13 +72,15 @@
bool InOverloadResolution,
StandardConversionSequence &SCS,
bool CStyle,
- bool AllowObjCWritebackConversion);
+ bool AllowObjCWritebackConversion,
+ bool IsDirectInit);
static bool IsTransparentUnionStandardConversion(Sema &S, Expr* From,
QualType &ToType,
bool InOverloadResolution,
StandardConversionSequence &SCS,
- bool CStyle);
+ bool CStyle,
+ bool IsDirectInit);
static OverloadingResult
IsUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
UserDefinedConversionSequence& User,
@@ -1185,6 +1187,10 @@
/// \param AllowObjCWritebackConversion Whether we allow the Objective-C
/// writeback conversion, which allows __autoreleasing id* parameters to
/// be initialized with __strong id* or __weak id* arguments.
+///
+/// \param IsDirectInit Whether or not this conversion occurs as a
+/// direct initialization. With CWG 1423, nullptr_t can only be
+/// converted to bool for direct-initialization.
static ImplicitConversionSequence
TryImplicitConversion(Sema &S, Expr *From, QualType ToType,
bool SuppressUserConversions,
@@ -1192,10 +1198,12 @@
bool InOverloadResolution,
bool CStyle,
bool AllowObjCWritebackConversion,
- bool AllowObjCConversionOnExplicit) {
+ bool AllowObjCConversionOnExplicit,
+ bool IsDirectInit) {
ImplicitConversionSequence ICS;
if (IsStandardConversion(S, From, ToType, InOverloadResolution,
- ICS.Standard, CStyle, AllowObjCWritebackConversion)){
+ ICS.Standard, CStyle, AllowObjCWritebackConversion,
+ IsDirectInit)){
ICS.setStandard();
return ICS;
}
@@ -1246,12 +1254,14 @@
bool AllowExplicit,
bool InOverloadResolution,
bool CStyle,
- bool AllowObjCWritebackConversion) {
+ bool AllowObjCWritebackConversion,
+ bool IsDirectInit) {
return ::TryImplicitConversion(*this, From, ToType,
SuppressUserConversions, AllowExplicit,
InOverloadResolution, CStyle,
AllowObjCWritebackConversion,
- /*AllowObjCConversionOnExplicit=*/false);
+ /*AllowObjCConversionOnExplicit=*/false,
+ IsDirectInit);
}
/// PerformImplicitConversion - Perform an implicit conversion of the
@@ -1261,14 +1271,17 @@
/// explicit user-defined conversions are permitted.
ExprResult
Sema::PerformImplicitConversion(Expr *From, QualType ToType,
- AssignmentAction Action, bool AllowExplicit) {
+ AssignmentAction Action, bool AllowExplicit,
+ bool IsDirectInit) {
ImplicitConversionSequence ICS;
- return PerformImplicitConversion(From, ToType, Action, AllowExplicit, ICS);
+ return PerformImplicitConversion(From, ToType, Action, AllowExplicit,
+ IsDirectInit, ICS);
}
ExprResult
Sema::PerformImplicitConversion(Expr *From, QualType ToType,
AssignmentAction Action, bool AllowExplicit,
+ bool IsDirectInit,
ImplicitConversionSequence& ICS) {
if (checkPlaceholderForOverload(*this, From))
return ExprError();
@@ -1286,7 +1299,8 @@
/*InOverloadResolution=*/false,
/*CStyle=*/false,
AllowObjCWritebackConversion,
- /*AllowObjCConversionOnExplicit=*/false);
+ /*AllowObjCConversionOnExplicit=*/false,
+ IsDirectInit);
return PerformImplicitConversion(From, ToType, ICS, Action);
}
@@ -1386,7 +1400,8 @@
static bool tryAtomicConversion(Sema &S, Expr *From, QualType ToType,
bool InOverloadResolution,
StandardConversionSequence &SCS,
- bool CStyle);
+ bool CStyle,
+ bool IsDirectInit);
/// IsStandardConversion - Determines whether there is a standard
/// conversion sequence (C++ [conv], C++ [over.ics.scs]) from the
@@ -1400,7 +1415,8 @@
bool InOverloadResolution,
StandardConversionSequence &SCS,
bool CStyle,
- bool AllowObjCWritebackConversion) {
+ bool AllowObjCWritebackConversion,
+ bool IsDirectInit) {
QualType FromType = From->getType();
// Standard conversions (C++ [conv])
@@ -1560,8 +1576,10 @@
FromType->isAnyPointerType() ||
FromType->isBlockPointerType() ||
FromType->isMemberPointerType() ||
- FromType->isNullPtrType())) {
+ (FromType->isNullPtrType() && IsDirectInit))) {
// Boolean conversions (C++ 4.12).
+ // CWG 1423: Implicit conversion from nullptr_t to bool only as direct-init
+ // adapted also in C++11 mode to mirror gcc's behaviour
SCS.Second = ICK_Boolean_Conversion;
FromType = S.Context.BoolTy;
} else if (FromType->isIntegralOrUnscopedEnumerationType() &&
@@ -1617,11 +1635,11 @@
SCS.Second = ICK_NoReturn_Adjustment;
} else if (IsTransparentUnionStandardConversion(S, From, ToType,
InOverloadResolution,
- SCS, CStyle)) {
+ SCS, CStyle, IsDirectInit)) {
SCS.Second = ICK_TransparentUnionConversion;
FromType = ToType;
} else if (tryAtomicConversion(S, From, ToType, InOverloadResolution, SCS,
- CStyle)) {
+ CStyle, IsDirectInit)) {
// tryAtomicConversion has updated the standard conversion sequence
// appropriately.
return true;
@@ -1679,7 +1697,8 @@
QualType &ToType,
bool InOverloadResolution,
StandardConversionSequence &SCS,
- bool CStyle) {
+ bool CStyle,
+ bool IsDirectInit) {
const RecordType *UT = ToType->getAsUnionType();
if (!UT || !UT->getDecl()->hasAttr<TransparentUnionAttr>())
@@ -1689,7 +1708,7 @@
// It's compatible if the expression matches any of the fields.
for (const auto *it : UD->fields()) {
if (IsStandardConversion(S, From, it->getType(), InOverloadResolution, SCS,
- CStyle, /*ObjCWritebackConversion=*/false)) {
+ CStyle, /*ObjCWritebackConversion=*/false, IsDirectInit)) {
ToType = it->getType();
return true;
}
@@ -2876,7 +2895,8 @@
static bool tryAtomicConversion(Sema &S, Expr *From, QualType ToType,
bool InOverloadResolution,
StandardConversionSequence &SCS,
- bool CStyle) {
+ bool CStyle,
+ bool IsDirectInit) {
const AtomicType *ToAtomic = ToType->getAs<AtomicType>();
if (!ToAtomic)
return false;
@@ -2884,7 +2904,7 @@
StandardConversionSequence InnerSCS;
if (!IsStandardConversion(S, From, ToAtomic->getValueType(),
InOverloadResolution, InnerSCS,
- CStyle, /*AllowObjCWritebackConversion=*/false))
+ CStyle, /*AllowObjCWritebackConversion=*/false, IsDirectInit))
return false;
SCS.Second = InnerSCS.Second;
@@ -4393,7 +4413,8 @@
/*InOverloadResolution=*/false,
/*CStyle=*/false,
/*AllowObjCWritebackConversion=*/false,
- /*AllowObjCConversionOnExplicit=*/false);
+ /*AllowObjCConversionOnExplicit=*/false,
+ /*IsDirectInit=*/false);
// Of course, that's still a reference binding.
if (ICS.isStandard()) {
@@ -4719,7 +4740,8 @@
InOverloadResolution,
/*CStyle=*/false,
AllowObjCWritebackConversion,
- /*AllowObjCConversionOnExplicit=*/false);
+ /*AllowObjCConversionOnExplicit=*/false,
+ /*IsDirectInit=*/false);
}
static bool TryCopyInitialization(const CanQualType FromQTy,
@@ -4919,7 +4941,8 @@
/*InOverloadResolution=*/false,
/*CStyle=*/false,
/*AllowObjCWritebackConversion=*/false,
- /*AllowObjCConversionOnExplicit=*/false);
+ /*AllowObjCConversionOnExplicit=*/false,
+ /*IsDirectInit=*/true);
}
/// PerformContextuallyConvertToBool - Perform a contextual conversion
@@ -5166,7 +5189,8 @@
/*InOverloadResolution=*/false,
/*CStyle=*/false,
/*AllowObjCWritebackConversion=*/false,
- /*AllowObjCConversionOnExplicit=*/true);
+ /*AllowObjCConversionOnExplicit=*/true,
+ /*IsDirectInit=*/true);
// Strip off any final conversions to 'id'.
switch (ICS.getKind()) {
Index: test/CXX/expr/expr.const/p3-0x.cpp
===================================================================
--- test/CXX/expr/expr.const/p3-0x.cpp (revision 238381)
+++ test/CXX/expr/expr.const/p3-0x.cpp (working copy)
@@ -84,7 +84,6 @@
using Int = A<1.0>; // expected-error {{conversion from 'double' to 'unsigned char' is not allowed in a converted constant expression}}
enum B : bool {
True = &a, // expected-error {{conversion from 'bool (*)(int)' to 'bool' is not allowed in a converted constant expression}}
- False = nullptr // expected-error {{conversion from 'nullptr_t' to 'bool' is not allowed in a converted constant expression}}
};
void c() {
// Note, promoted type of switch is 'int'.
Index: test/SemaCXX/conversion.cpp
===================================================================
--- test/SemaCXX/conversion.cpp (revision 238381)
+++ test/SemaCXX/conversion.cpp (working copy)
@@ -151,9 +151,8 @@
}
namespace test7 {
- bool fun() {
- bool x = nullptr; // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
+ void fun() {
+ bool x(nullptr); // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
if (nullptr) {} // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
- return nullptr; // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
}
}
Index: test/SemaCXX/nullptr.cpp
===================================================================
--- test/SemaCXX/nullptr.cpp (revision 238381)
+++ test/SemaCXX/nullptr.cpp (working copy)
@@ -25,7 +25,14 @@
pf = null;
void (A::*pmf)() = nullptr;
pmf = null;
- bool b = nullptr;
+ // CWG 1423
+ bool bd(nullptr);
+ bool bdl{nullptr};
+ bool bc = nullptr; // expected-error {{cannot initialize}}
+ bool bcl = {nullptr}; // expected-error {{cannot initialize}}
+ []() -> bool { return nullptr; }(); // expected-error {{cannot initialize}}
+ void(*pfb)(bool);
+ pfb(nullptr); // expected-error {{cannot initialize}}
// Can't convert nullptr to integral implicitly.
uintptr_t i = nullptr; // expected-error {{cannot initialize}}
More information about the cfe-dev
mailing list