[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-06 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfac4e3c5f8a0: [clang-tidy] Fix PR26274 (authored by alexfh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70974/new/ https://reviews.llvm.org/D70974

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thanks for the fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70974/new/ https://reviews.llvm.org/D70974 ___

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. Build result: pass - 60438 tests passed, 0 failed and 726 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:79 + } else { // Any other kind of token is unexpected here. +return llvm::None; + } aaron.ballman wrote: > How well do these test

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 232294. alexfh marked 2 inline comments as done. alexfh added a comment. - Added a test with an empty attribute list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70974/new/ https://reviews.llvm.org/D70974

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:79 + } else { // Any other kind of token is unexpected here. +return llvm::None; + } How well do these test cases work? ```

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:54 + const LangOptions ) { + // Loc should be at the begin of the namespace decl (usually, `namespace` +

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D70974#1769036 , @gribozavr2 wrote: > In D70974#1768902 , @aaron.ballman > wrote: > > > In D70974#1768871 , @gribozavr2 > > wrote: > > > > > I'm

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. Build result: pass - 60438 tests passed, 0 failed and 726 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 232140. alexfh marked an inline comment as done. alexfh added a comment. - Added support for inline namespaces and namespace attributes, fixed a typo, added tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. In D70974#1768902 , @aaron.ballman wrote: > In D70974#1768871 , @gribozavr2 > wrote: > > > I'm not

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D70974#1768871 , @gribozavr2 wrote: > I'm not convinced this feature is worth implementing at all, because there's > a good alternative to a macro here -- a namespace alias. What is the reason > to use a macro instead

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a reviewer: gribozavr2. gribozavr2 added a comment. I'm not convinced this feature is worth implementing at all, because there's a good alternative to a macro here -- a namespace alias. What is the reason to use a macro instead of a namespace alias? Repository: rG LLVM

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for working on this! Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:55 - if (!locationsInSameFile(Sources, ND->getBeginLoc(), ND->getRBraceLoc())) + // Ignore namespaces inside macros and namespaces

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-03 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. Build result: pass - 60438 tests passed, 0 failed and 726 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added reviewers: gribozavr, aaron.ballman. Herald added a subscriber: xazax.hun. Herald added a project: clang. This commit fixes http://llvm.org/PR26274 in a simpler and more correct way than 4736d63f752f8d13f4c6a9afd558565c32119718