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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

The analyzer can only reason about const variables this way, right? Maybe we should only import the initializers for such variables to have better performance? What do you think?

Also, I wonder what happens with user-defined classes. Will the analyzer evaluate the constructor if the variable is imported?



================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:114
+  llvm::Expected<const VarDecl *>
+  getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir,
+                       StringRef IndexName);
----------------
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.


================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:138
   llvm::Expected<const FunctionDecl *> importDefinition(const FunctionDecl *FD);
+  llvm::Expected<const VarDecl *> importDefinition(const VarDecl *VD);
 
----------------
Same question as above.


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:114
 
+bool HasDefinition(const FunctionDecl *D, const FunctionDecl *&DefD) {
+  return D->hasBody(DefD);
----------------
Functions should start with lowercase letter. Maybe these could be static?


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:143
+CrossTranslationUnitContext::findDefInDeclContext(const DeclContext *DC,
+                                                  StringRef LookupFnName) {
   assert(DC && "Declaration Context must not be null");
----------------
Fn in the name refers to a function, maybe that could be changed as well.


================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:354
+    if (!VD->hasExternalStorage() ||
+        VD->getAnyInitializer() != nullptr)
+      return true;
----------------
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.


https://reviews.llvm.org/D46421