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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to