r249995 - [Sema] Allow C conversions in C overload logic
Bob Wilson via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 8 12:38:11 PST 2016
George,
This change caused a serious regression for Objective-C method lookup. See PR26085 (http://llvm.org/pr26085).
For the test case in that PR, Sema::SelectBestMethod looks at the two candidate "test" methods. It will match the second one, but in the process of considering the first candidate, an error diagnostic is generated. This happens within the call to CheckSingleAssignmentConstraints that was added here in IsStandardConversion. The "Diagnose" argument in that call is set to false, but the diagnostic is generated anyway. For the test case in the PR, the diagnostic comes from CheckObjCARCConversion, but it looks like there are some other diagnostics that could also be generated from within CheckSingleAssignmentConstraints.
I think I could manage a fix, e.g., by threading the “Diagnose” flag through all the relevant code and consistently checking it before emitting diagnostics, but I’m not especially familiar with this part of clang. If you or someone else who knows more about this area can figure out the best way to fix it, I would appreciate it.
—Bob
> On Oct 11, 2015, at 1:13 PM, George Burgess IV via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> Author: gbiv
> Date: Sun Oct 11 15:13:20 2015
> New Revision: 249995
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249995&view=rev
> Log:
> [Sema] Allow C conversions in C overload logic
>
> C allows for some implicit conversions that C++ does not, e.g. void* ->
> char*. This patch teaches clang that these conversions are okay when
> dealing with overloads in C.
>
> Differential Revision: http://reviews.llvm.org/D13604
>
> Modified:
> cfe/trunk/include/clang/Sema/Overload.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaOverload.cpp
> cfe/trunk/test/Sema/overloadable.c
>
> Modified: cfe/trunk/include/clang/Sema/Overload.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Overload.h?rev=249995&r1=249994&r2=249995&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Overload.h (original)
> +++ cfe/trunk/include/clang/Sema/Overload.h Sun Oct 11 15:13:20 2015
> @@ -83,7 +83,8 @@ namespace clang {
> ICK_TransparentUnionConversion, ///< Transparent Union Conversions
> ICK_Writeback_Conversion, ///< Objective-C ARC writeback conversion
> ICK_Zero_Event_Conversion, ///< Zero constant to event (OpenCL1.2 6.12.10)
> - ICK_Num_Conversion_Kinds ///< The number of conversion kinds
> + ICK_C_Only_Conversion, ///< Conversions allowed in C, but not C++
> + ICK_Num_Conversion_Kinds, ///< The number of conversion kinds
> };
>
> /// ImplicitConversionRank - The rank of an implicit conversion
> @@ -95,7 +96,9 @@ namespace clang {
> ICR_Promotion, ///< Promotion
> ICR_Conversion, ///< Conversion
> ICR_Complex_Real_Conversion, ///< Complex <-> Real conversion
> - ICR_Writeback_Conversion ///< ObjC ARC writeback conversion
> + ICR_Writeback_Conversion, ///< ObjC ARC writeback conversion
> + ICR_C_Conversion ///< Conversion only allowed in the C standard.
> + /// (e.g. void* to char*)
> };
>
> ImplicitConversionRank GetConversionRank(ImplicitConversionKind Kind);
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=249995&r1=249994&r2=249995&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sun Oct 11 15:13:20 2015
> @@ -8292,19 +8292,23 @@ public:
> QualType LHSType,
> QualType RHSType);
>
> - /// Check assignment constraints and prepare for a conversion of the
> - /// RHS to the LHS type.
> + /// Check assignment constraints and optionally prepare for a conversion of
> + /// the RHS to the LHS type. The conversion is prepared for if ConvertRHS
> + /// is true.
> AssignConvertType CheckAssignmentConstraints(QualType LHSType,
> ExprResult &RHS,
> - CastKind &Kind);
> + CastKind &Kind,
> + bool ConvertRHS = true);
>
> // CheckSingleAssignmentConstraints - Currently used by
> // CheckAssignmentOperands, and ActOnReturnStmt. Prior to type checking,
> - // this routine performs the default function/array converions.
> + // this routine performs the default function/array converions, if ConvertRHS
> + // is true.
> AssignConvertType CheckSingleAssignmentConstraints(QualType LHSType,
> ExprResult &RHS,
> bool Diagnose = true,
> - bool DiagnoseCFAudited = false);
> + bool DiagnoseCFAudited = false,
> + bool ConvertRHS = true);
>
> // \brief If the lhs type is a transparent union, check whether we
> // can initialize the transparent union with the given expression.
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=249995&r1=249994&r2=249995&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sun Oct 11 15:13:20 2015
> @@ -5376,13 +5376,13 @@ CastKind Sema::PrepareScalarCast(ExprRes
> return CK_IntegralToFloating;
> case Type::STK_IntegralComplex:
> Src = ImpCastExprToType(Src.get(),
> - DestTy->castAs<ComplexType>()->getElementType(),
> - CK_IntegralCast);
> + DestTy->castAs<ComplexType>()->getElementType(),
> + CK_IntegralCast);
> return CK_IntegralRealToComplex;
> case Type::STK_FloatingComplex:
> Src = ImpCastExprToType(Src.get(),
> - DestTy->castAs<ComplexType>()->getElementType(),
> - CK_IntegralToFloating);
> + DestTy->castAs<ComplexType>()->getElementType(),
> + CK_IntegralToFloating);
> return CK_FloatingRealToComplex;
> case Type::STK_MemberPointer:
> llvm_unreachable("member pointer type in C");
> @@ -6867,7 +6867,7 @@ Sema::CheckAssignmentConstraints(SourceL
> ExprResult RHSPtr = &RHSExpr;
> CastKind K = CK_Invalid;
>
> - return CheckAssignmentConstraints(LHSType, RHSPtr, K);
> + return CheckAssignmentConstraints(LHSType, RHSPtr, K, /*ConvertRHS=*/false);
> }
>
> /// CheckAssignmentConstraints (C99 6.5.16) - This routine currently
> @@ -6889,7 +6889,7 @@ Sema::CheckAssignmentConstraints(SourceL
> /// Sets 'Kind' for any result kind except Incompatible.
> Sema::AssignConvertType
> Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS,
> - CastKind &Kind) {
> + CastKind &Kind, bool ConvertRHS) {
> QualType RHSType = RHS.get()->getType();
> QualType OrigLHSType = LHSType;
>
> @@ -6911,7 +6911,7 @@ Sema::CheckAssignmentConstraints(QualTyp
> CheckAssignmentConstraints(AtomicTy->getValueType(), RHS, Kind);
> if (result != Compatible)
> return result;
> - if (Kind != CK_NoOp)
> + if (Kind != CK_NoOp && ConvertRHS)
> RHS = ImpCastExprToType(RHS.get(), AtomicTy->getValueType(), Kind);
> Kind = CK_NonAtomicToAtomic;
> return Compatible;
> @@ -6941,7 +6941,7 @@ Sema::CheckAssignmentConstraints(QualTyp
> // CK_VectorSplat does T -> vector T, so first cast to the
> // element type.
> QualType elType = cast<ExtVectorType>(LHSType)->getElementType();
> - if (elType != RHSType) {
> + if (elType != RHSType && ConvertRHS) {
> Kind = PrepareScalarCast(RHS, elType);
> RHS = ImpCastExprToType(RHS.get(), elType, Kind);
> }
> @@ -6974,7 +6974,8 @@ Sema::CheckAssignmentConstraints(QualTyp
> // Arithmetic conversions.
> if (LHSType->isArithmeticType() && RHSType->isArithmeticType() &&
> !(getLangOpts().CPlusPlus && LHSType->isEnumeralType())) {
> - Kind = PrepareScalarCast(RHS, LHSType);
> + if (ConvertRHS)
> + Kind = PrepareScalarCast(RHS, LHSType);
> return Compatible;
> }
>
> @@ -7099,7 +7100,8 @@ Sema::CheckAssignmentConstraints(QualTyp
> // Only under strict condition T^ is compatible with an Objective-C pointer.
> if (RHSType->isBlockPointerType() &&
> LHSType->isBlockCompatibleObjCPointerType(Context)) {
> - maybeExtendBlockObject(RHS);
> + if (ConvertRHS)
> + maybeExtendBlockObject(RHS);
> Kind = CK_BlockPointerToObjCPointerCast;
> return Compatible;
> }
> @@ -7225,9 +7227,16 @@ Sema::CheckTransparentUnionArgumentConst
> }
>
> Sema::AssignConvertType
> -Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &RHS,
> +Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
> bool Diagnose,
> - bool DiagnoseCFAudited) {
> + bool DiagnoseCFAudited,
> + bool ConvertRHS) {
> + // If ConvertRHS is false, we want to leave the caller's RHS untouched. Sadly,
> + // we can't avoid *all* modifications at the moment, so we need some somewhere
> + // to put the updated value.
> + ExprResult LocalRHS = CallerRHS;
> + ExprResult &RHS = ConvertRHS ? CallerRHS : LocalRHS;
> +
> if (getLangOpts().CPlusPlus) {
> if (!LHSType->isRecordType() && !LHSType->isAtomicType()) {
> // C++ 5.17p3: If the left operand is not of class type, the
> @@ -7276,7 +7285,8 @@ Sema::CheckSingleAssignmentConstraints(Q
> CastKind Kind;
> CXXCastPath Path;
> CheckPointerConversion(RHS.get(), LHSType, Kind, Path, false);
> - RHS = ImpCastExprToType(RHS.get(), LHSType, Kind, VK_RValue, &Path);
> + if (ConvertRHS)
> + RHS = ImpCastExprToType(RHS.get(), LHSType, Kind, VK_RValue, &Path);
> return Compatible;
> }
>
> @@ -7287,6 +7297,7 @@ Sema::CheckSingleAssignmentConstraints(Q
> //
> // Suppress this for references: C++ 8.5.3p5.
> if (!LHSType->isReferenceType()) {
> + // FIXME: We potentially allocate here even if ConvertRHS is false.
> RHS = DefaultFunctionArrayLvalueConversion(RHS.get());
> if (RHS.isInvalid())
> return Incompatible;
> @@ -7303,7 +7314,7 @@ Sema::CheckSingleAssignmentConstraints(Q
>
> CastKind Kind = CK_Invalid;
> Sema::AssignConvertType result =
> - CheckAssignmentConstraints(LHSType, RHS, Kind);
> + CheckAssignmentConstraints(LHSType, RHS, Kind, ConvertRHS);
>
> // C99 6.5.16.1p2: The value of the right operand is converted to the
> // type of the assignment expression.
> @@ -7325,7 +7336,8 @@ Sema::CheckSingleAssignmentConstraints(Q
> return Compatible;
> }
>
> - RHS = ImpCastExprToType(E, Ty, Kind);
> + if (ConvertRHS)
> + RHS = ImpCastExprToType(E, Ty, Kind);
> }
> return result;
> }
>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=249995&r1=249994&r2=249995&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Sun Oct 11 15:13:20 2015
> @@ -130,7 +130,11 @@ ImplicitConversionRank clang::GetConvers
> ICR_Complex_Real_Conversion,
> ICR_Conversion,
> ICR_Conversion,
> - ICR_Writeback_Conversion
> + ICR_Writeback_Conversion,
> + ICR_Exact_Match, // NOTE(gbiv): This may not be completely right --
> + // it was omitted by the patch that added
> + // ICK_Zero_Event_Conversion
> + ICR_C_Conversion
> };
> return Rank[(int)Kind];
> }
> @@ -162,7 +166,9 @@ static const char* GetImplicitConversion
> "Complex-real conversion",
> "Block Pointer conversion",
> "Transparent Union Conversion",
> - "Writeback conversion"
> + "Writeback conversion",
> + "OpenCL Zero Event Conversion",
> + "C specific type conversion"
> };
> return Name[Kind];
> }
> @@ -1411,7 +1417,7 @@ static bool tryAtomicConversion(Sema &S,
> bool InOverloadResolution,
> StandardConversionSequence &SCS,
> bool CStyle);
> -
> +
> /// IsStandardConversion - Determines whether there is a standard
> /// conversion sequence (C++ [conv], C++ [over.ics.scs]) from the
> /// expression From to the type ToType. Standard conversion sequences
> @@ -1434,13 +1440,10 @@ static bool IsStandardConversion(Sema &S
> SCS.CopyConstructor = nullptr;
>
> // There are no standard conversions for class types in C++, so
> - // abort early. When overloading in C, however, we do permit
> - if (FromType->isRecordType() || ToType->isRecordType()) {
> - if (S.getLangOpts().CPlusPlus)
> - return false;
> -
> - // When we're overloading in C, we allow, as standard conversions,
> - }
> + // abort early. When overloading in C, however, we do permit them.
> + if (S.getLangOpts().CPlusPlus &&
> + (FromType->isRecordType() || ToType->isRecordType()))
> + return false;
>
> // The first conversion can be an lvalue-to-rvalue conversion,
> // array-to-pointer conversion, or function-to-pointer conversion
> @@ -1649,9 +1652,9 @@ static bool IsStandardConversion(Sema &S
> // tryAtomicConversion has updated the standard conversion sequence
> // appropriately.
> return true;
> - } else if (ToType->isEventT() &&
> + } else if (ToType->isEventT() &&
> From->isIntegerConstantExpr(S.getASTContext()) &&
> - (From->EvaluateKnownConstInt(S.getASTContext()) == 0)) {
> + From->EvaluateKnownConstInt(S.getASTContext()) == 0) {
> SCS.Second = ICK_Zero_Event_Conversion;
> FromType = ToType;
> } else {
> @@ -1690,11 +1693,28 @@ static bool IsStandardConversion(Sema &S
> }
> SCS.setToType(2, FromType);
>
> + if (CanonFrom == CanonTo)
> + return true;
> +
> // If we have not converted the argument type to the parameter type,
> - // this is a bad conversion sequence.
> - if (CanonFrom != CanonTo)
> + // this is a bad conversion sequence, unless we're resolving an overload in C.
> + if (S.getLangOpts().CPlusPlus || !InOverloadResolution)
> return false;
>
> + ExprResult ER = ExprResult{From};
> + auto Conv = S.CheckSingleAssignmentConstraints(ToType, ER,
> + /*Diagnose=*/false,
> + /*DiagnoseCFAudited=*/false,
> + /*ConvertRHS=*/false);
> + if (Conv != Sema::Compatible)
> + return false;
> +
> + SCS.setAllToTypes(ToType);
> + // We need to set all three because we want this conversion to rank terribly,
> + // and we don't know what conversions it may overlap with.
> + SCS.First = ICK_C_Only_Conversion;
> + SCS.Second = ICK_C_Only_Conversion;
> + SCS.Third = ICK_C_Only_Conversion;
> return true;
> }
>
> @@ -4993,6 +5013,7 @@ static bool CheckConvertedConstantConver
> case ICK_TransparentUnionConversion:
> case ICK_Writeback_Conversion:
> case ICK_Zero_Event_Conversion:
> + case ICK_C_Only_Conversion:
> return false;
>
> case ICK_Lvalue_To_Rvalue:
> @@ -5785,7 +5806,7 @@ ObjCMethodDecl *Sema::SelectBestMethod(S
> Match = false;
> break;
> }
> -
> +
> ImplicitConversionSequence ConversionState
> = TryCopyInitialization(*this, argExpr, param->getType(),
> /*SuppressUserConversions*/false,
>
> Modified: cfe/trunk/test/Sema/overloadable.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/overloadable.c?rev=249995&r1=249994&r2=249995&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/overloadable.c (original)
> +++ cfe/trunk/test/Sema/overloadable.c Sun Oct 11 15:13:20 2015
> @@ -85,3 +85,17 @@ void local() {
> void after_local_1(int) __attribute__((overloadable)); // expected-error {{conflicting types}}
> void after_local_2(int); // expected-error {{must have the 'overloadable' attribute}}
> void after_local_3(int) __attribute__((overloadable));
> +
> +// Make sure we allow C-specific conversions in C.
> +void conversions() {
> + void foo(char *c) __attribute__((overloadable));
> + void foo(char *c) __attribute__((overloadable, enable_if(c, "nope.jpg")));
> +
> + void *ptr;
> + foo(ptr);
> +
> + void multi_type(unsigned char *c) __attribute__((overloadable));
> + void multi_type(signed char *c) __attribute__((overloadable));
> + unsigned char *c;
> + multi_type(c);
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list