[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue May 4 12:17:28 PDT 2021
martong added a comment.
Thank you for this patch!
Could you please provide a lit test that ignites the over and over parsing behaviour? I think you need to create two files and the second one should contain parser error(s).
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:257
llvm::Optional<InvocationListTy> InvocationList;
+ index_error_code InvocationListParsingError = index_error_code::no_error;
};
----------------
I think it would be more consistent to make this an `llvm::Error`. I.e. we would store the result of the whole `lazyInitInvocationList` in this member variable. And as such, a better name could be `PreviousResult`. And this could be an `Optional` to indicate that we have never called `lazyInitInvocationList` before.
This means we would check in `lazyInitInvocationList` whether the `PreviousResult` is set and if yes is it an error. And if both conditions are true then return with the stashed error.
================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:670
return llvm::Error::success();
+ else if (index_error_code::no_error != InvocationListParsingError)
+ return llvm::make_error<IndexError>(InvocationListParsingError);
----------------
The `if` here is superfluous because the condition must be `true` always, otherwise we would have an `InvocationList`, wouldn't we?
Or we can have this condition, but then the `if (InvocatioList)` is not needed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101763/new/
https://reviews.llvm.org/D101763
More information about the cfe-commits
mailing list