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



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

Reply via email to