[clang-tidy] Add `performance-avoid-endl` check

[AMS21]

Summary

This check flags uses of std::endl on streams and suggests using the newline character '\n' instead. std::endl performs two operations: it writes a newline character to the output stream and then flushes the stream buffer, which can be less efficient than writing a single newline character using '\n'.

This fixes llvm#35321

[AMS21]

This check is mostly working. Added notes about problems I know about.

clang-tools-extra/clang-tidy/performance/DontUseEndlCheck.cpp

44

This doesn't quite work and I'm not sure why or what would work. Any help would be appreciated.

Report for this like

cpp
std::cout << std::endl;

looks like this:

std::cout << std::endl;
             ^~~~~
             '\n'

So the start location is correct but the end is not.

clang-tools-extra/test/clang-tidy/checkers/performance/dont-use-endl.cpp

45–71

All these test are missing checks for the fixit. See comment above why they are missing.

[carlosgalvezp]

For consistency with other checks, please rename it to performance-avoid-endl. You can use the rename_check.py to easily do this :)

[AMS21]

Rename to performance-avoid-endl

[carlosgalvezp]

Thank you for the contribution! Looks good in general, have minor comments.

clang-tools-extra/docs/clang-tidy/checks/performance/avoid-endl.rst

1 ↗

(On Diff #513582)

Please wrap file to 80 chars

9 ↗

(On Diff #513582)

Nit: maybe document the rationale for using '\n' instead of "\n"? Readability-wise it's more consistent to always use double-quotes.

23 ↗

(On Diff #513582)

The rendering does not display lines numbers so this can be confusing. I believe we can just omit it - it's clear what it's referring too.

23 ↗

(On Diff #513582)

This is repetition of the Rationale on line 8, I would just remove it and say "this gets transformed into:"

clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

47 ↗

(On Diff #513582)

Use CHECK-FIXES to also test the fixes.

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

47 ↗

(On Diff #513582)

Please add the complete error message, which includes the check name in brackets (see similar tests as reference)

[PiotrZSL]

Consider extending this check to suport also std::ends, maybe name it performance-avoid-endl-ends.

clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp

1 ↗

(On Diff #513582)

rename script got problem with this comment, you may need to align it manually

clang-tools-extra/clang-tidy/performance/DontUseEndlCheck.cpp

26

unnecessary limit... (in my project we use custom stream class for logging).

Finder->addMatcher(cxxOperatorCallExpr(unless(isExpansionInSystemHeader()),
                                       hasOverloadedOperatorName("<<"),
                                       unless(isMacroExpansion()),
                                       hasRHS(ignoringImplicit(declRefExpr(to(namedDecl(hasAnyName("endl", "ends")).bind("decl"))).bind("expr")))

something like this should be sufficient...
If you do not plan to remove restriction for basic_ostream, make it configurable.

And other problem is that some << operators does not need to be methods, they can be functions, in such case you may run into issues, but you could just read of type from expr... instead processing argument, or class.

Like `cxxOperatorCallExpr(hasType(references(cxxRecordDecl(....`

44

Use getTokenRange

[njames93]

For what its worth, there typically isn't any performance gain using \n over std::endl when writing to std::cerr or std::clog (or their wide string counterparts) as every write operation on those implicitly calls flush.
However If the fix was changed to convert std::cerr << "Hello" << std::endl into std::cerr << "Hello\n"; That would have a noticeable difference on performance(as it would only be 1 flush rather than 2).

clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp

36 ↗

(On Diff #513582)

Assert on this, all paths should have this node bound.

40 ↗

(On Diff #513582)

Wrap the std::endl in single quotes.

Also it may be wise to spell this how the user has it spelt

std::cout << std::endl; // do not use 'std::endl'
using namespace std;
cout << endl; // do not use 'endl'

clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

8–11 ↗

(On Diff #513582)

These definitions can be elided. Same goes below

24–25 ↗

(On Diff #513582)

Can you also add definitions and tests for std::wostream with std::wcout/std::wcerr

27–28 ↗

(On Diff #513582)

cout and cerr aren't iostreams, they are just ostreams.

46 ↗

(On Diff #513582)

I know it's asking a lot, but it would be amazing if the fix here could be changed to

std::cout << "World\n";

To do this would require checking if the first argument to the endl operator<< call was another operator<< call whose second argument was a string literal.

47 ↗

(On Diff #513582)

That's not necessary. A lot of test files elide this as there is nothing to gain from it.

[AMS21]

Implement suggested fixes

clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

46 ↗

(On Diff #513582)

I agree this would be even cleaner

[njames93]

clang-tools-extra/clang-tidy/performance/AvoidEndlEndsCheck.cpp

28 ↗

(On Diff #513878)

Why are you matching on ends.
That manipulator just inserts a null character to the stream(akin to doing std::cout << '\0';)
More to the point changing replacing this call with std::cout << '\n' changes behaviour and will likely cause UB as this modifier is meant for null-terminating character buffers to pass to C functions.

Double side note the point I made before about how you can turn << "World" << endl into << "World\n" also wouldn't work here as << "World\0" does the same thing as << "World" due to "World" already being a null terminated string.

[PiotrZSL]

clang-tools-extra/clang-tidy/performance/AvoidEndlEndsCheck.cpp

28 ↗

(On Diff #513878)

Good point, so maybe lets get rid std::ends, probably that could be covered in different check. I personalty for my project got ends and endl on forbidd list, but thats more to avoid control characters in logs, we can probably create new check for that case, and don't change this one.

Sorry for confusion...

[AMS21]

Remove std::ends from the check

[AMS21]

I've also notices that we don't handle this case

std::endl(std::cout);

Although a rather unusual thing to use, its still valid and has the same problems.

[AMS21]

Handle the function calling case

[AMS21]

Fix docs

[AMS21]

Minor code cleanup

[PiotrZSL]

Overall looks good, didn't found any bugs, just some potential improvements.

clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp

23 ↗

(On Diff #514908)

Consider using TK_IgnoreUnlessSpelledInSource like in other checks, this should allow you to remove ignoringImplicit.

24 ↗

(On Diff #514908)

cxxOperatorCallExpr inherits from callExpr.
Instead of using here expr, use callExpr, that should allow to filter more ast elements.

28 ↗

(On Diff #514908)

consider ::std::endl unless you plan to support any endl function

30 ↗

(On Diff #514908)

unless(isExpansionInSystemHeader()) is not needed here, you have it in line 24, thats sufficient.

31 ↗

(On Diff #514908)

same here consider ::std::endl

32 ↗

(On Diff #514908)

put this argumentCountIs first, it will be cheaper to check than string compare.

48–51 ↗

(On Diff #514908)

entire reading of text is redundant, in log you can put simply:
"do not use 'std::endl' with streams; use '\\n' instead"

61 ↗

(On Diff #514908)

this shouldn't be needed, you already checked that you have 1 argument

63–66 ↗

(On Diff #514908)

this again looks redundant..., just use std::endl in diag

68–70 ↗

(On Diff #514908)

move this to line 80, you won't need to assign this to variable

clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.h

16 ↗

(On Diff #514908)

sync this text with a description in release notes, and first sentence in documentation

clang-tools-extra/docs/clang-tidy/checks/performance/avoid-endl.rst

7 ↗

(On Diff #514908)

should be '\n', same in documentation, and check comment

40 ↗

(On Diff #514908)

and std::cout, std::cerr, std::clog, std::wcout, std::wcerr and std::wclog.
simply use wording like "standard C++ streams" and then put some examples in ().

41 ↗

(On Diff #514908)

Unless std::ios_base::sync_with_stdio is set to false.

53 ↗

(On Diff #514908)

use 2 spaces instead of 4

[AMS21]

Implement suggested changes.

[AMS21]

clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp

48–51 ↗

(On Diff #514908)

I did this before and it was suggested that we match the users spelling. See njames93 comment above.
But honestly I'm fine either way. Hard coding the std::endl definitely makes the code simpler.

Wrap the std::endl in single quotes.

Also it may be wise to spell this how the user has it spelt

std::cout << std::endl; // do not use 'std::endl'
using namespace std;
cout << endl; // do not use 'endl'

61 ↗

(On Diff #514908)

That's true. I still like to write asserts like this, because the code in this code block requires the argument count to 1. So if something would be changed in the future here, you would hopefully trip the assert and see that the code below needs to be adjusted.

And since this is only checked for debug builds, I don't see the harm here.

[PiotrZSL]

More or less looks good.

[AMS21]

If someone could be so kind as to push this on my behalf, it would be greatly appreciated

clang-tools-extra/clang-tidy/performance/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyPerformanceModule
+  DontUseEndlCheck.cpp
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp
   InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp

clang-tools-extra/clang-tidy/performance/DontUseEndlCheck.h

+//===--- DontUseEndlCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_DONTUSEENDLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_DONTUSEENDLCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// ClangTidyCheck to flag all uses of std::endl on iostreams.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/dont-use-endl.html
+class DontUseEndlCheck : public ClangTidyCheck {
+public:
+  DontUseEndlCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_DONTUSEENDLCHECK_H

clang-tools-extra/clang-tidy/performance/DontUseEndlCheck.cpp

+//===--- DontUseEndlCheck.cpp - clang-tidy --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DontUseEndlCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+void DontUseEndlCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      findAll(cxxOperatorCallExpr(
+          hasOverloadedOperatorName("<<"),
+          callee(cxxMethodDecl(ofClass(
+              hasAnyName("::std::basic_ostream", "::std::basic_iostream")))),
+          hasDescendant(
+              expr(declRefExpr(to(functionDecl(hasName("::std::endl"))))
+                       .bind("endl"))))),
+      this);
+}
+
+void DontUseEndlCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *EndlCall = Result.Nodes.getNodeAs<DeclRefExpr>("endl");
+
+  if (!EndlCall)
+    return;
+
+  auto Diag = diag(EndlCall->getBeginLoc(),
+                   "do not use std::endl with iostreams; use '\\n' instead");
+
+  // Add a fix-it hint to replace std::endl with '\n'
+  Diag << FixItHint::CreateReplacement(
+      CharSourceRange::getCharRange(EndlCall->getSourceRange()), "'\\n'");
+}
+
+} // namespace clang::tidy::performance

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

 //===-- PerformanceTidyModule.cpp - clang-tidy ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "DontUseEndlCheck.h"
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitConversionInLoopCheck.h"
 #include "InefficientAlgorithmCheck.h"
 #include "InefficientStringConcatenationCheck.h"
 #include "InefficientVectorOperationCheck.h"
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoAutomaticMoveCheck.h"
 #include "NoIntToPtrCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
 namespace clang::tidy {
 namespace performance {
 
 class PerformanceModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<DontUseEndlCheck>("performance-dont-use-endl");
     CheckFactories.registerCheck<FasterStringFindCheck>(
         "performance-faster-string-find");
     CheckFactories.registerCheck<ForRangeCopyCheck>(
         "performance-for-range-copy");
     CheckFactories.registerCheck<ImplicitConversionInLoopCheck>(
         "performance-implicit-conversion-in-loop");
     CheckFactories.registerCheck<InefficientAlgorithmCheck>(
         "performance-inefficient-algorithm");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`llvmlibc-inline-function-decl
   tagged with the ``LIBC_INLINE`` macro.
 
 - New :doc:`modernize-type-traits
   <clang-tidy/checks/modernize/type-traits>` check.
 
   Converts standard library type traits of the form ``traits<...>::type`` and
   ``traits<...>::value`` into ``traits_t<...>`` and ``traits_v<...>`` respectively.
 
+- New :doc:`performance-dont-use-endl
+  <clang-tidy/checks/performance/dont-use-endl>` check.
+
+  Finds uses of ``std::endl`` on streams and replaces them with ``'\n'``.
+
 - New :doc:`readability-avoid-unconditional-preprocessor-if
   <clang-tidy/checks/readability/avoid-unconditional-preprocessor-if>` check.
 
   Finds code blocks that are constantly enabled or disabled in preprocessor
   directives by analyzing ``#if`` conditions, such as ``#if 0`` and
   ``#if 1``, etc.
 
 - New :doc:`readability-operators-representation

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `objc-forbidden-subclassing <objc/forbidden-subclassing.html>`_,
    `objc-missing-hash <objc/missing-hash.html>`_,
    `objc-nsdate-formatter <objc/nsdate-formatter.html>`_,
    `objc-nsinvocation-argument-lifetime <objc/nsinvocation-argument-lifetime.html>`_, "Yes"
    `objc-property-declaration <objc/property-declaration.html>`_, "Yes"
    `objc-super-self <objc/super-self.html>`_, "Yes"
    `openmp-exception-escape <openmp/exception-escape.html>`_,
    `openmp-use-default-none <openmp/use-default-none.html>`_,
+   `performance-dont-use-endl <performance/dont-use-endl.html>`_, "Yes"
    `performance-faster-string-find <performance/faster-string-find.html>`_, "Yes"
    `performance-for-range-copy <performance/for-range-copy.html>`_, "Yes"
    `performance-implicit-conversion-in-loop <performance/implicit-conversion-in-loop.html>`_,
    `performance-inefficient-algorithm <performance/inefficient-algorithm.html>`_, "Yes"
    `performance-inefficient-string-concatenation <performance/inefficient-string-concatenation.html>`_,
    `performance-inefficient-vector-operation <performance/inefficient-vector-operation.html>`_, "Yes"
    `performance-move-const-arg <performance/move-const-arg.html>`_, "Yes"
    `performance-move-constructor-init <performance/move-constructor-init.html>`_,

clang-tools-extra/docs/clang-tidy/checks/performance/dont-use-endl.rst

+.. title:: clang-tidy - performance-dont-use-endl
+
+performance-dont-use-endl
+=========================
+
+Checks for uses of ``std::endl`` on iostreams and suggests using the newline character ``'\n'`` instead.
+
+Rationale:
+Using ``std::endl`` on iostreams can be less efficient than using the newline character ``'\n'`` because ``std::endl`` performs two operations: it writes a newline character to the output stream and then flushes the stream buffer. Writing a single newline character using ``'\n'`` does not trigger a flush, which can improve performance. In addition, flushing the stream buffer can cause additional overhead when working with streams that are buffered.
+
+Example:
+
+Consider the following code:
+
+.. code-block:: c++
+
+    #include <iostream>
+
+    int main() {
+        std::cout << "Hello" << std::endl;
+    }
+
+The ``std::endl`` on line 4 performs two operations: it writes a newline character to the ``std::cout`` stream and then flushes the stream buffer. This can be less efficient than using the newline character ``'\n'`` instead:
+
+.. code-block:: c++
+    #include <iostream>
+
+    int main() {
+        std::cout << "Hello" << '\n';
+    }
+
+This code writes a single newline character to the ``std::cout`` stream without flushing the stream buffer.
+
+If you do need to flush the stream buffer explicitly, you can just use ``std::flush``.

clang-tools-extra/test/clang-tidy/checkers/performance/dont-use-endl.cpp

+// RUN: %check_clang_tidy %s performance-dont-use-endl %t
+
+namespace std
+{
+  template <typename CharT>
+  class basic_ostream {
+    public:
+    template <typename T>
+    basic_ostream& operator<<(T) {
+      return *this;
+    }
+
+    basic_ostream& operator<<( basic_ostream<CharT>& (*func)
+        (basic_ostream<CharT>&))
+    {
+      return func(*this);
+    }
+  };
+
+  template <typename CharT>
+  class basic_iostream : public basic_ostream<CharT> {
+  };
+
+  using ostream = basic_ostream<char>;
+  using iostream = basic_iostream<char>;
+
+  iostream cout;
+  iostream cerr;
+
+  template<typename CharT>
+  basic_ostream<CharT>& endl(basic_ostream<CharT>& os)
+  {
+    return os;
+  }
+} // namespace std
+
+void good() {
+  std::cout << "Hello" << '\n';
+  std::cout << "World\n";
+
+  std::cerr << "Hello" << '\n';
+  std::cerr << "World\n";
+}
+
+void bad() {
+  std::cout << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with iostreams; use '\n' instead
+  std::cerr << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with iostreams; use '\n' instead
+}
+
+void bad_single_argument() {
+  std::cout << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use std::endl with iostreams; use '\n' instead
+  std::cerr << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use std::endl with iostreams; use '\n' instead
+}
+
+void bad_multiple() {
+  std::cout << "Hello" << std::endl << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with iostreams; use '\n' instead
+  std::cerr << "Hello" << std::endl << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with iostreams; use '\n' instead
+}
+
+void bad_user_stream() {
+  std::iostream my_stream;
+
+  my_stream << "Hi" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not use std::endl with iostreams; use '\n' instead
+}