[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus edited reviewers, added: NoQ; removed: dergachev.a.
Szelethus added a subscriber: NoQ.
Szelethus added a comment.
This revision is now accepted and ready to land.

Let's also have a link to your cfe-dev mail in this patch: http://lists.llvm.org/pipermail/cfe-dev/2019-January/060968.html

Overall, I like this quite a bit, as I personally experienced the consequence of this invalidation technique while developing `UninitializedObjectChecker`. I'd be interested to see how many more reports will it emit with this patch, I have a suspicion that it'll be very significant.

Let's wait for what @NoQ thinks of this patch.

> What should be the spelling of such an annotation?

How about these: `uses_offsetof`, `may_use_offsetof`?

> How to handle indirect calls? Even if we were in an ideal world where all the callees are annotated, we might not know who the actual callee is (function pointers, virtual calls etc). So what should we do? Less or more invalidation for all indirect calls or have a separate mechanism to let the user define at the call site how to handle a specific call?

Hmm, I suspect that such functions are few and far in between, so maybe the inconvenience of annotating calls to a function through a function pointer that may use `offsetof` can be justified. 
For virtual methods, I guess we can't expect the user to always have the ability of adding the annotation to first base where the virtual method is declared, and not even CTU can ensure that we can scan all methods that implement it. Maybe for these very exceptionally rare cases, annotating the actual calls to the functions would also be justified.

Sadly, I can't say anything meaningful about Artem's ideas on top of my head. :)



================
Comment at: test/Analysis/cxx-uninitialized-object.cpp:371
   PassingToUnknownFunctionTest1(int) {
-    mayInitialize(a);
-    // All good!
+    mayInitialize(a); // expected-warning{{1 uninitialized field at the end of the constructor call}}
   }
----------------
I bet that the current invalidation technique crippled much of this checker's capabilities, so I'm happy to see it change. Hooray!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57230/new/

https://reviews.llvm.org/D57230