sammccall added a comment. Nice!
================ Comment at: clang/lib/Basic/SourceManager.cpp:796 + // larger than SLocOffset. + const SrcMgr::SLocEntry *I = LocalSLocEntryTable.end(); + // LessIndex - This is the lower bound of the range that we're searching. ---------------- hmm, while here, why not just get rid of `I` and use `GreaterIndex` throughout? Of its uses: line 806 and 815 would be clearer, 814 would be less clear, 796 and 813 are the same, and 827 goes away entirely. (This is a little thing to nitpick, but I always hate trying to read this function) ================ Comment at: clang/lib/Basic/SourceManager.cpp:809 // Find the FileID that contains this. "I" is an iterator that points to a // FileID whose offset is known to be larger than SLocOffset. ---------------- this comment duplicates your new one above ================ Comment at: clang/lib/Basic/SourceManager.cpp:897 // actually a lower index! unsigned GreaterIndex = I; unsigned LessIndex = LoadedSLocEntryTable.size(); ---------------- is the same optimization available here? I think getFileIDLoaded() is important for module builds and when we use PCH. Though admittedly I don't think this is relevant to the lexer path we've been looking at: those locations should always be local. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135132/new/ https://reviews.llvm.org/D135132 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
