r249995 - [Sema] Allow C conversions in C overload logic
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 12 13:51:19 PST 2016
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.wilson at 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 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160112/f3f1a7ff/attachment-0001.html>
More information about the cfe-commits
mailing list