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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
