[PATCH] D19960: [scan-build] fix warnings emitted on Clang CodeGen code base

[Apelete Seketeli via cfe-commits]

apelete added inline comments.

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1317
@@ -1316,2 +1316,3 @@
       }
+      assert(V && "constant must be not NULL at this point");
       TemplateParams.push_back(DBuilder.createTemplateValueParameter(
----------------
dblaikie wrote:
> It looks like this assertion could actually be a different assertion a little higher up.
> 
> For 'V' to be non-null, one of the if/else if chain above must fire.
> 
> So change the last else if to an else, and the dyn_cast to a cast, and the cast will fail an internal assertion if it's not valid (& the analyzer can easily then see that at least one of the assignments to V happens - whether or not the analyzer assumes that all the initializers of V are non-null, that's a separate issue...)
I do not understand what you are suggesting here.
Are you suggesting I should try those changes and see for myself that at least one of the assignments to V actually happens (which would mean that the warning is a false positive), or are you suggesting that the right way to fix it is to remove the assert and replace the last 'else if' construct with an 'else' ?

================
Comment at: lib/CodeGen/CodeGenModule.cpp:2302
@@ -2301,2 +2301,3 @@
                                                  unsigned AddrSpace) {
+  assert(D && "variable declaration must be not NULL");
   if (LangOpts.CUDA && LangOpts.CUDAIsDevice) {
----------------
dblaikie wrote:
> Again, a bit confused about whether you're proposing fixing nearly every pointer parameter in Clang and LLVM to assert non-null... I think we'd need a discussion about what that looks like.
Sorry my intentions were not clear.
The only function pointer parameters I'm asserting are the ones that trigger a warning when running scan-build.
This might not be the right way to fix the issue, I will be waiting for you advice in that case (please have a look at my reply to your concern on the mailing list).


http://reviews.llvm.org/D19960