<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 13, 2009, at 2:00 AM, Chris Lattner wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>On Jul 10, 2009, at 4:35 PM, Steve Naroff wrote:<br><blockquote type="cite">Author: snaroff<br></blockquote><blockquote type="cite">Date: Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">New Revision: 75314<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=75314&view=rev">http://llvm.org/viewvc/llvm-project?rev=75314&view=rev</a><br></blockquote><blockquote type="cite">Log:<br></blockquote><blockquote type="cite">This patch includes a conceptually simple, but very intrusive/pervasive change.<br></blockquote><br>Yay, thanks for working on this. </div></blockquote><div><br></div>No problem. Thanks a lot for taking your time to comment on a patch of this girth...much appreciated.</div><div><br><blockquote type="cite"><div>Does this add support for Class<x> and friends? If so, please add testcases.<br><br></div></blockquote><div><br></div>It provides almost all of the support. I wanted to finish adding "Class<x>" in a separate patch. When I do, I'll add test cases.</div><div><br><blockquote type="cite"><div>...<br><br><blockquote type="cite">class ObjCObjectPointerType : public Type, public llvm::FoldingSetNode {<br></blockquote><blockquote type="cite"> QualType PointeeType; // will always point to an interface type.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> // List of protocols for this protocol conforming object type<br></blockquote><blockquote type="cite"> // List is sorted on protocol name. No protocol is entered more than once.<br></blockquote><blockquote type="cite"> llvm::SmallVector<ObjCProtocolDecl*, 8> Protocols;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> ObjCObjectPointerType(QualType T, ObjCProtocolDecl **Protos, unsigned NumP) :<br></blockquote><blockquote type="cite"> Type(ObjCObjectPointer, QualType(), /*Dependent=*/false),<br></blockquote><blockquote type="cite"> PointeeType(T), Protocols(Protos, Protos+NumP) { }<br></blockquote><blockquote type="cite"> friend class ASTContext; // ASTContext creates these.<br></blockquote><blockquote type="cite"> friend class ObjCInterfaceType; // To enable 'id' and 'Class' predicates.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> static ObjCInterfaceType *IdInterfaceT;<br></blockquote><blockquote type="cite"> static ObjCInterfaceType *ClassInterfaceT;<br></blockquote><blockquote type="cite"> static void setIdInterface(QualType T) {<br></blockquote><br>Please don't use static members within the class. This is not safe when there are multiple translation units open at a time and when threading is happening. Please put state like this in ASTContext.<br><br></div></blockquote><div><br></div>If I put the state in ASTContext, then ObjCObjectPointerType::isObjCIdType() and Type::isObjCIdType() will need a pointer to it (which is what I was trying to avoid).</div><div><br></div><div>For now, I'll just make the change and live with a less convenient API. We could have a low cost union (by playing games with the low order bits of the ObjCInterfaceType), however I'd prefer not to fiddle with this for now.</div><div><br><blockquote type="cite"><div><blockquote type="cite">@@ -3198,8 +3186,30 @@<br></blockquote><blockquote type="cite">/// compatible for assignment from RHS to LHS. This handles validation of any<br></blockquote><blockquote type="cite">/// protocol qualifiers on the LHS or RHS.<br></blockquote><blockquote type="cite">///<br></blockquote><blockquote type="cite">+/// FIXME: Move the following to ObjCObjectPointerType/ObjCInterfaceType.<br></blockquote><blockquote type="cite">+bool ASTContext::canAssignObjCInterfaces(const ObjCObjectPointerType *LHSOPT,<br></blockquote><blockquote type="cite">+ const ObjCObjectPointerType *RHSOPT) {<br></blockquote><blockquote type="cite">+ // If either interface represents the built-in 'id' or 'Class' types,<br></blockquote><blockquote type="cite">+ // then return true (no need to call canAssignObjCInterfaces()).<br></blockquote><blockquote type="cite">+ if (LHSOPT->isObjCIdType() || RHSOPT->isObjCIdType() ||<br></blockquote><blockquote type="cite">+ LHSOPT->isObjCClassType() || RHSOPT->isObjCClassType())<br></blockquote><blockquote type="cite">+ return true;<br></blockquote><br>It seems common to check for "isObjCIdType || isObjCClassType" and:<br><br><blockquote type="cite">bool ASTContext::canAssignObjCInterfaces(const ObjCInterfaceType *LHS,<br></blockquote><blockquote type="cite"> const ObjCInterfaceType *RHS) {<br></blockquote><blockquote type="cite">+ // If either interface represents the built-in 'id' or 'Class' types,<br></blockquote><blockquote type="cite">+ // then return true.<br></blockquote><blockquote type="cite">+ if (LHS->isObjCIdInterface() || RHS->isObjCIdInterface() ||<br></blockquote><blockquote type="cite">+ LHS->isObjCClassInterface() || RHS->isObjCClassInterface())<br></blockquote><br><br>"isObjCIdInterface || isObjCClassInterface"<br><br>Does it make sense to add direct predicates for these?</div></blockquote><div><br></div>Sure. How about isObjCBuiltinType() and isObjCBuiltinInterface()?</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -382,7 +382,8 @@<br></blockquote><blockquote type="cite"> const Expr* SubExpr = E->getSubExpr();<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> // Check for pointer->pointer cast<br></blockquote><blockquote type="cite">- if (SubExpr->getType()->isPointerType()) {<br></blockquote><blockquote type="cite">+ if (SubExpr->getType()->isPointerType() ||<br></blockquote><blockquote type="cite">+ SubExpr->getType()->isObjCObjectPointerType()) {<br></blockquote><br>Would it make sense to add a "isAnyPointerType()" method to do both these checks?</div></blockquote><div><br></div>I don't think so. It will save a few keystrokes, however I believe the above is clearer.</div><div><br></div><div><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp Fri Jul 10 18:34:53 2009<br></blockquote><br>I'll let Ted review libanalysis and friends.<br><br></div></blockquote><div><br></div>Ted's already reviewed these (since he contributed many of libanalysis changes...).</div><div><br><blockquote type="cite"><div><br><blockquote type="cite">+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -761,8 +761,10 @@<br></blockquote><blockquote type="cite"> // Unsupported types<br></blockquote><blockquote type="cite"> return llvm::DIType();<br></blockquote><blockquote type="cite"> case Type::ObjCObjectPointer: // Encode id<p> in debug info just like id.<br></blockquote><blockquote type="cite">- return Slot = getOrCreateType(M->getContext().getObjCIdType(), Unit);<br></blockquote><blockquote type="cite">-<br></blockquote><blockquote type="cite">+ {<br></blockquote><blockquote type="cite">+ ObjCObjectPointerType *OPT = cast<ObjCObjectPointerType>(Ty);<br></blockquote><blockquote type="cite">+ return Slot = CreateType(OPT->getInterfaceType(), Unit);<br></blockquote><blockquote type="cite">+ }<br></blockquote><br>I don't think this is right. Given something like id<foo>, this should call CreateType on "id". Given NSString<foo> it should call CreateType on "NSString*", not "NSString".<br><br></div></blockquote><div><br></div>I'll take a look.</div><div><br><blockquote type="cite"><div><blockquote type="cite">+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -987,7 +987,7 @@<br></blockquote><blockquote type="cite">}<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &Ops) {<br></blockquote><blockquote type="cite">- if (!Ops.Ty->isPointerType()) {<br></blockquote><blockquote type="cite">+ if (!Ops.Ty->isPointerType() && !Ops.Ty->isObjCObjectPointerType()) {<br></blockquote><br>Another candidate for "isAnyPointerType"? What happens when you add an objc pointer to an integer? </div></blockquote><div><br></div>You can't have an objc pointer to an integer (that's why I called the class ObjCObjectPointerType)....an ObjCObjectPointerType can only refer to an interface (built-in or user-defined).</div><div><br></div><div>Maybe I don't understand your question.</div><div><br><blockquote type="cite"><div>Isn't it the same as adding to a pointer to struct? It seems that this code could be merged now.<br><br><blockquote type="cite">+++ cfe/trunk/lib/CodeGen/CodeGenTypes.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -215,6 +215,7 @@<br></blockquote><blockquote type="cite">const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) {<br></blockquote><blockquote type="cite"> const clang::Type &Ty = *Context.getCanonicalType(T);<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">+ //T->dump();<br></blockquote><br>Please remove.</div></blockquote><div><br></div>Sure.</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite"> switch (Ty.getTypeClass()) {<br></blockquote><blockquote type="cite">#define TYPE(Class, Base)<br></blockquote><blockquote type="cite">#define ABSTRACT_TYPE(Class, Base)<br></blockquote><blockquote type="cite">@@ -353,10 +354,14 @@<br></blockquote><blockquote type="cite"> return T;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">- case Type::ObjCObjectPointer:<br></blockquote><blockquote type="cite">- // Protocols don't influence the LLVM type.<br></blockquote><blockquote type="cite">- return ConvertTypeRecursive(Context.getObjCIdType());<br></blockquote><blockquote type="cite">-<br></blockquote><blockquote type="cite">+ case Type::ObjCObjectPointer: {<br></blockquote><blockquote type="cite">+ // Qualified id types don't influence the LLVM type, here we always return<br></blockquote><blockquote type="cite">+ // an opaque type for 'id'.<br></blockquote><blockquote type="cite">+ const llvm::Type *&T = InterfaceTypes[0];<br></blockquote><blockquote type="cite">+ if (!T)<br></blockquote><blockquote type="cite">+ T = llvm::OpaqueType::get();<br></blockquote><blockquote type="cite">+ return llvm::PointerType::getUnqual(T);<br></blockquote><blockquote type="cite">+ }<br></blockquote><br>This doesn't seem like the right thing, can you check with Daniel on this?</div></blockquote><div><br></div>Daniel suggested this solution to me (so I guess I've already checked:-)</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -527,13 +525,15 @@<br></blockquote><blockquote type="cite"> case 2:<br></blockquote><blockquote type="cite"> if (!TypeID->isStr("id"))<br></blockquote><blockquote type="cite"> break;<br></blockquote><blockquote type="cite">- Context.setObjCIdType(Context.getTypeDeclType(New));<br></blockquote><blockquote type="cite">- objc_types = true;<br></blockquote><blockquote type="cite">+ // Install the built-in type for 'id', ignoring the current definition.<br></blockquote><blockquote type="cite">+ New->setTypeForDecl(Context.getObjCIdType().getTypePtr());<br></blockquote><blockquote type="cite">+ return;<br></blockquote><blockquote type="cite"> break;<br></blockquote><br>Please remove the dead break.</div></blockquote><div><br></div>Will do.</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -1844,6 +1844,17 @@<br></blockquote><blockquote type="cite"> BaseExpr = RHSExp;<br></blockquote><blockquote type="cite"> IndexExpr = LHSExp;<br></blockquote><blockquote type="cite"> ResultType = PTy->getPointeeType();<br></blockquote><blockquote type="cite">+ } else if (const ObjCObjectPointerType *PTy =<br></blockquote><blockquote type="cite">+ LHSTy->getAsObjCObjectPointerType()) {<br></blockquote><blockquote type="cite">+ BaseExpr = LHSExp;<br></blockquote><blockquote type="cite">+ IndexExpr = RHSExp;<br></blockquote><blockquote type="cite">+ ResultType = PTy->getPointeeType();<br></blockquote><blockquote type="cite">+ } else if (const ObjCObjectPointerType *PTy =<br></blockquote><blockquote type="cite">+ RHSTy->getAsObjCObjectPointerType()) {<br></blockquote><blockquote type="cite">+ // Handle the uncommon case of "123[Ptr]".<br></blockquote><blockquote type="cite">+ BaseExpr = RHSExp;<br></blockquote><blockquote type="cite">+ IndexExpr = LHSExp;<br></blockquote><blockquote type="cite">+ ResultType = PTy->getPointeeType();<br></blockquote><br>It seems that the objc and normal pointer type can be merged with your new "getpointeetype" method.<br><br></div></blockquote><div><br></div>Makes sense. Note: This is an example of a cleanup that I wanted to defer (for this mega patch).</div><div><br></div><div><blockquote type="cite"><div><blockquote type="cite">@@ -2212,12 +2225,71 @@<br></blockquote><blockquote type="cite"> << DeclarationName(&Member) << int(OpKind == tok::arrow));<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">+ // Handle properties on ObjC 'Class' types.<br></blockquote><blockquote type="cite">+ if (OpKind == tok::period && (BaseType->isObjCClassType())) {<br></blockquote><br>It's minor, but no need for the redundant parens here.<br><br></div></blockquote><div><br></div>O.k.</div><div><br><blockquote type="cite"><div><br><blockquote type="cite">+ // Also must look for a getter name which uses property syntax.<br></blockquote><blockquote type="cite">+ Selector Sel = PP.getSelectorTable().getNullarySelector(&Member);<br></blockquote><blockquote type="cite">+ if (ObjCMethodDecl *MD = getCurMethodDecl()) {<br></blockquote><blockquote type="cite">+ ObjCInterfaceDecl *IFace = MD->getClassInterface();<br></blockquote><blockquote type="cite">+ ObjCMethodDecl *Getter;<br></blockquote><blockquote type="cite">+ // FIXME: need to also look locally in the implementation.<br></blockquote><blockquote type="cite">+ if ((Getter = IFace->lookupClassMethod(Sel))) {<br></blockquote><blockquote type="cite">+ // Check the use of this method.<br></blockquote><blockquote type="cite">+ if (DiagnoseUseOfDecl(Getter, MemberLoc))<br></blockquote><blockquote type="cite">+ return ExprError();<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ // If we found a getter then this may be a valid dot-reference, we<br></blockquote><blockquote type="cite">+ // will look for the matching setter, in case it is needed.<br></blockquote><blockquote type="cite">+ Selector SetterSel =<br></blockquote><blockquote type="cite">+ SelectorTable::constructSetterName(PP.getIdentifierTable(),<br></blockquote><blockquote type="cite">+ PP.getSelectorTable(), &Member);<br></blockquote><br><br><blockquote type="cite">+ ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);<br></blockquote><blockquote type="cite">+ if (!Setter) {<br></blockquote><blockquote type="cite">+ // If this reference is in an @implementation, also check for 'private'<br></blockquote><blockquote type="cite">+ // methods.<br></blockquote><blockquote type="cite">+ Setter = FindMethodInNestedImplementations(IFace, SetterSel);<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ // Look through local category implementations associated with the class.<br></blockquote><blockquote type="cite">+ if (!Setter) {<br></blockquote><blockquote type="cite">+ for (unsigned i = 0; i < ObjCCategoryImpls.size() && !Setter; i++) {<br></blockquote><blockquote type="cite">+ if (ObjCCategoryImpls[i]->getClassInterface() == IFace)<br></blockquote><blockquote type="cite">+ Setter = ObjCCategoryImpls[i]->getClassMethod(SetterSel);<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ }<br></blockquote><br>This code looks like it is something that should be factored out into a helper method. Are there other pieces of code doing the same lookup algorithm?<br><br></div></blockquote><div><br></div>Yep. I agree a helper would be nice. Since this doesn't really relate to the ObjCObjectPointerType related changes, I'll note this as a separate cleanup (the only reason in showed up in the patch is I move the code).</div><div><br><blockquote type="cite"><div><blockquote type="cite"> // Handle properties on qualified "id" protocols.<br></blockquote><blockquote type="cite">+ const ObjCObjectPointerType *QIdTy;<br></blockquote><blockquote type="cite">+ if (OpKind == tok::period && (QIdTy = BaseType->getAsObjCQualifiedIdType())) {<br></blockquote><br>Can this be merged with the non-qualified case?</div></blockquote><div><br></div>Possibly...will need to look deeper. </div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">@@ -3517,6 +3523,29 @@<br></blockquote><blockquote type="cite"> return Incompatible;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">+ if (isa<ObjCObjectPointerType>(lhsType)) {<br></blockquote><blockquote type="cite">+ if (rhsType->isIntegerType())<br></blockquote><blockquote type="cite">+ return IntToPointer;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ if (isa<PointerType>(rhsType)) {<br></blockquote><blockquote type="cite">+ QualType lhptee = lhsType->getAsObjCObjectPointerType()->getPointeeType();<br></blockquote><blockquote type="cite">+ QualType rhptee = rhsType->getAsPointerType()->getPointeeType();<br></blockquote><blockquote type="cite">+ return CheckPointeeTypesForAssignment(lhptee, rhptee);<br></blockquote><br>Why not use lhsType->getPointeeType() (and also for rhsType)?</div></blockquote><div><br></div>Since the routine is already doing the "isa" sniffing to determine what types we have, I didn't think it would be any clearer (or efficient).</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ if (rhsType->isObjCObjectPointerType()) {<br></blockquote><blockquote type="cite">+ QualType lhptee = lhsType->getAsObjCObjectPointerType()->getPointeeType();<br></blockquote><blockquote type="cite">+ QualType rhptee = rhsType->getAsObjCObjectPointerType()->getPointeeType();<br></blockquote><blockquote type="cite">+ return CheckPointeeTypesForAssignment(lhptee, rhptee);<br></blockquote><br>That would allow merging this case in as well.</div></blockquote><div><br></div>Same response as above.</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">@@ -3776,12 +3823,23 @@<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> // Put any potential pointer into PExp<br></blockquote><blockquote type="cite"> Expr* PExp = lex, *IExp = rex;<br></blockquote><blockquote type="cite">- if (IExp->getType()->isPointerType())<br></blockquote><blockquote type="cite">+ if (IExp->getType()->isPointerType() ||<br></blockquote><blockquote type="cite">+ IExp->getType()->isObjCObjectPointerType())<br></blockquote><br>->isAnyPointerType()</div></blockquote><div><br></div>As a said earlier, I'm not convinced adding another predicate would make the code clearer (though it would definitely make it more terse).</div><div><br></div><div>Maybe a better name would sway my opinion. isCOrObjCPointer() is more descriptive, however one of the uglier names I've ever seen:-)</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite"> std::swap(PExp, IExp);<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">- if (const PointerType *PTy = PExp->getType()->getAsPointerType()) {<br></blockquote><blockquote type="cite">+ if (PExp->getType()->isPointerType() ||<br></blockquote><blockquote type="cite">+ PExp->getType()->isObjCObjectPointerType()) {<br></blockquote><br>again.<br><br><blockquote type="cite">+<br></blockquote><blockquote type="cite"> if (IExp->getType()->isIntegerType()) {<br></blockquote><blockquote type="cite">- QualType PointeeTy = PTy->getPointeeType();<br></blockquote><blockquote type="cite">+ QualType PointeeTy;<br></blockquote><blockquote type="cite">+ const PointerType *PTy;<br></blockquote><blockquote type="cite">+ const ObjCObjectPointerType *OPT;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ if ((PTy = PExp->getType()->getAsPointerType()))<br></blockquote><blockquote type="cite">+ PointeeTy = PTy->getPointeeType();<br></blockquote><blockquote type="cite">+ else if ((OPT = PExp->getType()->getAsObjCObjectPointerType()))<br></blockquote><blockquote type="cite">+ PointeeTy = OPT->getPointeeType();<br></blockquote><br>This should be able to use QualType::getPointeeType() instead of the if/elseif</div></blockquote><div><br></div>This isn't possible without more code reorganization (I already tried it). The code that follows is interested in both PTy and OPT (not only the pointee type). </div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">@@ -3855,10 +3913,16 @@<br></blockquote><blockquote type="cite"> if (CompLHSTy) *CompLHSTy = compType;<br></blockquote><blockquote type="cite"> return compType;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite">-<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite"> // Either ptr - int or ptr - ptr.<br></blockquote><blockquote type="cite">- if (const PointerType *LHSPTy = lex->getType()->getAsPointerType()) {<br></blockquote><blockquote type="cite">- QualType lpointee = LHSPTy->getPointeeType();<br></blockquote><blockquote type="cite">+ if (lex->getType()->isPointerType() ||<br></blockquote><blockquote type="cite">+ lex->getType()->isObjCObjectPointerType()) {<br></blockquote><br>isAnyPointerType()</div></blockquote><div><br></div>You really like that predicate:-)</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+ QualType lpointee;<br></blockquote><blockquote type="cite">+ if (const PointerType *LHSPTy = lex->getType()->getAsPointerType())<br></blockquote><blockquote type="cite">+ lpointee = LHSPTy->getPointeeType();<br></blockquote><blockquote type="cite">+ else if (const ObjCObjectPointerType *OPT =<br></blockquote><blockquote type="cite">+ lex->getType()->getAsObjCObjectPointerType())<br></blockquote><blockquote type="cite">+ lpointee = OPT->getPointeeType();<br></blockquote><br>lex->getType()->getPointeeType()</div></blockquote><div><br></div>This one does look like a candidate (good catch).</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">@@ -4226,19 +4289,27 @@<br></blockquote><blockquote type="cite"> ImpCastExprToType(rex, lType);<br></blockquote><blockquote type="cite"> return ResultTy;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite">+ if (lType->isObjCObjectPointerType() && rType->isObjCObjectPointerType()) {<br></blockquote><blockquote type="cite">+ if (!Context.areComparableObjCPointerTypes(lType, rType)) {<br></blockquote><blockquote type="cite">+ Diag(Loc, diag::ext_typecheck_comparison_of_distinct_pointers)<br></blockquote><blockquote type="cite"> << lType << rType << lex->getSourceRange() << rex->getSourceRange();<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite">+ if (lType->isObjCQualifiedIdType() && rType->isObjCQualifiedIdType()) {<br></blockquote><blockquote type="cite">+ if (ObjCQualifiedIdTypesAreCompatible(lType, rType, true)) {<br></blockquote><blockquote type="cite">+ ImpCastExprToType(rex, lType);<br></blockquote><blockquote type="cite">+ return ResultTy;<br></blockquote><blockquote type="cite">+ } else {<br></blockquote><blockquote type="cite">+ Diag(Loc, diag::warn_incompatible_qualified_id_operands)<br></blockquote><blockquote type="cite">+ << lType << rType << lex->getSourceRange() << rex->getSourceRange();<br></blockquote><blockquote type="cite">+ ImpCastExprToType(rex, lType);<br></blockquote><blockquote type="cite">+ return ResultTy;<br></blockquote><br>This else case can be unnested and can fall through to share the ImpCastExprToType + return.<br><br><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ ImpCastExprToType(rex, lType);<br></blockquote><blockquote type="cite">+ return ResultTy;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"> }<br></blockquote><br><blockquote type="cite">- if ((lType->isPointerType() || lType->isObjCQualifiedIdType()) &&<br></blockquote><blockquote type="cite">+ if ((lType->isPointerType() || lType->isObjCObjectPointerType()) &&<br></blockquote><br>isAnyPointerType()<br><br><blockquote type="cite">@@ -4250,7 +4321,7 @@<br></blockquote><blockquote type="cite"> return ResultTy;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"> if (lType->isIntegerType() &&<br></blockquote><blockquote type="cite">- (rType->isPointerType() || rType->isObjCQualifiedIdType())) {<br></blockquote><blockquote type="cite">+ (rType->isPointerType() || rType->isObjCObjectPointerType())) {<br></blockquote><br>isAnyPointerType()<br><br><blockquote type="cite">@@ -4524,9 +4594,17 @@<br></blockquote><blockquote type="cite"> Diag(OpLoc, diag::warn_increment_bool) << Op->getSourceRange();<br></blockquote><blockquote type="cite"> } else if (ResType->isRealType()) {<br></blockquote><blockquote type="cite"> // OK!<br></blockquote><blockquote type="cite">- } else if (const PointerType *PT = ResType->getAsPointerType()) {<br></blockquote><blockquote type="cite">+ } else if (ResType->getAsPointerType() ||ResType->isObjCObjectPointerType()) {<br></blockquote><br>isAnyPointerType(), please don't use "getAs" in a bool context.<br><br><blockquote type="cite">+ QualType PointeeTy;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ if (const PointerType *PTy = ResType->getAsPointerType())<br></blockquote><blockquote type="cite">+ PointeeTy = PTy->getPointeeType();<br></blockquote><blockquote type="cite">+ else if (const ObjCObjectPointerType *OPT =<br></blockquote><blockquote type="cite">+ ResType->getAsObjCObjectPointerType())<br></blockquote><blockquote type="cite">+ PointeeTy = OPT->getPointeeType();<br></blockquote><br>QualType::getPointeeType().<br><br><blockquote type="cite">+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -1482,7 +1482,8 @@<br></blockquote><blockquote type="cite">QualType Sema::FindCompositePointerType(Expr *&E1, Expr *&E2) {<br></blockquote><blockquote type="cite"> assert(getLangOptions().CPlusPlus && "This function assumes C++");<br></blockquote><blockquote type="cite"> QualType T1 = E1->getType(), T2 = E2->getType();<br></blockquote><blockquote type="cite">- if(!T1->isPointerType() && !T2->isPointerType())<br></blockquote><blockquote type="cite">+ if(!T1->isPointerType() && !T2->isPointerType() &&<br></blockquote><blockquote type="cite">+ !T1->isObjCObjectPointerType() && !T2->isObjCObjectPointerType())<br></blockquote><br>isAnyPointerType()<br><br><blockquote type="cite">+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Jul 10 18:34:53 2009<br></blockquote><br>wow, nice cleanups!<br><br><blockquote type="cite"><br></blockquote><blockquote type="cite">+ if (rhsOPT->qual_empty()) {<br></blockquote><blockquote type="cite">+ // If the RHS is a unqualified interface pointer "NSString*",<br></blockquote><blockquote type="cite">+ // make sure we check the class hierarchy.<br></blockquote><blockquote type="cite">+ if (ObjCInterfaceDecl *rhsID = rhsOPT->getInterfaceDecl()) {<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator I = lhsQID->qual_begin(),<br></blockquote><blockquote type="cite">+ E = lhsQID->qual_end(); I != E; ++I) {<br></blockquote><blockquote type="cite">+ // when comparing an id<P> on lhs with a static type on rhs,<br></blockquote><blockquote type="cite">+ // see if static class implements all of id's protocols, directly or<br></blockquote><blockquote type="cite">+ // through its super class and categories.<br></blockquote><blockquote type="cite">+ if (!ClassImplementsProtocol(*I, rhsID, true))<br></blockquote><blockquote type="cite">+ return false;<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ return true;<br></blockquote><br>This return can be removed.</div></blockquote><div><br></div>O.K.</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ // If there are no qualifiers and no interface, we have an 'id'.<br></blockquote><blockquote type="cite">+ return true;<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ // Both the right and left sides have qualifiers.<br></blockquote><blockquote type="cite"> for (ObjCObjectPointerType::qual_iterator I = lhsQID->qual_begin(),<br></blockquote><blockquote type="cite"> E = lhsQID->qual_end(); I != E; ++I) {<br></blockquote><blockquote type="cite"> ObjCProtocolDecl *lhsProto = *I;<br></blockquote><blockquote type="cite">@@ -803,28 +779,26 @@<br></blockquote><blockquote type="cite"> // when comparing an id<P> on lhs with a static type on rhs,<br></blockquote><blockquote type="cite"> // see if static class implements all of id's protocols, directly or<br></blockquote><blockquote type="cite"> // through its super class and categories.<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator J = rhsOPT->qual_begin(),<br></blockquote><blockquote type="cite">+ E = rhsOPT->qual_end(); J != E; ++J) {<br></blockquote><blockquote type="cite">+ ObjCProtocolDecl *rhsProto = *J;<br></blockquote><blockquote type="cite"> if (ProtocolCompatibleWithProtocol(lhsProto, rhsProto) ||<br></blockquote><blockquote type="cite"> (compare && ProtocolCompatibleWithProtocol(rhsProto, lhsProto))) {<br></blockquote><blockquote type="cite"> match = true;<br></blockquote><blockquote type="cite"> break;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite">+ // If the RHS is a qualified interface pointer "NSString<P>*",<br></blockquote><blockquote type="cite">+ // make sure we check the class hierarchy.<br></blockquote><blockquote type="cite">+ if (ObjCInterfaceDecl *rhsID = rhsOPT->getInterfaceDecl()) {<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator I = lhsQID->qual_begin(),<br></blockquote><blockquote type="cite">+ E = lhsQID->qual_end(); I != E; ++I) {<br></blockquote><blockquote type="cite">+ // when comparing an id<P> on lhs with a static type on rhs,<br></blockquote><blockquote type="cite">+ // see if static class implements all of id's protocols, directly or<br></blockquote><blockquote type="cite">+ // through its super class and categories.<br></blockquote><blockquote type="cite">+ if (ClassImplementsProtocol(*I, rhsID, true)) {<br></blockquote><blockquote type="cite">+ match = true;<br></blockquote><blockquote type="cite">+ break;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"> }<br></blockquote><br>Does this loop exist elsewhere? If so, please factor out to a helper function.</div></blockquote><div><br></div>I'll look...</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite"> }<br></blockquote><blockquote type="cite">@@ -837,7 +811,52 @@<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> const ObjCObjectPointerType *rhsQID = rhs->getAsObjCQualifiedIdType();<br></blockquote><blockquote type="cite"> assert(rhsQID && "One of the LHS/RHS should be id<x>");<br></blockquote><blockquote type="cite">-<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ if (const ObjCObjectPointerType *lhsOPT =<br></blockquote><blockquote type="cite">+ lhs->getAsObjCInterfacePointerType()) {<br></blockquote><blockquote type="cite">+ if (lhsOPT->qual_empty()) {<br></blockquote><blockquote type="cite">+ bool match = false;<br></blockquote><blockquote type="cite">+ if (ObjCInterfaceDecl *lhsID = lhsOPT->getInterfaceDecl()) {<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator I = rhsQID->qual_begin(),<br></blockquote><blockquote type="cite">+ E = rhsQID->qual_end(); I != E; ++I) {<br></blockquote><blockquote type="cite">+ // when comparing an id<P> on lhs with a static type on rhs,<br></blockquote><blockquote type="cite">+ // see if static class implements all of id's protocols, directly or<br></blockquote><blockquote type="cite">+ // through its super class and categories.<br></blockquote><blockquote type="cite">+ if (ClassImplementsProtocol(*I, lhsID, true)) {<br></blockquote><blockquote type="cite">+ match = true;<br></blockquote><blockquote type="cite">+ break;<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ if (!match)<br></blockquote><blockquote type="cite">+ return false;<br></blockquote><br>Please factor the inner loop out to a helper function. This allows you to use early return instead of the 'match' bool + break.<br><br><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ return true;<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+ // Both the right and left sides have qualifiers.<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator I = lhsOPT->qual_begin(),<br></blockquote><blockquote type="cite">+ E = lhsOPT->qual_end(); I != E; ++I) {<br></blockquote><blockquote type="cite">+ ObjCProtocolDecl *lhsProto = *I;<br></blockquote><blockquote type="cite">+ bool match = false;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ // when comparing an id<P> on lhs with a static type on rhs,<br></blockquote><blockquote type="cite">+ // see if static class implements all of id's protocols, directly or<br></blockquote><blockquote type="cite">+ // through its super class and categories.<br></blockquote><blockquote type="cite">+ for (ObjCObjectPointerType::qual_iterator J = rhsQID->qual_begin(),<br></blockquote><blockquote type="cite">+ E = rhsQID->qual_end(); J != E; ++J) {<br></blockquote><blockquote type="cite">+ ObjCProtocolDecl *rhsProto = *J;<br></blockquote><blockquote type="cite">+ if (ProtocolCompatibleWithProtocol(lhsProto, rhsProto) ||<br></blockquote><blockquote type="cite">+ (compare && ProtocolCompatibleWithProtocol(rhsProto, lhsProto))) {<br></blockquote><blockquote type="cite">+ match = true;<br></blockquote><blockquote type="cite">+ break;<br></blockquote><br>This seems very similar to the above loops, it would be nice to commonize them somehow.<br><br><blockquote type="cite">+++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Jul 10 18:34:53 2009<br></blockquote><br>Please ask Doug to review this.</div></blockquote><div><br></div>Sure...Doug and I already </div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+ // Beyond this point, both types need to be C pointers or block pointers.<br></blockquote><blockquote type="cite"> QualType ToPointeeType;<br></blockquote><blockquote type="cite">- const PointerType* ToTypePtr = ToType->getAsPointerType();<br></blockquote><blockquote type="cite">- if (ToTypePtr)<br></blockquote><blockquote type="cite">- ToPointeeType = ToTypePtr->getPointeeType();<br></blockquote><blockquote type="cite">+ if (const PointerType *ToCPtr = ToType->getAsPointerType())<br></blockquote><blockquote type="cite">+ ToPointeeType = ToCPtr->getPointeeType();<br></blockquote><blockquote type="cite"> else if (const BlockPointerType *ToBlockPtr = ToType->getAsBlockPointerType())<br></blockquote><blockquote type="cite"> ToPointeeType = ToBlockPtr->getPointeeType();<br></blockquote><blockquote type="cite"> else<br></blockquote><blockquote type="cite"> return false;<br></blockquote><br>These can use Type::getPointeeType(). Incidentally, please update the comment on Type::getPointeeType() to fix it to say that it works on block pointers. It does, but the comment says it doesn't.<br><br></div></blockquote><div><br></div>Will do.</div><div><br><blockquote type="cite"><div><blockquote type="cite"><br></blockquote><blockquote type="cite"> QualType FromPointeeType;<br></blockquote><blockquote type="cite">- const PointerType *FromTypePtr = FromType->getAsPointerType();<br></blockquote><blockquote type="cite">- if (FromTypePtr)<br></blockquote><blockquote type="cite">- FromPointeeType = FromTypePtr->getPointeeType();<br></blockquote><blockquote type="cite">- else if (const BlockPointerType *FromBlockPtr<br></blockquote><blockquote type="cite">- = FromType->getAsBlockPointerType())<br></blockquote><blockquote type="cite">+ if (const PointerType *FromCPtr = FromType->getAsPointerType())<br></blockquote><blockquote type="cite">+ FromPointeeType = FromCPtr->getPointeeType();<br></blockquote><blockquote type="cite">+ else if (const BlockPointerType *FromBlockPtr = FromType->getAsBlockPointerType())<br></blockquote><blockquote type="cite"> FromPointeeType = FromBlockPtr->getPointeeType();<br></blockquote><blockquote type="cite"> else<br></blockquote><blockquote type="cite"> return false;<br></blockquote><br>These can use Type::getPointeeType().<br><br><br><blockquote type="cite">+ if (const ObjCObjectPointerType *FromPtrType =<br></blockquote><blockquote type="cite">+ FromType->getAsObjCObjectPointerType())<br></blockquote><blockquote type="cite">+ if (const ObjCObjectPointerType *ToPtrType =<br></blockquote><blockquote type="cite">+ ToType->getAsObjCObjectPointerType()) {<br></blockquote><blockquote type="cite">+ // Objective-C++ conversions are always okay.<br></blockquote><blockquote type="cite">+ // FIXME: We should have a different class of conversions for the<br></blockquote><blockquote type="cite">+ // Objective-C++ implicit conversions.<br></blockquote><blockquote type="cite">+ if (FromPtrType->isObjCIdType() || ToPtrType->isObjCIdType() ||<br></blockquote><blockquote type="cite">+ FromPtrType->isObjCClassType() || ToPtrType->isObjCClassType())<br></blockquote><br>Can use some sort of merged predicate for isObjCIdType || isObjCClassType.<br><br><blockquote type="cite"><br></blockquote><blockquote type="cite">+++ cfe/trunk/test/CodeGenObjC/encode-test.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -1,7 +1,7 @@<br></blockquote><blockquote type="cite">// RUN: clang-cc -triple=i686-apple-darwin9 -fnext-runtime -emit-llvm -o %t %s &&<br></blockquote><blockquote type="cite">// RUN: grep -e "\^{Innermost=CC}" %t | count 1 &&<br></blockquote><blockquote type="cite">-// RUN: grep -e "{Derived=#ib32b8b3b8sb16b8b8b2b8ccb6}" %t | count 1 &&<br></blockquote><blockquote type="cite">-// RUN: grep -e "{B1=#@c}" %t | count 1 &&<br></blockquote><blockquote type="cite">+// RUN: grep -e "{Derived=^{objc_class}ib32b8b3b8sb16b8b8b2b8ccb6}" %t | count 1 &&<br></blockquote><blockquote type="cite">+// RUN: grep -e "{B1=^{objc_class}@c}" %t | count 1 &&<br></blockquote><blockquote type="cite">// RUN: grep -e "v12@0:4\[3\[4@]]8" %t | count 1 &&<br></blockquote><blockquote type="cite">// RUN: grep -e "r\^{S=i}" %t | count 1 &&<br></blockquote><blockquote type="cite">// RUN: grep -e "\^{Object=#}" %t | count 1<br></blockquote><br>The output of @encode changed? Isn't this an ABI bug??</div></blockquote><div><br></div><div>This following two issues are a result of moving away from the C-structure dependency (which is a hack). More info...</div><div><br></div><div>We no longer treat "struct objc_class *" as synonymous with "Class". This is a side-effect of removing "ASTContext::isObjCClassStructType(T)".</div><div><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(170, 13, 145); ">@interface<span style="color: #000000"> B1 </span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">{</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">struct</span> objc_class *isa;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> Int1 *sBase;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">char</span> <span style="color: #3f6e74">c</span>;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">}</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(170, 13, 145); ">@end</div><div><font class="Apple-style-span" color="#AA0D91" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#AA0D91" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><span class="Apple-style-span" style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; "><div>Since it does effect GCC binary compatibility, we could certainly add back a hack to make them synonymous.</div><div><br></div></span></span></font></div><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/test/CodeGenObjC/overloadable.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -3,8 +3,8 @@<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">@class C;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">-// RUN: grep _Z1fP11objc_object %t | count 1 &&<br></blockquote><blockquote type="cite">-void __attribute__((overloadable)) f(C *c) { }<br></blockquote><blockquote type="cite">+// RUN: grep _Z1fP2id %t | count 1 &&<br></blockquote><blockquote type="cite">+void __attribute__((overloadable)) f(id c) { }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">// RUN: grep _Z1fP1C %t | count 1<br></blockquote><blockquote type="cite">-void __attribute__((overloadable)) f(id c) { }<br></blockquote><blockquote type="cite">+void __attribute__((overloadable)) f(C *c) { }<br></blockquote><br>Likewise, mangling changing sounds like a serious ABI bug.</div></blockquote><div><br></div>Same issue as above (but with ASTContext::isObjCIdStructType()). We now mangle "id" as "id" (not the underlying structure).</div><div><br></div><div><div>Since it does effect GCC binary compatibility, we could certainly add back a hack to make them synonymous.</div><div><br></div><blockquote type="cite"><div><br><br><blockquote type="cite">==============================================================================<br></blockquote><blockquote type="cite">--- cfe/trunk/test/PCH/objc_exprs.m (original)<br></blockquote><blockquote type="cite">+++ cfe/trunk/test/PCH/objc_exprs.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -6,7 +6,7 @@<br></blockquote><blockquote type="cite">// RUN: clang-cc -fblocks -include-pch %t -fsyntax-only -verify %s<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">// Expressions<br></blockquote><blockquote type="cite">-int *A1 = (objc_string)0; // expected-warning {{'struct objc_object *'}}<br></blockquote><blockquote type="cite">+int *A1 = (objc_string)0; // expected-warning {{aka 'id'}}<br></blockquote><br>Nice.<br><br><blockquote type="cite">+++ cfe/trunk/test/SemaObjC/comptypes-5.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -26,8 +26,8 @@<br></blockquote><blockquote type="cite"> MyOtherClass<MyProtocol> *obj_c_super_p_q = nil;<br></blockquote><blockquote type="cite"> MyClass<MyProtocol> *obj_c_cat_p_q = nil;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">- obj_c_cat_p = obj_id_p; // expected-warning {{incompatible type assigning 'id<MyProtocol>', expected 'MyClass *'}}<br></blockquote><blockquote type="cite">- obj_c_super_p = obj_id_p; // expected-warning {{incompatible type assigning 'id<MyProtocol>', expected 'MyOtherClass *'}}<br></blockquote><blockquote type="cite">+ obj_c_cat_p = obj_id_p;<br></blockquote><blockquote type="cite">+ obj_c_super_p = obj_id_p;<br></blockquote><br>Is this supposed to be allowed?</div></blockquote><div><br></div>GCC warns...</div><div><br></div><div><div>test/SemaObjC/comptypes-5.m:29: warning: assignment from distinct Objective-C type</div><div>test/SemaObjC/comptypes-5.m:30: warning: assignment from distinct Objective-C type</div><div><br></div><div>I decided to allow it. Rationale: Both MyClass and MyOtherClass implement MyProtocol. Since the protocols "match", and you can assign any 'id' to an interface type (without warning), I decided to allow it. I'm happy to put back the warning if others feel strongly (Fariborz?).</div><div><br></div><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/test/SemaObjC/conditional-expr-3.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -59,7 +59,7 @@<br></blockquote><blockquote type="cite">}<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">void f10(int cond, id<P0,P1> x0, id<P0,P2> x1) {<br></blockquote><blockquote type="cite">- barP2(cond ? x0 : x1);<br></blockquote><blockquote type="cite">+ barP2(cond ? x0 : x1); // expected-warning {{incompatible type passing 'id<P0,P1>', expected 'id<P2>'}}<br></blockquote><br>Shouldn't the merged type of ?: be id<P0> here, not id<P0,P1> ?</div></blockquote><div><br></div>Need to research...</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/test/SemaObjC/message.m Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -95,6 +95,6 @@<br></blockquote><blockquote type="cite">void foo4() {<br></blockquote><blockquote type="cite"> struct objc_object X[10];<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">- [X rect];<br></blockquote><blockquote type="cite">+ [(id)X rect];<br></blockquote><blockquote type="cite">}<br></blockquote><br>This is a bug, we should be performing unary "array -> pointer" decay here. This was in response to a bugzilla, we need to support this.</div></blockquote><div><br></div><div>I don't see the issue here. Both GCC and clang warn if no cast is used.</div><div><br></div><div>[steve-naroffs-imac-3:~/llvm/tools/clang] snaroff% cc -c xx.m</div><div>xx.m: In function ‘foo4’:</div><div>xx.m:29: warning: invalid receiver type ‘objc_object [10]’</div><div><br></div><div>[steve-naroffs-imac-3:~/llvm/tools/clang] snaroff% ../../Debug/bin/clang -c xx.m</div><div>xx.m:29:3: warning: receiver type 'struct objc_object *' is not 'id' or interface pointer, consider casting it to 'id'</div><div> [X rect];</div><div> ^~</div><div>xx.m:29:3: warning: method '-rect' not found (return type defaults to 'id')</div><div> [X rect];</div><div> ^~~~~~~~</div><div>2 diagnostics generated.</div><div><br></div><blockquote type="cite"><div><br><br><blockquote type="cite">+++ cfe/trunk/test/SemaObjCXX/overload.mm Fri Jul 10 18:34:53 2009<br></blockquote><blockquote type="cite">@@ -1,4 +1,5 @@<br></blockquote><blockquote type="cite">// RUN: clang-cc -fsyntax-only -verify %s<br></blockquote><blockquote type="cite">+// XFAIL<br></blockquote><blockquote type="cite">@interface Foo<br></blockquote><blockquote type="cite">@end<br></blockquote><br>What is the problem with this test?</div></blockquote><div><br></div>There were several problems (mostly related in incomplete handling of ObjC types in the C++ infrastructure). I discussed this with Doug and we decided to add the XFAIL.</div><div><br><blockquote type="cite"><div><br><br>Thanks again for working on this Steve! I know it's largely thankless, but it's a huge cleanup.</div></blockquote><div><br></div>I appreciate the encouragement. I think this kind of hygiene is quite important. Kind of like getting my teeth cleaned:-)</div><div><br></div><div>snaroff</div><div><br></div><div><br></div><div><br><blockquote type="cite"><div><br><br>-Chris<br></div></blockquote></div><br></body></html>