martong added a comment.
I have a feeling that LoadPass and LoagGuard could be (should be) merged
together, other than that we are getting close.
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:185
+
+ /// Cached access to ASTUnit mapping information is implemented in this
+ /// section.
----------------
Perhaps this comment is not necessary, because it is not obvious where does the
mentioned "section" ends and this is a bit redundant with the comments for the
classes below.
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:229
+
+ /// The thresholding of AST-loads is implemented in this section.
+
----------------
We may not need this comment.
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:233
+ /// counter on successful load. Member `CanBegin` is used to signal, that the
+ /// import attempt should be made at the beginning. Member `WasSuccesful`
+ /// signifies whether the load is successfully finished. The counter is
----------------
Could you please add these descriptions above the member declarations?
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:245
+ unsigned &NumASTLoaded;
+ bool CanBegin;
+ bool WasSuccessful;
----------------
This name `CanBegin` sounds strange to me. Perhaps `Enabled`?
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:251
+ /// it is responsible for handing out LoadPass instances.
+ class LoadGuard {
+ public:
----------------
I have a feeling that LoadPass and LoagGuard could be (should be) merged
together.
================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:352
+
+CrossTranslationUnitContext::LoadPass::LoadPass(unsigned &NumASTLoaded,
+ bool CanBegin)
----------------
I think you could leave the implementation of LoadPass:: member functions in
the header, because they are so small. And that way I think code comprehension
could improve.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
