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

Balázs Kéri via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 6 00:03:07 PST 2020


balazske added inline comments.


================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:222
+
+  struct ASTLoader {
+    /// Load the ASTUnit by an identifier. Subclasses should determine what this
----------------
`class` would look better here? (The descendants are `class` too.)


================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:225
+    /// would be.
+    virtual LoadResultTy load(StringRef Identifier) = 0;
+    virtual ~ASTLoader() = default;
----------------
Probably it is good to mention in the documentation that the function is used with a string read from a file and the type of the file determines the meaning of the `Identifier` here (the calling code does have no direct knowledge about it).


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:460
+  /// Ideally there is exactly one entry in the compilation database that
+  /// matchse the source file.
+  if (ASTs.size() != 1)
----------------
matches


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:604
+                               index_error_code::failed_to_get_external_ast);
+}
+
----------------
Members of `ASTOnDemandLoader` would be better in a single group in the source file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665





More information about the cfe-commits mailing list