llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Tudor (kvxb) <details> <summary>Changes</summary> …tion The readability-isolate-declaration check produces incorrect fix-its when a declarator list spans multiple lines and a comment appears after a comma. Previously, comments were placed between the type and the next declarator, producing misleading code. This patch updates createIsolatedDecls to detect when the first line of a subsequent declarator's snippet consists entirely of a comment. Such comments are now moved to the end of the previous declaration, preserving their original indentation. Comments that share a line with a declarator are left in place. Fixes #<!-- -->226670. Note: Used claude for partial code generation. --- Full diff: https://github.com/llvm/llvm-project/pull/237775.diff 2 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp (+55-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration.cpp (+15-3) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp index 07673e3207f3bf..8493281d74aa3f 100644 --- a/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp @@ -226,6 +226,33 @@ collectSourceRanges(llvm::ArrayRef<SourceRange> Ranges, const SourceManager &SM, return Snippets; } +/// Strips leading line (//) and block (/* */) comments from a snippet, +/// along with surrounding whitespace. Returns the remaining text, or an +/// empty StringRef if the snippet consists entirely of comments. +static StringRef stripLeadingComments(StringRef Snippet) { + Snippet = Snippet.ltrim(); + while (!Snippet.empty()) { + if (Snippet.starts_with("//")) { + size_t Newline = Snippet.find('\n'); + if (Newline == StringRef::npos) + return ""; // Entire snippet is a comment. + Snippet = Snippet.substr(Newline + 1).ltrim(); + } else if (Snippet.starts_with("/*")) { + size_t End = Snippet.find("*/"); + if (End == StringRef::npos) + return Snippet; // Malformed block comment + Snippet = Snippet.substr(End + 2).ltrim(); + } else { + break; + } + } + return Snippet; +} + +/// Constructs isolated declaration strings from extracted snippets. +/// The first element of Snippets is the type, followed by each declarator. +/// For subsequent declarations, comments that appear on their own line before +/// the declarator are moved to the end of the previous declaration. /// Expects a vector {TypeSnippet, Firstdecl, SecondDecl, ...}. static std::vector<std::string> createIsolatedDecls(llvm::ArrayRef<StringRef> Snippets) { @@ -233,12 +260,38 @@ createIsolatedDecls(llvm::ArrayRef<StringRef> Snippets) { assert(Snippets.size() > 2 && "Not enough snippets to create isolated decls"); std::vector<std::string> Decls(Snippets.size() - 1); - for (std::size_t I = 1; I < Snippets.size(); ++I) + for (std::size_t I = 1; I < Snippets.size(); ++I) { + StringRef DeclSnippet = Snippets[I]; + + // For subsequent declarations, check if a leading comment belongs to the + // previous declaration. + if (I > 1) { + StringRef FirstLine = DeclSnippet.split('\n').first; + StringRef FirstLineStripped = stripLeadingComments(FirstLine); + + // If the first line consists entirely of a comment, it belongs to the + // previous declaration. + if (FirstLineStripped.trim().empty()) { + StringRef FullyStripped = stripLeadingComments(DeclSnippet); + + if (!FullyStripped.ltrim().empty()) { + StringRef CommentPart = + DeclSnippet.substr(0, DeclSnippet.size() - FullyStripped.size()); + CommentPart = CommentPart.rtrim(); + + if (!CommentPart.empty()) + Decls[I - 2] += CommentPart.str(); + DeclSnippet = FullyStripped; + } + } + } + Decls[I - 1] = Twine(Snippets[0]) .concat(Snippets[0].ends_with(' ') ? "" : " ") - .concat(Snippets[I].ltrim()) + .concat(DeclSnippet.ltrim()) .concat(";") .str(); + } return Decls; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration.cpp index 94b6f7f1a5d91b..a2eb2e1c73f480 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration.cpp @@ -25,9 +25,21 @@ void f2() { ; // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability // CHECK-FIXES: int /* here is a comment */ v1; - // CHECK-FIXES: int /* here is a comment */ // another comment - // CHECK-FIXES: v2 = 42 // Ok, more comments - // CHECK-FIXES: ; + // CHECK-FIXES: // another comment + // CHECK-FIXES: int /* here is a comment */ v2 = 42 // Ok, more comments + // CHECK-FIXES: ; + + int a, // comment a + b; // comment b + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int a; // comment a + // CHECK-FIXES: int b; // comment b + + int t, /* comment t */ + v; /* comment v */ + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int t; /* comment t */ + // CHECK-FIXES: int v; /* comment v */ auto int1 = 42, int2 = 0, int3 = 43; // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability `````````` </details> https://github.com/llvm/llvm-project/pull/237775 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
