[cfe-commits] r56135 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/SemaObjC/conditional-expr-4.m

Daniel Dunbar daniel at zuster.org
Thu Sep 11 16:12:46 PDT 2008


Author: ddunbar
Date: Thu Sep 11 18:12:46 2008
New Revision: 56135

URL: http://llvm.org/viewvc/llvm-project?rev=56135&view=rev
Log:
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).
 - <rdar://problem/6212771>

Add test case for issues fixed above, XFAIL though because it exposed
a new issue in property handling.

Added:
    cfe/trunk/test/SemaObjC/conditional-expr-4.m
Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=56135&r1=56134&r2=56135&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Sep 11 18:12:46 2008
@@ -1302,33 +1302,18 @@
   }
   // 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<P>) : (id<P>)) 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 @@
         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 @@
       // 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<xx>" 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))

Added: cfe/trunk/test/SemaObjC/conditional-expr-4.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/conditional-expr-4.m?rev=56135&view=auto

==============================================================================
--- cfe/trunk/test/SemaObjC/conditional-expr-4.m (added)
+++ cfe/trunk/test/SemaObjC/conditional-expr-4.m Thu Sep 11 18:12:46 2008
@@ -0,0 +1,78 @@
+// RUN: clang -fsyntax-only %s
+// XFAIL
+// <rdar://problem/6212771>
+
+#define nil ((void*) 0)
+
+ at interface A 
+ at property int x;
+ at end
+
+ at interface B : A
+ at 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
+
+ at protocol P0 @end
+
+id f2(int cond, id<P0> a, void *b) {
+  return cond ? a : b;
+}
+
+id f3(int cond, id<P0> 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.
+ at protocol P1
+ at property int x;
+ at end
+
+int f5(int cond, id<P1> a, id<P1> 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<P1> 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<P1> a) {
+  return (cond ? a : nil).x;
+}
+
+int f8(int cond, id<P1> 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<P1> a, A *b) {
+  return (cond ? a : b).x; // expected-error {{incompatible operand types}}
+}





More information about the cfe-commits mailing list