Skip to content

Commit

Permalink
Revert "CWG2352: Allow qualification conversions during reference bin…
Browse files Browse the repository at this point in the history 
…ding."

This reverts commit de21704.

Regressed/causes this to error due to ambiguity:

  void f(const int * const &);
  void f(int *);
  int main() {
    int * x;
    f(x);
  }

(in case it's important - the original case where this turned up was a
member function overload in a class template with, essentially:

  f(const T1&)
  f(T2*)

(where T1 == X const *, T2 == X))

It's not super clear to me if this ^ is expected behavior, in which case
I'm sorry about the revert & happy to look into ways to fix the original
code.
  • Loading branch information
@dwblaikie
dwblaikie committed Dec 27, 2019
1 parent c3d3569 commit d801823
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 250 deletions.
3 changes: 1 addition & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Expand Up @@ -1933,8 +1933,7 @@ def err_lvalue_reference_bind_to_unrelated : Error<
"cannot bind to a value of unrelated type}1,2">;
def err_reference_bind_drops_quals : Error<
"binding reference %diff{of type $ to value of type $|to value}0,1 "
"%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space|"
"not permitted due to incompatible qualifiers}2">;
"%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space}2">;
def err_reference_bind_failed : Error<
"reference %diff{to %select{type|incomplete type}1 $ could not bind to an "
"%select{rvalue|lvalue}2 of type $|could not bind to %select{rvalue|lvalue}2 of "
Expand Down
2 changes: 0 additions & 2 deletions clang/lib/Sema/SemaExprCXX.cpp
View file
Expand Up @@ -5864,8 +5864,6 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
// one of the operands is reference-compatible with the other, in order
// to support conditionals between functions differing in noexcept. This
// will similarly cover difference in array bounds after P0388R4.
// FIXME: If LTy and RTy have a composite pointer type, should we convert to
// that instead?
ExprValueKind LVK = LHS.get()->getValueKind();
ExprValueKind RVK = RHS.get()->getValueKind();
if (!Context.hasSameType(LTy, RTy) &&
Expand Down
8 changes: 1 addition & 7 deletions clang/lib/Sema/SemaInit.cpp
View file
Expand Up @@ -8919,17 +8919,11 @@ bool InitializationSequence::Diagnose(Sema &S,
S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
<< NonRefType << SourceType << 1 /*addr space*/
<< Args[0]->getSourceRange();
else if (DroppedQualifiers.hasQualifiers())
else
S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
<< NonRefType << SourceType << 0 /*cv quals*/
<< Qualifiers::fromCVRMask(DroppedQualifiers.getCVRQualifiers())
<< DroppedQualifiers.getCVRQualifiers() << Args[0]->getSourceRange();
else
// FIXME: Consider decomposing the type and explaining which qualifiers
// were dropped where, or on which level a 'const' is missing, etc.
S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
<< NonRefType << SourceType << 2 /*incompatible quals*/
<< Args[0]->getSourceRange();
break;
}

Expand Down
290 changes: 135 additions & 155 deletions clang/lib/Sema/SemaOverload.cpp
View file
Expand Up @@ -3153,70 +3153,6 @@ static bool isNonTrivialObjCLifetimeConversion(Qualifiers FromQuals,
return true;
}

/// Perform a single iteration of the loop for checking if a qualification
/// conversion is valid.
///
/// Specifically, check whether any change between the qualifiers of \p
/// FromType and \p ToType is permissible, given knowledge about whether every
/// outer layer is const-qualified.
static bool isQualificationConversionStep(QualType FromType, QualType ToType,
bool CStyle,
bool &PreviousToQualsIncludeConst,
bool &ObjCLifetimeConversion) {
Qualifiers FromQuals = FromType.getQualifiers();
Qualifiers ToQuals = ToType.getQualifiers();

// Ignore __unaligned qualifier if this type is void.
if (ToType.getUnqualifiedType()->isVoidType())
FromQuals.removeUnaligned();

// Objective-C ARC:
// Check Objective-C lifetime conversions.
if (FromQuals.getObjCLifetime() != ToQuals.getObjCLifetime()) {
if (ToQuals.compatiblyIncludesObjCLifetime(FromQuals)) {
if (isNonTrivialObjCLifetimeConversion(FromQuals, ToQuals))
ObjCLifetimeConversion = true;
FromQuals.removeObjCLifetime();
ToQuals.removeObjCLifetime();
} else {
// Qualification conversions cannot cast between different
// Objective-C lifetime qualifiers.
return false;
}
}

// Allow addition/removal of GC attributes but not changing GC attributes.
if (FromQuals.getObjCGCAttr() != ToQuals.getObjCGCAttr() &&
(!FromQuals.hasObjCGCAttr() || !ToQuals.hasObjCGCAttr())) {
FromQuals.removeObjCGCAttr();
ToQuals.removeObjCGCAttr();
}

// -- for every j > 0, if const is in cv 1,j then const is in cv
// 2,j, and similarly for volatile.
if (!CStyle && !ToQuals.compatiblyIncludes(FromQuals))
return false;

// For a C-style cast, just require the address spaces to overlap.
// FIXME: Does "superset" also imply the representation of a pointer is the
// same? We're assuming that it does here and in compatiblyIncludes.
if (CStyle && !ToQuals.isAddressSpaceSupersetOf(FromQuals) &&
!FromQuals.isAddressSpaceSupersetOf(ToQuals))
return false;

// -- if the cv 1,j and cv 2,j are different, then const is in
// every cv for 0 < k < j.
if (!CStyle && FromQuals.getCVRQualifiers() != ToQuals.getCVRQualifiers() &&
!PreviousToQualsIncludeConst)
return false;

// Keep track of whether all prior cv-qualifiers in the "to" type
// include const.
PreviousToQualsIncludeConst =
PreviousToQualsIncludeConst && ToQuals.hasConst();
return true;
}

/// IsQualificationConversion - Determines whether the conversion from
/// an rvalue of type FromType to ToType is a qualification conversion
/// (C++ 4.4).
Expand All @@ -3242,16 +3178,73 @@ Sema::IsQualificationConversion(QualType FromType, QualType ToType,
bool PreviousToQualsIncludeConst = true;
bool UnwrappedAnyPointer = false;
while (Context.UnwrapSimilarTypes(FromType, ToType)) {
if (!isQualificationConversionStep(FromType, ToType, CStyle,
PreviousToQualsIncludeConst,
ObjCLifetimeConversion))
return false;
// Within each iteration of the loop, we check the qualifiers to
// determine if this still looks like a qualification
// conversion. Then, if all is well, we unwrap one more level of
// pointers or pointers-to-members and do it all again
// until there are no more pointers or pointers-to-members left to
// unwrap.
UnwrappedAnyPointer = true;

Qualifiers FromQuals = FromType.getQualifiers();
Qualifiers ToQuals = ToType.getQualifiers();

// Ignore __unaligned qualifier if this type is void.
if (ToType.getUnqualifiedType()->isVoidType())
FromQuals.removeUnaligned();

// Objective-C ARC:
// Check Objective-C lifetime conversions.
if (FromQuals.getObjCLifetime() != ToQuals.getObjCLifetime() &&
UnwrappedAnyPointer) {
if (ToQuals.compatiblyIncludesObjCLifetime(FromQuals)) {
if (isNonTrivialObjCLifetimeConversion(FromQuals, ToQuals))
ObjCLifetimeConversion = true;
FromQuals.removeObjCLifetime();
ToQuals.removeObjCLifetime();
} else {
// Qualification conversions cannot cast between different
// Objective-C lifetime qualifiers.
return false;
}
}

// Allow addition/removal of GC attributes but not changing GC attributes.
if (FromQuals.getObjCGCAttr() != ToQuals.getObjCGCAttr() &&
(!FromQuals.hasObjCGCAttr() || !ToQuals.hasObjCGCAttr())) {
FromQuals.removeObjCGCAttr();
ToQuals.removeObjCGCAttr();
}

// -- for every j > 0, if const is in cv 1,j then const is in cv
// 2,j, and similarly for volatile.
if (!CStyle && !ToQuals.compatiblyIncludes(FromQuals))
return false;

// -- if the cv 1,j and cv 2,j are different, then const is in
// every cv for 0 < k < j.
if (!CStyle && FromQuals.getCVRQualifiers() != ToQuals.getCVRQualifiers()
&& !PreviousToQualsIncludeConst)
return false;

// Keep track of whether all prior cv-qualifiers in the "to" type
// include const.
PreviousToQualsIncludeConst
= PreviousToQualsIncludeConst && ToQuals.hasConst();
}

// Allows address space promotion by language rules implemented in
// Type::Qualifiers::isAddressSpaceSupersetOf.
Qualifiers FromQuals = FromType.getQualifiers();
Qualifiers ToQuals = ToType.getQualifiers();
if (!ToQuals.isAddressSpaceSupersetOf(FromQuals) &&
!FromQuals.isAddressSpaceSupersetOf(ToQuals)) {
return false;
}

// We are left with FromType and ToType being the pointee types
// after unwrapping the original FromType and ToType the same number
// of times. If we unwrapped any pointers, and if FromType and
// of types. If we unwrapped any pointers, and if FromType and
// ToType have the same unqualified type (since we checked
// qualifiers above), then this is a qualification conversion.
return UnwrappedAnyPointer && Context.hasSameUnqualifiedType(FromType,ToType);
Expand Down Expand Up @@ -3997,41 +3990,32 @@ CompareStandardConversionSequences(Sema &S, SourceLocation Loc,
// top-level cv-qualifiers, and the type to which the reference
// initialized by S2 refers is more cv-qualified than the type
// to which the reference initialized by S1 refers.
// FIXME: This should have been updated by DR2352, but was overlooked. The
// corrected rule is:
// -- S1 and S2 include reference bindings, and references refer to types
// T1 and T2, respectively, where T2 is reference-compatible with T1.
QualType T1 = SCS1.getToType(2);
QualType T2 = SCS2.getToType(2);
T1 = S.Context.getCanonicalType(T1);
T2 = S.Context.getCanonicalType(T2);
Qualifiers T1Quals, T2Quals;
QualType UnqualT1 = S.Context.getUnqualifiedArrayType(T1, T1Quals);
QualType UnqualT2 = S.Context.getUnqualifiedArrayType(T2, T2Quals);
if (UnqualT1 == UnqualT2) {
// Objective-C++ ARC: If the references refer to objects with different
// lifetimes, prefer bindings that don't change lifetime.
if (SCS1.ObjCLifetimeConversionBinding !=
SCS2.ObjCLifetimeConversionBinding) {
return SCS1.ObjCLifetimeConversionBinding
? ImplicitConversionSequence::Worse
: ImplicitConversionSequence::Better;
}

// Objective-C++ ARC: If the references refer to objects with different
// lifetimes, prefer bindings that don't change lifetime.
//
// FIXME: Should this really override ordering based on qualification
// conversions? In the correspnding check for pointers, we treat a case
// where one candidate has worse qualifications and the other has a
// lifetime conversion as ambiguous.
if (SCS1.ObjCLifetimeConversionBinding !=
SCS2.ObjCLifetimeConversionBinding &&
S.Context.hasSameUnqualifiedType(T1, T2)) {
return SCS1.ObjCLifetimeConversionBinding
? ImplicitConversionSequence::Worse
: ImplicitConversionSequence::Better;
}

if (!S.Context.hasSameType(T1, T2)) {
// FIXME: Unfortunately, there are pairs of types that admit reference
// bindings in both directions, so we can't shortcut the second check
// here.
bool Better =
S.CompareReferenceRelationship(Loc, T2, T1) == Sema::Ref_Compatible;
bool Worse =
S.CompareReferenceRelationship(Loc, T1, T2) == Sema::Ref_Compatible;
if (Better && Worse)
return ImplicitConversionSequence::Indistinguishable;
if (Better)
// If the type is an array type, promote the element qualifiers to the
// type for comparison.
if (isa<ArrayType>(T1) && T1Quals)
T1 = S.Context.getQualifiedType(UnqualT1, T1Quals);
if (isa<ArrayType>(T2) && T2Quals)
T2 = S.Context.getQualifiedType(UnqualT2, T2Quals);
if (T2.isMoreQualifiedThan(T1))
return ImplicitConversionSequence::Better;
if (Worse)
else if (T1.isMoreQualifiedThan(T2))
return ImplicitConversionSequence::Worse;
}
}
Expand Down Expand Up @@ -4418,19 +4402,10 @@ static bool isTypeValid(QualType T) {
return true;
}

static QualType withoutUnaligned(ASTContext &Ctx, QualType T) {
if (!T.getQualifiers().hasUnaligned())
return T;

Qualifiers Q;
T = Ctx.getUnqualifiedArrayType(T, Q);
Q.removeUnaligned();
return Ctx.getQualifiedType(T, Q);
}

/// CompareReferenceRelationship - Compare the two types T1 and T2 to
/// determine whether they are reference-compatible,
/// reference-related, or incompatible, for use in C++ initialization by
/// determine whether they are reference-related,
/// reference-compatible, reference-compatible with added
/// qualification, or incompatible, for use in C++ initialization by
/// reference (C++ [dcl.ref.init]p4). Neither type can be a reference
/// type, and the first type (T1) is the pointee type of the reference
/// type being initialized.
Expand All @@ -4452,17 +4427,10 @@ Sema::CompareReferenceRelationship(SourceLocation Loc,
ReferenceConversions &Conv = ConvOut ? *ConvOut : ConvTmp;
Conv = ReferenceConversions();

// C++2a [dcl.init.ref]p4:
// C++ [dcl.init.ref]p4:
// Given types "cv1 T1" and "cv2 T2," "cv1 T1" is
// reference-related to "cv2 T2" if T1 is similar to T2, or
// reference-related to "cv2 T2" if T1 is the same type as T2, or
// T1 is a base class of T2.
// "cv1 T1" is reference-compatible with "cv2 T2" if
// a prvalue of type "pointer to cv2 T2" can be converted to the type
// "pointer to cv1 T1" via a standard conversion sequence.

// Check for standard conversions we can apply to pointers: derived-to-base
// conversions, ObjC pointer conversions, and function pointer conversions.
// (Qualification conversions are checked last.)
QualType ConvertedT2;
if (UnqualT1 == UnqualT2) {
// Nothing to do.
Expand All @@ -4476,47 +4444,59 @@ Sema::CompareReferenceRelationship(SourceLocation Loc,
Conv |= ReferenceConversions::ObjC;
else if (UnqualT2->isFunctionType() &&
IsFunctionConversion(UnqualT2, UnqualT1, ConvertedT2)) {
// C++1z [dcl.init.ref]p4:
// cv1 T1" is reference-compatible with "cv2 T2" if [...] T2 is "noexcept
// function" and T1 is "function"
//
// We extend this to also apply to 'noreturn', so allow any function
// conversion between function types.
Conv |= ReferenceConversions::Function;
// No need to check qualifiers; function types don't have them.
return Ref_Compatible;
} else
return Ref_Incompatible;

// At this point, we know that T1 and T2 are reference-related (at
// least).

// If the type is an array type, promote the element qualifiers to the type
// for comparison.
if (isa<ArrayType>(T1) && T1Quals)
T1 = Context.getQualifiedType(UnqualT1, T1Quals);
if (isa<ArrayType>(T2) && T2Quals)
T2 = Context.getQualifiedType(UnqualT2, T2Quals);

// C++ [dcl.init.ref]p4:
// "cv1 T1" is reference-compatible with "cv2 T2" if T1 is
// reference-related to T2 and cv1 is the same cv-qualification
// as, or greater cv-qualification than, cv2. For purposes of
// overload resolution, cases for which cv1 is greater
// cv-qualification than cv2 are identified as
// reference-compatible with added qualification (see 13.3.3.2).
//
// Note that we also require equivalence of Objective-C GC and address-space
// qualifiers when performing these computations, so that e.g., an int in
// address space 1 is not reference-compatible with an int in address
// space 2.
if (T1Quals.getObjCLifetime() != T2Quals.getObjCLifetime() &&
T1Quals.compatiblyIncludesObjCLifetime(T2Quals)) {
if (isNonTrivialObjCLifetimeConversion(T2Quals, T1Quals))
Conv |= ReferenceConversions::ObjCLifetime;

T1Quals.removeObjCLifetime();
T2Quals.removeObjCLifetime();
}
bool ConvertedReferent = Conv != 0;

// We can have a qualification conversion. Compute whether the types are
// similar at the same time.
bool PreviousToQualsIncludeConst = true;
do {
if (T1 == T2)
break;
// MS compiler ignores __unaligned qualifier for references; do the same.
T1Quals.removeUnaligned();
T2Quals.removeUnaligned();

// We will need a qualification conversion.
if (T1Quals != T2Quals)
Conv |= ReferenceConversions::Qualification;

// MS compiler ignores __unaligned qualifier for references; do the same.
T1 = withoutUnaligned(Context, T1);
T2 = withoutUnaligned(Context, T2);

// If we find a qualifier mismatch, the types are not reference-compatible,
// but are still be reference-related if they're similar.
bool ObjCLifetimeConversion = false;
if (!isQualificationConversionStep(T2, T1, /*CStyle=*/false,
PreviousToQualsIncludeConst,
ObjCLifetimeConversion))
return (ConvertedReferent || Context.hasSimilarType(T1, T2))
? Ref_Related
: Ref_Incompatible;

// FIXME: Should we track this for any level other than the first?
if (ObjCLifetimeConversion)
Conv |= ReferenceConversions::ObjCLifetime;
} while (Context.UnwrapSimilarTypes(T1, T2));

// At this point, if the types are reference-related, we must either have the
// same inner type (ignoring qualifiers), or must have already worked out how
// to convert the referent.
return (ConvertedReferent || Context.hasSameUnqualifiedType(T1, T2))
? Ref_Compatible
: Ref_Incompatible;
if (T1Quals.compatiblyIncludes(T2Quals))
return Ref_Compatible;
else
return Ref_Related;
}

/// Look for a user-defined conversion to a value reference-compatible
Expand Down

0 comments on commit d801823

Please sign in to comment.