[cfe-dev] Additional annotations for static analysis (Objective C designated initializers)

[Louis Gerbarg]

I should have time to make the changes in next couple of days, some
quick thoughts

On Wed, Nov 5, 2008 at 9:51 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
> On Nov 1, 2008, at 10:47 PM, Louis Gerbarg wrote:
>
>> Anyway, let me know what you think. I think it should be stylistically
>> consistent with the rest of the codebase, but C++ is a little rusty
>> (Imagine that, someone trying to enhance the Objective C analyzer
>> doesn't write much C++ ;-)
>
> Here's some comments on the actual check.  Comments are inline.  One meta
> comment: the LLVM coding convention is to stay within 80 columns; there are
> a few cases in the patch where the lines are slightly too long.

I need to write a script to check my patches for that, I have been
spoiled by large monitors for far too long.

> +  ObjCInterfaceDecl* D = ID->getClassInterface();
> +  ObjCInterfaceDecl* C = D->getSuperClass();
>
> Looks good.
>
> +  bool hasDI = false;
> +  bool superHasDI = false;
>
> Looks good.
> +
> +  // Check if self or super has declared designated initializers
> +  for (ObjCInterfaceDecl::instmeth_iterator I=D->instmeth_begin(),
> +       E=D->instmeth_end(); I!=E; ++I) {
> +
> +    ObjCMethodDecl* M = *I;
> +    if (M->getAttr<DesignatedInitializerAttr>()) {
> +      hasDI = true;
> +      break;
> +    }
> +  }
>
> Looks great.
>
> +
> +  if (C) {
> +    for (ObjCInterfaceDecl::instmeth_iterator I=C->instmeth_begin(),
> +         E=C->instmeth_end(); I!=E; ++I) {
> +
> +      ObjCMethodDecl* M = *I;
> +      if (M->getAttr<DesignatedInitializerAttr>()) {
> +        superHasDI = true;
> +        break;
> +      }
> +    }
> +  }

I actually just threw that in at the last minute because I was trying
to be as unobtrusive as possible in my AST modifications. I think
ultimately it would be better to just have a flag in the
ObjCInterfaceDecl, and to set it if/when we encounter the attribute on
one of its methods, and just call the getter here. If I am going to
factor this out anyway I would rather switch to doing that, unless you
think it is a bad idea.