balazske added a comment.
I discovered that there are problems with this change. The number of loaded AST
units is not incremented correctly and the "CTU loaded AST file" is displayed
for every access to a AST unit (even if it was got from cache). The problem is
that the counting and print of A
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367829: [CrossTU][NFCI] Refactor loadExternalAST function
(authored by gamesh411, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D64753?vs=213311&id=213315#toc
Repository:
rL LLVM
gamesh411 updated this revision to Diff 213311.
gamesh411 added a comment.
- Remove unused member Limit
- Rebase to current master
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
Files:
clang/include/clan
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
LGTM! Thanks! Please do the commit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
__
gamesh411 updated this revision to Diff 212788.
gamesh411 added a comment.
Specify the exact meaning of successful storage
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
Files:
clang/include/clang/CrossT
martong added inline comments.
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:489
+ if (DisplayCTUProgress) {
+if (llvm::Expected FileName =
+ASTStorage.getFileForFunction(LookupName, CrossTUDir, IndexName))
` LoadOperation` should not be
gamesh411 marked 2 inline comments as done.
gamesh411 added a comment.
Updated the revision.
I find this solution definitely more compact with responsibilities more
separated. One more thing that comes to mind is that maybe the whole explicit
passing of CTUDir and IndexName arguments is a bit ve
gamesh411 updated this revision to Diff 212290.
gamesh411 marked 5 inline comments as done.
gamesh411 added a comment.
- Merge RAII class
- Update comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
Fi
gamesh411 marked 11 inline comments as done.
gamesh411 added a comment.
Thanks for pointing out these issues. Most of them are agreed. Merging the RAII
counter with the threshold checker class, however, does not seem like a good
decision for me. What would be the benefits of merging the two?
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 impleme
gamesh411 updated this revision to Diff 211363.
gamesh411 added a comment.
Too much autoformat fixed
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
https://reviews.llvm.org/D64753
Files:
clang/include/clang/CrossTU/CrossTranslationUnit
gamesh411 updated this revision to Diff 211361.
gamesh411 added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Refactor functionality into local classes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64753/new/
htt
martong added a comment.
Thank you Endre, this patch is a great initiative.
However, I think we can do better encapsulation then just the reorganization of
the functions:
We could encapsulate into a nested class `NameASTUnitMap` and the functions
which operate on this (`getCachedASTUnitForName`,
gamesh411 created this revision.
gamesh411 added a reviewer: martong.
Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs.
Herald added a project: clang.
Refactor loadExternalAST method of CrossTranslationUnitContext in order to
reduce maintenance burden and so that features are eas
14 matches
Mail list logo
