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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
