ilya-biryukov added a comment. Neat! Many thanks, that's a very useful cleanup. A few suggestions from my side.
================ Comment at: clang-tools-extra/clangd/SourceCode.cpp:332 +bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM) { + return Loc.isValid() && SM.isWrittenInMainFile(SM.getFileLoc(Loc)); +} ---------------- NIT: why not `getExpansionLoc`? Its implementation and description seems simpler and (I believe) it produces locations inside the same files as `getFileLoc` (as both the macro arg and the macro name of the expansion are always coming from the same file) ================ Comment at: clang-tools-extra/clangd/SourceCode.h:81 +/// +/// if the Loc is a macro location, the function calls getFileLoc to get the +/// file location before checking. ---------------- Could we add a description without mentioning other `SourceManager` functions here? I believe this should be a fair description of what this function is doing: ``` For macro locations, returns iff the macro is being expanded inside the main file. ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64915/new/ https://reviews.llvm.org/D64915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits