[PATCH] D147194: [clang-tidy] fix concat-nest-namespace fix hint remove the macro
Piotr Zegar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 30 12:29:21 PDT 2023
PiotrZSL added a comment.
You changed half of this check logic, please take a look into those issues for this check:
https://github.com/llvm/llvm-project/issues/60051
https://github.com/llvm/llvm-project/issues/60048
https://github.com/llvm/llvm-project/issues/60035
https://github.com/llvm/llvm-project/issues/57530
https://github.com/llvm/llvm-project/issues/56022
https://github.com/llvm/llvm-project/issues/43351
https://github.com/llvm/llvm-project/issues/41015
Add issues that you fixing to commit message.
Also look into these patches:
https://reviews.llvm.org/D61989
https://reviews.llvm.org/D141787
Update release notes (add info about fixes for this check).
Would be good to run it on some code to check how this behave, except that looks fine.
================
Comment at: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:49
const LangOptions &LangOpts) {
// FIXME: This logic breaks when there is a comment with ':'s in the middle.
+ return getRawStringRef(ReplacementRange, Sources, LangOpts).count(':') ==
----------------
check if you can do something with this...
add test for:
with something like:
```
namespace x1 {
// namespace x3::x4 {
namespace x2 {
void foo();
}
// }
}
```
================
Comment at: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:70
+ File.data() + LocInfo.second, File.end());
+ L.SetCommentRetentionState(WithComment);
+ // Find the token.
----------------
HerrCai0907 wrote:
> This part of code copy from `Lexer::findNextToken` except this line. Should I refactor `Lexer::findNextToken` for example add a parameter to control it?
> But `Lexer::findNextToken` is involved with lots of code. Maybe use another patch to do it?
Dont change Lexer.
Look into clang-tools-extra/clang-tidy/utils/LexerUtils.h, if you can find something useful there,
if not then move this function there.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h:8
// CHECK-FIXES: void t();
-// CHECK-FIXES-NEXT: } // namespace nn1
+// CHECK-FIXES-NEXT: } // namespace nn2
----------------
is check also fixes namespace comments ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147194/new/
https://reviews.llvm.org/D147194
More information about the cfe-commits
mailing list