[libc-commits] [PATCH] D106511: [libc] Optimize and standardize the C string functions

[Alf via Phabricator via libc-commits]

gAlfonso-bit added a comment.

In D106511#2906481 <https://reviews.llvm.org/D106511#2906481>, @sivachandra wrote:

> In D106511#2905415 <https://reviews.llvm.org/D106511#2905415>, @gAlfonso-bit wrote:
>
>> Is this good? @sivachandra
>
> A large part of this change is what I would would consider as "churn". So, if you can demonstrate why you want this change, it will be easier to review for people like me (for whom it might not be clear as to why one flavor is better than the other). For example, for code this like:
>
>   const char ch = static_cast<char>(c); // c is of type int
>
> The implicit conversion rules make it equivalent to
>
>   const char ch = c;
>
> Unless you want the code to work under `-Wconversion` or something similar, the explicit cast is redundant. Speaking for myself (as a reviewer),  if you can let us know the intention of your change, I will be able read it with that in my mind.
>
> Don't get me wrong here; I totally appreciate you taking time to do this. In fact, I have already commented earlier that some of the changes you included in here are good from consistency and readability point of view, but that they should be done separately in order not to mix-up concerns.

Well, implicit casts on a foundational library like libc isn't a good idea. A library this important needs all the explicit casts and clear code it can have, whether or not -Wconversion is on


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

https://reviews.llvm.org/D106511