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

[Louis Gerbarg]

Thanks for the various bits of feedback everyone gave me, a lot of
people seemed to like the basic idea of the patch, no one had much in
the way of implementation thoughts, I will take that either to mean my
code is perfect, or more likely most of the people who can critique
the code are busy trying to push out the next LLVM release ;-)

Anyway, having though through a few of the semantic issues:

1) Yes, multiple designated initializers are allowed, though it is
uncommon, but making it an error is wrong
2) Inference is not feasible. If you have all the source it might be,
but the analyzer has to do it against headers for libraries it does
not have the sources for. For instance, if I compile a subclass of
NSView I do not have the implementation of NSView, which means I
cannot infer NSView's designated initializers, which means I can't
make sure my subclass calls them. That makes me think annotation is
definitely the correct way to go.

Okay, so, the final rules seem to boil down to this:

1) If a class declares any designated initializers make sure all
methods named either or one of the designated initializers, or call
one of the designated initializers
2) if a class and its superclass both declare designated initializers
make sure that the classes designated initializers call the
superclasses designated initializers

Okay, so having said that, here is a patch to add the attribute. I
removed the patch to the ObjCInterfaceDecl, a dynamically generate the
the info during analysis. I could save a little work by adding one
bool to the ObjCInterfaceDecl, but something does not feel totally
clean to me about adding info that is only useful during static
analysis during all compiles. On the other hand the attribute is
always there, so that might just be in my head.

I am also attaching a patch to the analyzer to use that info and apply
the above rules. The patch simply makes sure the appropriate msg sends
appear in the appropriate methods, it does not perform any path
sensitive analysis to guarantee control flows through them, so it
might miss some potential errors even if annotated. I figured this was
a good enough start to catch some stuff, it lets us debate if adding
an attribute is okay, and I don't have to figure out how the all those
bits of the analyzer work right off the bat.

Also, since there is no path sensitive analysis I just mark the init
method that is wrong, not the particular call site that is in
violation.

Attached is a test file that correctly generates the following errors:

$ ./Debug/bin/clang -warn-objc-designated-initializer  -verify
designated-initializer-annotation.m
Warnings seen but not expected:
  Line 14: The initializer method '-[TestObject init]' does not call a
designated initializer
  Line 22: The initializer method '-[TestObject init3]' does not call
a designated initializer
  Line 38: The designated initializer method '-[TestSubclass init4]'
does not call a designated initializer for superclass 'TestObject'

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++ ;-)

Louis
-------------- next part --------------
A non-text attachment was scrubbed...
Name: designated_init_attribute.patch
Type: application/octet-stream
Size: 3092 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20081102/58bb0e3d/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: designated_init_analyzer.patch
Type: application/octet-stream
Size: 7432 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20081102/58bb0e3d/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: designated-initializer-annotation.m
Type: application/octet-stream
Size: 523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20081102/58bb0e3d/attachment-0002.obj>