From: Daniel Dunbar Date: Thu, 11 Sep 2008 23:12:46 +0000 (+0000) Subject: Iterate on sema for :? in Objective-C: X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5e155f0c9bd5916d47a7d99dd8d5b26bdb44d835;p=clang Iterate on sema for :? in Objective-C: - Follow C99 behavior of using other operand type when one of operands is a null pointer constant. - Fix overenthusiastic devolving of any Objective-C types to id: o If either operand has an Objective-C object type then: - If both operands are interfaces and either operand can be assigned to the other, use that type as the composite type. - Otherwise, if either type is id, use id as the composite type. - Otherwise, warn about incompatible types and use id as the composite type. - Return handling of qualified idea to separate test following general pointer type checking. o Upgraded from old code to allow devolving to id (without warning, which matches GCC). - Add test case for issues fixed above, XFAIL though because it exposed a new issue in property handling. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@56135 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 032df06c72..017e19df6c 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -1302,33 +1302,18 @@ inline QualType Sema::CheckConditionalOperands( // C99 6.5.15 } // C99 6.5.15p6 - "if one operand is a null pointer constant, the result has // the type of the other operand." - if ((lexT->isPointerType() || lexT->isBlockPointerType()) && + if ((lexT->isPointerType() || lexT->isBlockPointerType() || + Context.isObjCObjectPointerType(lexT)) && rex->isNullPointerConstant(Context)) { ImpCastExprToType(rex, lexT); // promote the null to a pointer. return lexT; } - if ((rexT->isPointerType() || rexT->isBlockPointerType()) && + if ((rexT->isPointerType() || rexT->isBlockPointerType() || + Context.isObjCObjectPointerType(rexT)) && lex->isNullPointerConstant(Context)) { ImpCastExprToType(lex, rexT); // promote the null to a pointer. return rexT; } - // Allow any Objective-C types to devolve to id type. - // FIXME: This seems to match gcc behavior, although that is very - // arguably incorrect. For example, (xxx ? (id

) : (id

)) has - // type id, which seems broken. - if (Context.isObjCObjectPointerType(lexT) && - Context.isObjCObjectPointerType(rexT)) { - // FIXME: This is not the correct composite type. This only - // happens to work because id can more or less be used anywhere, - // however this may change the type of method sends. - // FIXME: gcc adds some type-checking of the arguments and emits - // (confusing) incompatible comparison warnings in some - // cases. Investigate. - QualType compositeType = Context.getObjCIdType(); - ImpCastExprToType(lex, compositeType); - ImpCastExprToType(rex, compositeType); - return compositeType; - } // Handle the case where both operands are pointers before we handle null // pointer constants in case both operands are null pointer constants. if (const PointerType *LHSPT = lexT->getAsPointerType()) { // C99 6.5.15p3,6 @@ -1355,23 +1340,55 @@ inline QualType Sema::CheckConditionalOperands( // C99 6.5.15 return destType; } - if (!Context.typesAreCompatible(lhptee.getUnqualifiedType(), - rhptee.getUnqualifiedType())) { + QualType compositeType = lexT; + + // If either type is an Objective-C object type then check + // compatibility according to Objective-C. + if (Context.isObjCObjectPointerType(lexT) || + Context.isObjCObjectPointerType(rexT)) { + // If both operands are interfaces and either operand can be + // assigned to the other, use that type as the composite + // type. This allows + // xxx ? (A*) a : (B*) b + // where B is a subclass of A. + // + // Additionally, as for assignment, if either type is 'id' + // allow silent coercion. Finally, if the types are + // incompatible then make sure to use 'id' as the composite + // type so the result is acceptable for sending messages to. + + // FIXME: This code should not be localized to here. Also this + // should use a compatible check instead of abusing the + // canAssignObjCInterfaces code. + const ObjCInterfaceType* LHSIface = lhptee->getAsObjCInterfaceType(); + const ObjCInterfaceType* RHSIface = rhptee->getAsObjCInterfaceType(); + if (LHSIface && RHSIface && + Context.canAssignObjCInterfaces(LHSIface, RHSIface)) { + compositeType = lexT; + } else if (LHSIface && RHSIface && + Context.canAssignObjCInterfaces(LHSIface, RHSIface)) { + compositeType = rexT; + } else if (Context.isObjCIdType(lhptee) || + Context.isObjCIdType(rhptee)) { + // FIXME: This code looks wrong, because isObjCIdType checks + // the struct but getObjCIdType returns the pointer to + // struct. This is horrible and should be fixed. + compositeType = Context.getObjCIdType(); + } else { + QualType incompatTy = Context.getObjCIdType(); + ImpCastExprToType(lex, incompatTy); + ImpCastExprToType(rex, incompatTy); + return incompatTy; + } + } else if (!Context.typesAreCompatible(lhptee.getUnqualifiedType(), + rhptee.getUnqualifiedType())) { Diag(questionLoc, diag::warn_typecheck_cond_incompatible_pointers, lexT.getAsString(), rexT.getAsString(), lex->getSourceRange(), rex->getSourceRange()); - // In this situation, assume a conservative type; in general - // we assume void* type. No especially good reason, but this - // is what gcc does, and we do have to pick to get a - // consistent AST. However, if either type is an Objective-C - // object type then use id. - QualType incompatTy; - if (Context.isObjCObjectPointerType(lexT) || - Context.isObjCObjectPointerType(rexT)) { - incompatTy = Context.getObjCIdType(); - } else { - incompatTy = Context.getPointerType(Context.VoidTy); - } + // In this situation, we assume void* type. No especially good + // reason, but this is what gcc does, and we do have to pick + // to get a consistent AST. + QualType incompatTy = Context.getPointerType(Context.VoidTy); ImpCastExprToType(lex, incompatTy); ImpCastExprToType(rex, incompatTy); return incompatTy; @@ -1383,12 +1400,36 @@ inline QualType Sema::CheckConditionalOperands( // C99 6.5.15 // type. // FIXME: Need to calculate the composite type. // FIXME: Need to add qualifiers - QualType compositeType = lexT; ImpCastExprToType(lex, compositeType); ImpCastExprToType(rex, compositeType); return compositeType; } } + // Need to handle "id" explicitly. Unlike "id", whose canonical type + // evaluates to "struct objc_object *" (and is handled above when comparing + // id with statically typed objects). + if (lexT->isObjCQualifiedIdType() || rexT->isObjCQualifiedIdType()) { + // GCC allows qualified id and any Objective-C type to devolve to + // id. Currently localizing to here until clear this should be + // part of ObjCQualifiedIdTypesAreCompatible. + if (ObjCQualifiedIdTypesAreCompatible(lexT, rexT, true) || + (lexT->isObjCQualifiedIdType() && + Context.isObjCObjectPointerType(rexT)) || + (rexT->isObjCQualifiedIdType() && + Context.isObjCObjectPointerType(lexT))) { + // FIXME: This is not the correct composite type. This only + // happens to work because id can more or less be used anywhere, + // however this may change the type of method sends. + // FIXME: gcc adds some type-checking of the arguments and emits + // (confusing) incompatible comparison warnings in some + // cases. Investigate. + QualType compositeType = Context.getObjCIdType(); + ImpCastExprToType(lex, compositeType); + ImpCastExprToType(rex, compositeType); + return compositeType; + } + } + // Selection between block pointer types is ok as long as they are the same. if (lexT->isBlockPointerType() && rexT->isBlockPointerType() && Context.getCanonicalType(lexT) == Context.getCanonicalType(rexT)) diff --git a/test/SemaObjC/conditional-expr-4.m b/test/SemaObjC/conditional-expr-4.m new file mode 100644 index 0000000000..8fdcf97afd --- /dev/null +++ b/test/SemaObjC/conditional-expr-4.m @@ -0,0 +1,78 @@ +// RUN: clang -fsyntax-only %s +// XFAIL +// + +#define nil ((void*) 0) + +@interface A +@property int x; +@end + +@interface B : A +@end + +// Basic checks... +id f0(int cond, id a, void *b) { + return cond ? a : b; +} +A *f0_a(int cond, A *a, void *b) { + return cond ? a : b; +} + +id f1(int cond, id a) { + return cond ? a : nil; +} +A *f1_a(int cond, A *a) { + return cond ? a : nil; +} + +// Check interaction with qualified id + +@protocol P0 @end + +id f2(int cond, id a, void *b) { + return cond ? a : b; +} + +id f3(int cond, id a) { + return cond ? a : nil; +} + +// Check that result actually has correct type. + +// Using properties is one way to find the compiler internal type of a +// conditional expression. Simple assignment doesn't work because if +// the type is id then it can be implicitly promoted. +@protocol P1 +@property int x; +@end + +int f5(int cond, id a, id b) { + // This should result in something with id type, currently. This is + // almost certainly wrong and should be fixed. + return (cond ? a : b).x; // expected-error {{member reference base type ('id') is not a structure or union}} +} +int f5_a(int cond, A *a, A *b) { + return (cond ? a : b).x; +} +int f5_b(int cond, A *a, B *b) { + return (cond ? a : b).x; +} + +int f6(int cond, id a, void *b) { + // This should result in something with id type, currently. + return (cond ? a : b).x; // expected-error {{member reference base type ('id') is not a structure or union}} +} + +int f7(int cond, id a) { + return (cond ? a : nil).x; +} + +int f8(int cond, id a, A *b) { + // GCC regards this as a warning (comparison of distinct Objective-C types lacks a cast) + return a == b; // expected-error {{invalid operands to binary expression}} +} + +int f9(int cond, id a, A *b) { + return (cond ? a : b).x; // expected-error {{incompatible operand types}} +}