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

[Ted Kremenek]

On Nov 6, 2008, at 12:02 AM, Louis Gerbarg wrote:

>> +
>> +  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.

Hi Louis,

I'm actually not certain what you meant by "that" in this last  
sentence.  Could you elucidate a little more?  (I'm a little tired  
right now, so it isn't obvious to me).

> I think
> ultimately it would be better to just have a flag in the
> ObjCInterfaceDecl,

I'm a little confused.  Why would you need a flag in  
ObjCInterfaceDecl?  It really does seem to me that all the information  
you need can be stored as an attribute.  The nice thing about the way  
we do attributes is that we only only pay a cost for them when we use  
them.  By storing extra flags in the Decl objects that are unused 99%  
of the time we bloat the ASTs.  There will always be other things to  
tack on to the ASTs; we like to keep the core data structures lean  
(when possible) and when we need auxiliary data we just record them in  
maps.  The static analyzer itself assumes that the ASTs are immutable,  
and we should probably be passing Decl* and Stmt* values around with a  
'const' qualifier (i.e., the static analyzer will never modify the  
ASTs, only build other data structures on top of them).

> 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.

I'm honestly a little unclear about the shape of your idea (please  
just clarify; again I'm a little tired right now).  If the idea is to  
store flags in ObjCInterfaceDecl that would be twiddled by an analysis  
pass, my belief is that really contrary to the clean design we're  
going for, not only in the static analyzer, but Clang in general.   
Having the ASTs store information specific to an analyzer pass is a  
layering violation; the ASTs should have no knowledge of their  
consumers, and the ASTs are meant to serve many masters.

I honestly think the patch that you have has a nice clean design.   
What problem are you trying to solve by changing it?

Cheers,
Ted