On Sep 13, 2012, at 11:34 AM, John McCall wrote: > On Sep 13, 2012, at 10:29 AM, Fariborz Jahanian wrote: >> Modified: cfe/trunk/lib/Sema/SemaType.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=163813&r1=163812&r2=163813&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaType.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaType.cpp Thu Sep 13 12:29:07 2012 >> @@ -3335,6 +3335,36 @@ >> return TInfo; >> } >> >> + >> +/// checkImplicitObjCParamAttribute - diagnoses when pointer to ObjC pointer >> +/// has implicit ownership attribute. >> +static void >> +checkImplicitObjCParamAttribute(Sema &S, Declarator &D, QualType T) { >> + if (!S.getLangOpts().ObjCAutoRefCount || >> + !S.OriginalLexicalContext || >> + (S.OriginalLexicalContext->getDeclKind() != Decl::ObjCImplementation >> && >> + S.OriginalLexicalContext->getDeclKind() != Decl::ObjCCategoryImpl)) >> + return; > > In general, please use isa<> checks instead of looking at getDeclKind() > directly. > > I'm sorry for the run-around; I'd forgotten the restricted scope of this > warning. > You're right that this is more appropriate to diagnose in SemaDeclObjC. > >> + if (!T->isObjCIndirectLifetimeType()) >> + return; > > You're basically doing most of the checks for this anyway; no need to do it > first off here. > >> + if (!T->isPointerType() && !T->isReferenceType()) >> + return; >> + QualType OrigT = T; >> + T = T->isPointerType() >> + ? T->getAs<PointerType>()->getPointeeType() >> + : T->getAs<ReferenceType>()->getPointeeType(); > > Please simplify and re-use your queries: > > if (const PointerType *PT = T->getAs<PointerType>()) { > T = PT->getPointeeType(); > } else if (const ReferenceType *RT = T->getAs<ReferenceType>()) { > T = RT->getPointeeType(); > } else { > return; > } > >> + if (T->isObjCLifetimeType()) { > > You don't need to check this; this is always true if there's a local > qualifier. > >> + // when lifetime is Qualifiers::OCL_None it means that it has >> + // no implicit ownership qualifier (which means it is explicit). >> + Qualifiers::ObjCLifetime lifetime = >> + T.getLocalQualifiers().getObjCLifetime(); > > // If we have a lifetime qualifier, but it's local, we must have inferred it. > if (T.getLocalQualifiers().hasObjCLifetime())
In r163824. Thanks for the simplification suggestions. - fariborz > > John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
