[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#1230420, @kadircet wrote:

> In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:
>
> > In https://reviews.llvm.org/D51747#1228919, @ilya-biryukov wrote:
> >
> > > > 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.
> > >
> > > Any thoughts on tampering with provided compile args in the first place? Is it fine for clangd to be opinionated about the warnings we choose to always show to the users?
> > >  E.g. I'm a big fan of various uninitialized warnings, but nevertheless don't think clangd should force them on everyone.
> >
> >
> > A few thoughts here:
> >
> > - does CDB describe user or project preferences? unclear.
> > - "show this warning for code I build" is a higher bar than "show this warning for code I edit". So CDB probably enables too few warnings.
> > - Some warnings play well with -Werror (like uninit warnings), some don't (like deprecated). -Werror projects often disable interesting warnings.
> >
> >   I'm not sure we should strictly follow the CDB, but the bar to override it should probably be high. Maybe the (default) behavior here should be "add -Wdeprecated -Wno-error=deprecated if -Werror is set"?
>
>
> I agree with checking for -Werror and then providing -Wno-error as well, if -Wdeprecated was not added already.


I'm nervous enough about trying to ad-hoc parse the flags that I'd be tempted just to ad -Wno-error=deprecated without checking to see if -Werror is set - it's a no-op if not. But up to you.

Ilya's suggestion of just respecting the compilation database also seems OK. It seems a bit sad for the users who won't see deprecation warnings because their project doesn't want to show them at build time. I'd lean towards adding this to see if anyone complains, but happy with whatever you two can agree on.

> Then keeping it as warnings shouldn't be too much of a problem except the case you mentioned, if user wants to see all diagnostics as a list suddenly they will get deprecations in that list as well :(.

I don't think that making them notes will avoid this though :-/


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747