Some clang-tidy warnings get struck-through

[arijun]

Some clang-tidy warnings are displayed with a strikethrough. From what I can tell, this happens with modernize- lints but not e.g. bugprone- or unused-includes. This is a recent development, previously they just had an orange underline. This, combined with the expansive way the error can be defined, can make it hard to read some things (crossing out all hundred lines of a typedefed enum makes it pretty hard to skim).

If this is considered desired behavior I would appreciate if someone could tell me a workaround to shut it off locally (preferably while maintaining the strikethrough behavior for deprecated code)

System information
Clangd version (from the log, or clangd --version): 16.0.2
clangd extension version: v0.1.24

[HighCommander4]

At the protocol level this is caused by the affected diagnostics having the Deprecated diagnostic tag attached to them.

[HighCommander4]

So one way to disable it in vscode is to set "editor.showDeprecated" to false.

preferably while maintaining the strikethrough behavior for deprecated code

Does the "deprecated code" situation apply to C++? If yes, could you show an example?

If not, then scoping the setting to just C++ should do it:

"[cpp]": {
  "editor.showDeprecated": false
}

[arijun]

Does the "deprecated code" situation apply to C++? If yes, could you show an example?

Yes, some of our teams will use deprecated to indicate there is a newer or more comprehensive function to use, without breaking existing usages. Here is a toy example:

The oldApiCall is correctly struck-through here.

In the interim I am disabling those lints so it's not a dealbreaker, but it would be nice to have them for code I have control over without making code not under my control unreadable.

[HighCommander4]

Ok, so that one is a -Wdeprecated diagnostic.

So, is your suggestion that clangd should send a Deprecated diagnostic tag for the -Wdeprecated diagnostic, but not for the clang-tidy modernize- diagnostics?

Conceptually the two seem similar ("you're using an old thing, you could be using a newer thing instead").

[arijun]

You're not wrong, although explicitly marking something as deprecated does seem like a stronger signal than the modernize- lints. If I call something that relies on deprecated code, I often have to make sure the deprecated calls don't have flaws that will affect me, something I never have to worry about with modernize-.

Truthfully, though, the main issue with the strikethrough comes from my example of typedef'ed enums, where it obscures important information. And that could probably be solved by changing the highlight region to just be on the typedef itself, or something similar. That's under the purview of clangd (I assume) so I will open a ticket there.

Until that gets changed there (if it does), would it be worth removing the strikethrough for modernize-use-using? I imagine I wouldn't be the only user with this issue.

[HighCommander4]

Truthfully, though, the main issue with the strikethrough comes from my example of typedef'ed enums, where it obscures important information. And that could probably be solved by changing the highlight region to just be on the typedef itself, or something similar.

Yeah, narrowing the diagnostic range to just the typedef keyword itself would make sense to me.

That's under the purview of clangd (I assume) so I will open a ticket there.

The diagnostic is produced by clang-tidy, clangd just passes it through without modifying most properties like the diagnostic range, so the place to file that issue would be https://github.com/llvm/llvm-project/issues which is where clang-tidy issues are tracked. (Please feel free to drop a link to it here once you've filed it.)

Until that gets changed there (if it does), would it be worth removing the strikethrough for modernize-use-using? I imagine I wouldn't be the only user with this issue.

cc @sam-mccall, who wrote the patch to use the Deprecated diagnostic tag for modernize- diagnostics, for thoughts on this.

I don't have a strong opinion either way. If this were to come up in a project I'm working on, my approach would be to use a .clang-tidy metadata file (which clangd also respects) to disable the modernize-use-using diagnostic in subdirectories where I'm not planning to promptly convert the typedefs to usings.

[arijun]

clang-tidy issue: llvm/llvm-project#62702

[sam-mccall]

The Deprecated label still seems semantically appropriate to me, given that the user has explicitly opted into wanting to modernize such uses - these checks are all off by default.

The end-to-end UX is poor here but there are many loosely-coupled layers involved (clang diagnostics, clang-tidy, specific check, clangd, LSP, vscode UI) and I think it's reasonable to expect the occasional wart.

Practically I agree that reducing the diagnostic range for this particular check is a good fix: it's the use of typedef that's the problem, not the other parts of the declaration.

(If clangd's heuristics to pick a range are preferring the fixit range over the diag loc, maybe we can add SourceRange(Loc, Loc) to the diag to suppress)

[oceanusxiv]

FWIW, as noted this affected other checks too. modernize-use-trailing-return-type for example. Under the current UX. Opting into this lint would mean all function names not using the trailing return type would be crossed out, making function names very hard to read.

I think there's a good case to be made that clang-tidy ought not to be issuing Deprecated labels for modernize lints. Even if you are opting into it, issuing a Deprecated tag seems a very strong signal to be send, putting it on par with actually explicitly marked deprecated code, which I do not think is true generally speaking. The fact that clang-tidy is issuing lints is itself already enough signal, adding a Deprecated is just excessive, and can mask actual deprecated warnings in editors.

[HighCommander4]

Not sure if this helps at all, but https://reviews.llvm.org/D154443 downgrades the severity of diagnostics with the Deprecated tag to DiagnosticSeverity.Hint.