kadircet added inline comments.
Comment at: clangd/ClangdServer.cpp:537
C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+ C->CommandLine.push_back("-Wdeprecated-declarations");
return std::move(*C);
sammccall wrote:
> kadircet wrote:
> > sammccal
kadircet updated this revision to Diff 165266.
kadircet marked an inline comment as done.
kadircet added a comment.
- Do not show up as errors even on codebases with -Werror.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51747
Files:
clangd/ClangdServer.cpp
unittests/clang
sammccall added inline comments.
Comment at: clangd/ClangdServer.cpp:537
C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+ C->CommandLine.push_back("-Wdeprecated-declarations");
return std::move(*C);
kadircet wrote:
> sammccall wrote:
> > as note
kadircet updated this revision to Diff 165252.
kadircet marked 5 inline comments as done.
kadircet added a comment.
- Resolve discussions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51747
Files:
clangd/ClangdServer.cpp
unittests/clangd/ClangdUnitTests.cpp
Index: unitt
kadircet added a comment.
As discussed offline with Ilya, decided to keep the compile flag addition since
it would be easier to pass the logic of a command line flag to that point. Also
not changing the severity level, since they will show up on diagnostics lists
in anyway it doesn't save much.
On Tue, Sep 11, 2018 at 3:43 PM Ilya Biryukov via Phabricator <
revi...@reviews.llvm.org> wrote:
> ilya-biryukov added a comment.
>
> In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:
>
> > A few thoughts here:
> >
> > - does CDB describe user or project preferences? unclear.
>
>
> Agr
ilya-biryukov added a comment.
In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:
> A few thoughts here:
>
> - does CDB describe user or project preferences? unclear.
Agree, it's a mix, defaults are from the project but users can add extra flags.
> - "show this warning for code I bu
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 complain
kadircet added a comment.
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
sammccall added a comment.
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% s
ilya-biryukov added a comment.
> 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.
A
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
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-biryuk
ilya-biryukov added a comment.
> 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
ioeric added a comment.
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
kadircet added a comment.
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.
ilya-biryukov added a comment.
Not sure if it's fine to hijack our own diagnostic-specific flags in to clang
command args.
Const 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 ot
ioeric added inline comments.
Comment at: clangd/Diagnostics.cpp:299
+D.Severity =
+D.Category == "Deprecations" ? DiagnosticsEngine::Note : DiagLevel;
return D;
kadircet wrote:
> Couldn't find a better way to check for this one. Category types a
kadircet updated this revision to Diff 164268.
kadircet added a comment.
- Formatting.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51747
Files:
clangd/ClangdServer.cpp
clangd/Diagnostics.cpp
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests
kadircet added inline comments.
Comment at: clangd/Diagnostics.cpp:299
+D.Severity =
+D.Category == "Deprecations" ? DiagnosticsEngine::Note : DiagLevel;
return D;
Couldn't find a better way to check for this one. Category types are coming
from
kadircet created this revision.
kadircet added reviewers: ioeric, sammccall, ilya-biryukov, hokein.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.
Adds deprecation warnings to diagnostics. Also lowers the severity from
warning to notes to not to annoy people that work on big co
21 matches
Mail list logo
