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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 18 02:23:01 PDT 2019


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


================
Comment at: clangd/Diagnostics.cpp:280
+      Main.relatedInformation->push_back(std::move(RelInfo));
+    }
   }
----------------
ilya-biryukov wrote:
> NIT: maybe call `OutFn` and return here to avoid checking for `EmitRelatedLocations` again?
> Would arguably make the code simpler, although would require another call to `OutFn(Main)` outside the if branch.
Yeah, I don't really see this as an improvement - it reduces the nesting, but makes the relation of conditions to code more confusing, I think.


================
Comment at: unittests/clangd/DiagnosticsTests.cpp:259
+#ifdef _WIN32
+      "c:\\path\\to\\foo\\bar\\main.cpp",
+#else
----------------
ilya-biryukov wrote:
> maybe use `testPath()` here to avoid PP directives?
Done - this requires changing the actual paths but it doesn't seem to matter.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267





More information about the cfe-commits mailing list