[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:23
 #include "llvm/ADT/StringMap.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/OperandTraits.h"
----------------
Perhaps this include is needed only in the .cpp file?


================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:97
 llvm::Expected<llvm::StringMap<std::string>>
-parseCrossTUIndex(StringRef IndexPath, StringRef CrossTUDir);
+parseCrossTUIndex(StringRef IndexPath);
 
----------------
Ah, so we use an absolute path for `IndexPath` from now on? If yes, could you please add that to the comments above?


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:121
+      return "Compile commands database contains multiple references to the "
+             "same sorce file.";
     }
----------------
typo: sorce -> source


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+
----------------
Seems like `Tool` has it's lifetime ended when `load()` finishes. But we have long living `ASTUnits` whose lifetime are longer then the `Tool` which created them. Is this not a problem?


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+
----------------
martong wrote:
> Seems like `Tool` has it's lifetime ended when `load()` finishes. But we have long living `ASTUnits` whose lifetime are longer then the `Tool` which created them. Is this not a problem?
`CI.getPCHContainerOperations()` is probably not needed, because we are not going to exercise the ASTReader, so this could be a nullptr (default param value).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665