[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: clangd/Diagnostics.cpp:271
+      if (!Note.AbsFile) {
+        log("Dropping note from unknown file: {0}", Note);
+        continue;
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Maybe `vlog`? This is what we use for dropped diagnostics, should probably stick to the same level with dropped notes (even though the dropped notes probably come up less often in practice).
> We seem to be dropping these only at related information case, what about flattening?
> Maybe we should get rid of them at that stage as well.
As with the other comment - you're right, and we'll drop these if we refactor - I don't think there's any need to drop them now, though.


================
Comment at: clangd/Diagnostics.cpp:417
     D.InsideMainFile = InsideMainFile;
     D.File = Info.getSourceManager().getFilename(Info.getLocation());
+    auto &SM = Info.getSourceManager();
----------------
kadircet wrote:
> Do we still need the File field as well?
It's more readable than full path - e.g. if the main TU is foo.cc and includes foo.h, this is "foo.h".

It's true that if we want to flatten as another pass, we're not going to make use of this information. I'd rather change that in the flatten-as-another-pass patch, so we can see whether the damage to output is worth the refactoring.
(And whether we want to explicitly compute a nice path somehow, etc)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267