kadircet marked 7 inline comments as done.
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:138
+ : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+ MainFileTokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ }
----------------
sammccall wrote:
> tokenizing the whole file an extra time on every AST build seems a bit sad -
> this is considerably more lexing than we were doing before. Probably doesn't
> matter?
>
> We could trim this to the preamble bounds I guess. Or even compute it once
> when the preamble is built, since we assume all the bytes are the same? I
> guess SourceLocations are the problem... we could just translate offsets into
> the new SM, but that gets messy.
> On the other hand, assuming the preamble isn't going to change at all seems
> like an assumption not long for this world.
> On the first hand again, maybe we'll have to revisit looots of stuff (go to
> definition and everything) once that assumption breaks anyway.
Implemented a way to partially tokenize a file in D74962.
> On the other hand, assuming the preamble isn't going to change at all seems
> like an assumption not long for this world.
It should be okay for replaypreambles as only clang tidy checkers depends on
this logic and we are not planning to emit diagnostics with stale preambles.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:175
+ auto HashTok =
+ llvm::find_if(MainFileTokens, [&HashLoc](const syntax::Token &T) {
+ return T.location().getRawEncoding() == HashLoc;
----------------
sammccall wrote:
> this looks like a linear search for each #include
made it logarithmic instead, we can also make it linear in total if we decide
to rely on the fact that `MainFileIncludes` are sorted. I believe it is
currently true but never promised by the include collector.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:186
+
+ // We imitate the PP logic here, except clang::Token::Flags, none of the
+ // callers seem to care about it (yet).
----------------
sammccall wrote:
> Not clear what "imitate the PP logic" means.
> We construct a fake 'import'/'include' token... nobody cares about
> clang::Token::Flags.
it was refering to the fact that we were performing the
`PP.LookupIdentifierInfo` call to set kind etc.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74842/new/
https://reviews.llvm.org/D74842
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
