Sorry for the delayed response; one of my filters silently marked this mail as read. Looking into it now. :)
On Fri, Jan 8, 2016 at 12:38 PM, Bob Wilson <bob.wil...@apple.com> wrote: > 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@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@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits