[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thank you MaskRay! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-18 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL361112: [clangd] Respect clang-tidy suppression comments (authored by MaskRay, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Could someone merge this and D61841 now that they're approved? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 199954. nridge marked 5 inline comments as done. nridge added a comment. Address latest review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953 Files:

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, this looks good. I think we should avoid the subclassing, but any of {generalize in the next patch, different approach for the next patch, subclass for now and clean up later}

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:243 +// future). +class ClangdDiagnosticConsumer : public StoreDiags { +public: sammccall wrote: > Thanks, this is much cleaner. > > I think we don't need the subclass at all

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 199551. nridge marked 9 inline comments as done. nridge added a comment. Address review comments, except for the derived class one, about which I have a question Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks, this looks nice. Main ideas here: - it'd be nice to get rid of the subclassing in the diag consumer if we can - i'm not sure the logic around LastErrorWasIgnored is completely accurate - can we add a unit test for this? The existing clang-tidy diag tests are in

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-05-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Friendly review ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 197040. nridge marked 3 inline comments as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953 Files:

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:395 + DiagLevel != DiagnosticsEngine::Fatal && + LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), +

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for doing this! Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:395 + DiagLevel != DiagnosticsEngine::Fatal && + LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), +

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge edited reviewers, added: ilya-biryukov; removed: hokein. nridge added a comment. Changing reviewers as I understand @hokein is on vacation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60953/new/ https://reviews.llvm.org/D60953

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. This partially fixes https://bugs.llvm.org/show_bug.cgi?id=41218. Repository: rG LLVM Github Monorepo