sammccall marked an inline comment as done.
sammccall added a comment.
In D58291#1400569 <https://reviews.llvm.org/D58291#1400569>, @kadircet wrote:
> LG but is this information really useful to users? According to LSP `The
> diagnostic's code, which might appear in the user interface.`, I think seeing
> this will be mostly noise for users.
It's a good question, it depends how this is surfaced, and we may want to tweak
the behavior or suppress entirely in some cases.
I think at least some are useful:
- clang-tidy check names are things users need to know about (used for
configuration)
- for warnings, we quite likely should replace with the most specific warning
category (e.g. "unreachable-code-loop-increment"), again these are used for
configuration (-W)
- for others, maybe we should at least trim the err_ prefix, or maybe drop them
entirely.
================
Comment at: clangd/Diagnostics.cpp:39
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
----------------
kadircet wrote:
> I suppose `CrossTUKinds` is left out intentionally ?
Yeah, this isn't really part of clang, and seems to be part of static analyzer.
Some places include it and others don't...
================
Comment at: clangd/Diagnostics.cpp:281
+ if (auto* Name = getDiagnosticCode(D.ID))
+ Main.code = Name;
if (Opts.EmbedFixesInDiagnostics) {
----------------
jkorous wrote:
> It seems to me that in case `ID` is undefined (hits the default case in
> `getDiagnosticCode`) we are calling `std::string` non-explicit constructor
> with `nullptr` which is UB.
> How about changing `char* getDiagnosticCode` to `Optional<char*>
> getDiagnosticCode` or similar to make it less error-prone?
This is an if statement - if the pointer is null we never assign to std::string.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58291/new/
https://reviews.llvm.org/D58291
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
