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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51747#1227921, @ioeric wrote:

> In https://reviews.llvm.org/D51747#1227089, @ilya-biryukov wrote:
>
> > > ! In https://reviews.llvm.org/D51747#1227719, @kadircet wrote:
> > > 
> > >> ! In https://reviews.llvm.org/D51747#1227089, @ilya-biryukov wrote:
> > > 
> > > Not sure if it's fine to hijack our own diagnostic-specific flags in to clang command args.
> > > 
> > > Cons that I see:
> > > 
> > > 1. There is no way for the users to turn them off if they find them non-useful. If we add a way, it would be more config parameters which overlap with other mechanism that we have - compiler flags.
> > > 2. Users who are used to having them as warnings will now see them as notes. Again, no way to tweak this behavior.
> > > 
> > >   What's our use-case? Maybe we should ask the clients to add -Wdeprecated if they care about those?
> > > 
> > >   PS In case I'm missing the context here, please let me know.
> >
> > Agree with you, I think it would be better to provide it as an option. https://reviews.llvm.org/D51724 with this one we added a way to show deprecated symbols on code completion responses and wanted to move forward with showing the ones that are already in existing code.
>
>
> I also agree with you regarding options. A pattern I've observed (in some infamous large codebase ;) is that warnings for deprecated symbols are disabled in the compile command as they can introduce too much noise during build, although it would make sense to show these warnings when user edits the code (most of the time). I think there can be other diagnostics that are more desirable as editor diagnostics than as compiler warnings/errors.


So I'll be (somewhat) the naysayer regarding options. 
We still have to choose a default, and almost everyone will use it, including those who don't like it. So we still need the best possible default, and shouldn't let talk of options distract from this.
Most of the value of adding an option is: if someone complains, we can tell them to go away :-) One possible corollary is: we shouldn't add the option until someone complains. I'm not 100% sure about that, though - not totally opposed to an option here.

But if we add an option, we need to decide what the *other* option should be too, and this isn't obvious (should it be "suppress deprecation warnings" or "use compilation database as-is"?

My vote for default behavior would be: add -Wdeprecated, and if deprecation diagnostics are **errors**, downgrade them to warnings. (Or better add `-Wdeprecated -Wno-error=deprecated`).
But I'm biased by mostly using -Wno-deprecated -Werror codebases, if I didn't I'd probably prefer to just respect the compile-commands.

(I'm not a fan of the downgrade-to-note behavior: notes do not carry the connotation of "bad", and deprecation warnings can have notes attached themselves which is confusing).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747