njames93 added a comment.
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).
================
Comment at: clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp:36
+
+ if (!EndlCall)
+ return;
----------------
Assert on this, all paths should have this node bound.
================
Comment at: clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp:40
+ auto Diag = diag(EndlCall->getBeginLoc(),
+ "do not use std::endl with iostreams; use '\\n' instead");
+
----------------
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/test/clang-tidy/checkers/performance/avoid-endl.cpp:8-11
+ template <typename T>
+ basic_ostream& operator<<(T) {
+ return *this;
+ }
----------------
These definitions can be elided. Same goes below
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp:24-25
+
+ using ostream = basic_ostream<char>;
+ using iostream = basic_iostream<char>;
+
----------------
Can you also add definitions and tests for `std::wostream` with
`std::wcout`/`std::wcerr`
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp:27-28
+
+ iostream cout;
+ iostream cerr;
+
----------------
`cout` and `cerr` aren't `iostreams`, they are just `ostreams`.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp:46
+void bad() {
+ std::cout << "World" << std::endl;
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with
iostreams; use '\n' instead
----------------
I know it's asking a lot, but it would be amazing if the fix here could be
changed to
```lang=c++
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.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp:47
+ 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;
----------------
carlosgalvezp wrote:
> carlosgalvezp wrote:
> > Use CHECK-FIXES to also test the fixes.
> Please add the complete error message, which includes the check name in
> brackets (see similar tests as reference)
That's not necessary. A lot of test files elide this as there is nothing to
gain from it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148318/new/
https://reviews.llvm.org/D148318
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
