[PATCH] D85753: [clangd] Discard diagnostics from another SourceManager.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz 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);
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85753/new/

https://reviews.llvm.org/D85753