hokein added a comment.
thanks for the comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:145
+ for (const auto &Ref : MAndRefs.second) {
+ auto L = sourceLocationInMainFile(SM, Ref.Rng.start);
+ if (!L) {
----------------
kadircet wrote:
> it's unfortunate that we're scanning the file for every occurrence of a macro
> just to get an offset. this might get problematic in big test files, what
> about introducing an offset to `MacroOccurence`?
> it's unfortunate that we're scanning the file for every occurrence of a macro
> just to get an offset. this might get problematic in big test files, what
> about introducing an offset to `MacroOccurence`?
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:145
+ for (const auto &Ref : MAndRefs.second) {
+ auto L = sourceLocationInMainFile(SM, Ref.Rng.start);
+ if (!L) {
----------------
hokein wrote:
> kadircet wrote:
> > it's unfortunate that we're scanning the file for every occurrence of a
> > macro just to get an offset. this might get problematic in big test files,
> > what about introducing an offset to `MacroOccurence`?
> > it's unfortunate that we're scanning the file for every occurrence of a
> > macro just to get an offset. this might get problematic in big test files,
> > what about introducing an offset to `MacroOccurence`?
>
>
thanks, it makes sense, sent out https://reviews.llvm.org/D153259.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:156
+
+ if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
+ Macros.push_back(
----------------
kadircet wrote:
> we should use `Macro->NameLoc`, not the definition loc inside macro-info
oh, right. NameLoc takes the preamble patch into account.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147034/new/
https://reviews.llvm.org/D147034
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
