[cfe-commits] r163813 - in /cfe/trunk/lib/Sema: SemaDeclObjC.cpp SemaType.cpp

[John McCall]

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())

John.