kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/Diagnostics.cpp:209 // Update diag to point at include inside main file. D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str(); D.Range = std::move(R); ---------------- adamcz wrote: > kadircet wrote: > > can't we rather record the mainfilename once and make use of it here? > > i believe the rest of the logic operates on the local sourcelocations/files > > associated with the sourcemanager inside the diagnostic in process. > getMainFileRange() doesn't. > > We can easily get main file from the original source manager or record it in > BeginSourceFile. The thing is that it's not easy to find a place in the main > file to put this diagnostic at. We can, of course, put it at the top of the > file, but normally we try to find the #include location in the main file. > That's done in the getMainFileRange() above and it mixes the location of the > diagnostic (which is for the new SourceManager) and the main file buffer > (which is for the OrigSrcMgr). > > I'm a bit worried that if we put a diagnostic relating to #include at the top > of the file, where another #include may be, it will become very confusing. > getMainFileRange() doesn't. ah right i forgot about that bit :/ > I'm a bit worried that if we put a diagnostic relating to #include at the top > of the file, where another #include may be, it will become very confusing. I totally agree. A crazy idea, we can register a callback to PP on `BeginSourceFile` to track file changes, than whenever we see a diagnostic from a different sourcemanager, we can map it back to last position a filechange was triggered. Ofc, this goes with the assumption that we see a filechanged event on `#include "modular.h"` line, but I guess we should at least get an includedirective callback or something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85753/new/ https://reviews.llvm.org/D85753 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
