[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.


================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:114
+  llvm::Expected<const VarDecl *>
+  getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir,
+                       StringRef IndexName);
----------------
xazax.hun wrote:
> I wonder if we need these overloads. Maybe having only the templated version and having a static assert for the supported types is better? Asking the other reviewers as well.
They are not required, but I thought they make the interface more clear and help prevent implementation in headers.


================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:354
+    if (!VD->hasExternalStorage() ||
+        VD->getAnyInitializer() != nullptr)
+      return true;
----------------
xazax.hun wrote:
> Will this work for zero initialized globals (i.e. not having an initializer expression) that are defined in the current translation unit? It would be great to have a test case for that.
If a variable has a definition in the current TU, it may not have another one else where, but you're correct that this case will continue here. It will however only result in a failed lookup.

Similar but backwards: If an extern var is defined in another TU with global zero init.

Overall I'd say that default initializing a constant to zero is pretty uncommon (and invalid in C++), so that in my opinion it's fine to not support it for now. It seems like there is no transparent way to get the initializer including that case or am I missing something?


https://reviews.llvm.org/D46421