owenca wrote:

Had we used `StringRef::contains` initially, would you still insist that it be 
replaced with a "more performant" local function? Please note that the code is 
not on a performance critical path, and the difference in time it takes to run 
the relevant unit tests for 100 times is within a few milliseconds on my 
system. For example:
```
time for i in {1..100}; do FormatTests 
--gtest_filter=FormatTestComments\.ReflowsComments* > /dev/null; done
```
Before:
```
real    0m1.244s
user    0m1.098s
sys     0m0.123s
```
After:
```
real    0m1.246s
user    0m1.102s
sys     0m0.121s
```
Runtime aside, it's bad for maintainability to hardcode the characters in 
`Blanks` in a switch statement when there's library function readily available.

https://github.com/llvm/llvm-project/pull/146245
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to