[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 10 07:02:14 PDT 2018


sammccall added inline comments.


================
Comment at: clangd/ClangdServer.cpp:535
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
----------------
can you add a comment that these flags work with both gcc and clang-cl driver modes?
It seems to be true. This is an important invariant (I should add a test for it) that isn't obvious (there's no test today because we didn't know about clang-cl at the time)


================
Comment at: clangd/ClangdServer.cpp:537
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated-declarations");
   return std::move(*C);
----------------
as noted above I think we should also have -Wno-error=deprecated-declarations

(do you want all of -Wdeprecated, actually?)


================
Comment at: clangd/Diagnostics.cpp:299
+    D.Severity =
+        D.Category == "Deprecations" ? DiagnosticsEngine::Note : DiagLevel;
     return D;
----------------
not sure what the concrete benefits are from using Note rather than Warning. It's semantically iffy, so if we do this it should have a comment justifying it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747





More information about the cfe-commits mailing list