[clang-tools-extra] [clang-tidy] add default error message for performance-avoid-endl (PR #107867)

Congcong Cai via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 13 06:09:03 PDT 2024


https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/107867

>From 31b01c0cd3fc809e5451853b820d021dd43d16c7 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Mon, 9 Sep 2024 22:32:48 +0800
Subject: [PATCH 1/7] [clang-tidy] add default error message for
 performance-avoid-endl

use ::std::endl as default message when matched expr does not have valid source text
---
 .../clang-tidy/performance/AvoidEndlCheck.cpp       | 13 ++++++-------
 clang-tools-extra/docs/ReleaseNotes.rst             |  4 ++++
 .../clang-tidy/checkers/performance/avoid-endl.cpp  | 11 +++++++++++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
index 8ecaa41754fb2b..4cfe1ca49f317c 100644
--- a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
@@ -46,14 +46,13 @@ void AvoidEndlCheck::check(const MatchFinder::MatchResult &Result) {
     // Handle the more common streaming '... << std::endl' case
     const CharSourceRange TokenRange =
         CharSourceRange::getTokenRange(Expression->getSourceRange());
-    const StringRef SourceText = Lexer::getSourceText(
+    StringRef SourceText = Lexer::getSourceText(
         TokenRange, *Result.SourceManager, Result.Context->getLangOpts());
-
-    auto Diag = diag(Expression->getBeginLoc(),
-                     "do not use '%0' with streams; use '\\n' instead")
-                << SourceText;
-
-    Diag << FixItHint::CreateReplacement(TokenRange, "'\\n'");
+    if (SourceText.empty())
+      SourceText = "::std::endl";
+    diag(Expression->getBeginLoc(),
+         "do not use '%0' with streams; use '\\n' instead")
+        << SourceText << FixItHint::CreateReplacement(TokenRange, "'\\n'");
   } else {
     // Handle the less common function call 'std::endl(...)' case
     const auto *CallExpression = llvm::cast<CallExpr>(Expression);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8d028f8863cb7a..6768e243add997 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,10 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-std-print>` check to support replacing
   member function calls too.
 
+- Improved :doc:`performance-avoid-endl
+  <clang-tidy/checks/performance/avoid-endl>` check by fixing incorrect
+  message.
+
 - Improved :doc:`readablility-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check
   by adding the option `UseUpperCaseLiteralSuffix` to select the
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
index 462b88773d231c..dd2d61a78d7965 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
@@ -225,3 +225,14 @@ void bad_custom_stream() {
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
   // CHECK-FIXES: logger << '\n';
 }
+
+namespace gh107859 {
+
+#define ENDL std::endl;
+
+void bad_macro() {
+  std::cout << ENDL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use '::std::endl' with streams; use '\n' instead [performance-avoid-endl]
+}
+
+} // namespace gh107859

>From 0af236e834483ee29189a4e3fa566647b8dea5a3 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 10 Sep 2024 07:21:10 +0800
Subject: [PATCH 2/7] Update
 clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp

---
 clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
index 4cfe1ca49f317c..bc12b9c73c0d3c 100644
--- a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
@@ -49,7 +49,7 @@ void AvoidEndlCheck::check(const MatchFinder::MatchResult &Result) {
     StringRef SourceText = Lexer::getSourceText(
         TokenRange, *Result.SourceManager, Result.Context->getLangOpts());
     if (SourceText.empty())
-      SourceText = "::std::endl";
+      SourceText = "std::endl";
     diag(Expression->getBeginLoc(),
          "do not use '%0' with streams; use '\\n' instead")
         << SourceText << FixItHint::CreateReplacement(TokenRange, "'\\n'");

>From 89313a20fbcd24c7ffe95b6ba9217e2629898f6f Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 10 Sep 2024 07:21:19 +0800
Subject: [PATCH 3/7] Update
 clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

---
 .../test/clang-tidy/checkers/performance/avoid-endl.cpp         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
index dd2d61a78d7965..df5ff56c827f7f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
@@ -232,7 +232,7 @@ namespace gh107859 {
 
 void bad_macro() {
   std::cout << ENDL;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use '::std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
 }
 
 } // namespace gh107859

>From f0cdfb49fc95d440ee06054b87f1e697af8db016 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 12 Sep 2024 12:28:22 +0800
Subject: [PATCH 4/7] fix

---
 .../clang-tidy/performance/AvoidEndlCheck.cpp             | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
index bc12b9c73c0d3c..e4ce253568563f 100644
--- a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
@@ -50,9 +50,11 @@ void AvoidEndlCheck::check(const MatchFinder::MatchResult &Result) {
         TokenRange, *Result.SourceManager, Result.Context->getLangOpts());
     if (SourceText.empty())
       SourceText = "std::endl";
-    diag(Expression->getBeginLoc(),
-         "do not use '%0' with streams; use '\\n' instead")
-        << SourceText << FixItHint::CreateReplacement(TokenRange, "'\\n'");
+    auto Builder = diag(Expression->getBeginLoc(),
+                        "do not use '%0' with streams; use '\\n' instead");
+    if (TokenRange.isValid())
+      Builder << SourceText
+              << FixItHint::CreateReplacement(TokenRange, "'\\n'");
   } else {
     // Handle the less common function call 'std::endl(...)' case
     const auto *CallExpression = llvm::cast<CallExpr>(Expression);

>From d7ce4dcb3932f5092af8667c57b6d47248c2d7af Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcong.cai at bmw.com>
Date: Thu, 12 Sep 2024 21:58:48 +0800
Subject: [PATCH 5/7] fix

---
 .../clang-tidy/performance/AvoidEndlCheck.cpp             | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
index e4ce253568563f..df50ba5bb6b57a 100644
--- a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
@@ -50,11 +50,11 @@ void AvoidEndlCheck::check(const MatchFinder::MatchResult &Result) {
         TokenRange, *Result.SourceManager, Result.Context->getLangOpts());
     if (SourceText.empty())
       SourceText = "std::endl";
-    auto Builder = diag(Expression->getBeginLoc(),
-                        "do not use '%0' with streams; use '\\n' instead");
+    auto Diag = diag(Expression->getBeginLoc(),
+                     "do not use '%0' with streams; use '\\n' instead")
+                << SourceText;
     if (TokenRange.isValid())
-      Builder << SourceText
-              << FixItHint::CreateReplacement(TokenRange, "'\\n'");
+      Diag << FixItHint::CreateReplacement(TokenRange, "'\\n'");
   } else {
     // Handle the less common function call 'std::endl(...)' case
     const auto *CallExpression = llvm::cast<CallExpr>(Expression);

>From c2e6983273cc239037329990ef45ddda62b402c9 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcong.cai at bmw.com>
Date: Fri, 13 Sep 2024 21:02:09 +0800
Subject: [PATCH 6/7] update release note

---
 clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6768e243add997..abb59364fbe45f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,8 +121,8 @@ Changes in existing checks
   member function calls too.
 
 - Improved :doc:`performance-avoid-endl
-  <clang-tidy/checks/performance/avoid-endl>` check by fixing incorrect
-  message.
+  <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as
+  placeholder when lexer cannot get source text.
 
 - Improved :doc:`readablility-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check

>From b76459833500d945a5186237428034f700f409cb Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcong.cai at bmw.com>
Date: Fri, 13 Sep 2024 21:08:47 +0800
Subject: [PATCH 7/7] fix function call also

---
 .../clang-tidy/performance/AvoidEndlCheck.cpp | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
index df50ba5bb6b57a..a394f5c6efa2a4 100644
--- a/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
@@ -60,25 +60,27 @@ void AvoidEndlCheck::check(const MatchFinder::MatchResult &Result) {
     const auto *CallExpression = llvm::cast<CallExpr>(Expression);
     assert(CallExpression->getNumArgs() == 1);
 
-    const StringRef SourceText = Lexer::getSourceText(
+    StringRef SourceText = Lexer::getSourceText(
         CharSourceRange::getTokenRange(
             CallExpression->getCallee()->getSourceRange()),
         *Result.SourceManager, Result.Context->getLangOpts());
+    if (SourceText.empty())
+      SourceText = "std::endl";
+    auto Diag = diag(CallExpression->getBeginLoc(),
+                     "do not use '%0' with streams; use '\\n' instead")
+                << SourceText;
 
     const CharSourceRange ArgTokenRange = CharSourceRange::getTokenRange(
         CallExpression->getArg(0)->getSourceRange());
     const StringRef ArgSourceText = Lexer::getSourceText(
         ArgTokenRange, *Result.SourceManager, Result.Context->getLangOpts());
-
-    const std::string ReplacementString =
-        std::string(ArgSourceText) + " << '\\n'";
-
-    diag(CallExpression->getBeginLoc(),
-         "do not use '%0' with streams; use '\\n' instead")
-        << SourceText
-        << FixItHint::CreateReplacement(
-               CharSourceRange::getTokenRange(CallExpression->getSourceRange()),
-               ReplacementString);
+    const CharSourceRange ReplacementRange =
+        CharSourceRange::getTokenRange(CallExpression->getSourceRange());
+    if (!ArgSourceText.empty() && ReplacementRange.isValid()) {
+      const std::string ReplacementString =
+          std::string(ArgSourceText) + " << '\\n'";
+      Diag << FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
+    }
   }
 }
 



More information about the cfe-commits mailing list