[PATCH] D148318: [clang-tidy] Add `performance-avoid-endl` check

[André Schackier via Phabricator via cfe-commits]

AMS21 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp:48-51
+    const CharSourceRange TokenRange =
+        CharSourceRange::getTokenRange(Expression->getSourceRange());
+    const StringRef SourceText = Lexer::getSourceText(
+        TokenRange, *Result.SourceManager, Result.Context->getLangOpts());
----------------
PiotrZSL wrote:
> entire reading of text is redundant, in log you can put simply:
> `"do not use 'std::endl' with streams; use '\\n' instead"`
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
> 
> ```lang=c++
> std::cout << std::endl; // do not use 'std::endl'
> using namespace std;
> cout << endl; // do not use 'endl'




================
Comment at: clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp:61
+    const auto *CallExpression = llvm::cast<CallExpr>(Expression);
+    assert(CallExpression->getNumArgs() == 1);
+
----------------
PiotrZSL wrote:
> this shouldn't be needed, you already checked that you have 1 argument
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148318/new/

https://reviews.llvm.org/D148318