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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
