[clang] [clang-format][NFC] Replace a function with StringRef::contains (PR #146245)

Björn Schäpers via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 2 12:38:46 PDT 2025


HazardyKnusperkeks wrote:

> > While it is less code, I find a bit harder to understand and the code gen is far worse: https://gcc.godbolt.org/z/KzG4YnTh3
> 
> `IsBlank` is misleading because of `std::isblank` whereas `Blanks.contains` is not, so the latter has better readability for me. As to the generated code, the latter is worse in runtime (IMO negligible in practice), but the code size is about the same with `-O2` and far better with `-Os` (again, negligible in practice). See [this](https://gcc.godbolt.org/z/YsaKa4hP7) modified example, which adds a call to the local function `IsBlank` to make the comparison fairer.
> 
> More importantly, we wouldn't need to update `IsBlank` if `Blanks` changes. This happened at least once ([580da27](https://github.com/llvm/llvm-project/commit/580da276161ee8b2aa7bc6bd4631cd8261fb9bfb)) before.

To make it fair you have to add the `static` to `IsBlank` and then it gets inlined again.

It's true, that `Blanks` and `IsBlank` have to be maintained simultaneously, but I don't think there will be new _blank_ characters.

And for the readability we obviously disagree.

https://github.com/llvm/llvm-project/pull/146245


More information about the cfe-commits mailing list