Harald-R wrote:

>  While I still won't be making a proper review for now, one key improvement 
> could be custom list of "size-functions". > By default it could be 
> size;length (since std::string have length) and users may specify other 
> functions that they have as well.
> Also, consider extending the check to accept free-standing versions of size 
> methods like std::size.

I have pushed an extra commit with these changes.

> One issue I have with this check is that it drastically reduce readability 
> (in my opinion).
> Do you know any projects (preferable medium to big in size) in the wild that 
> already follow this style?

This is a good point, with which I agree. I have tried searching for projects 
that follow this style, but I have not seen it as a convention. There are 
instances of it being used, but they seem to be isolated: `span::subspan` 
implementation in 
[libcxx](https://github.com/llvm/llvm-project/blob/release/22.x/libcxx/include/span#L349-L353),
 
[boost](https://github.com/boostorg/core/blob/a90a31934fe8bcb6e6be6dfea77b80492c7b6c81/include/boost/core/span.hpp#L278-L279)
 or 
[MSVC](https://github.com/microsoft/STL/blob/vs-2022-17.14/stl/inc/span#L414-L416).
 Using `span` for boundary checks would avoid the issue entirely.

I've been searching for how other large projects deal with these cases. I see 
that the Linux Kernel for example uses the 
[__builtin_add_overflow](https://github.com/torvalds/linux/blob/v7.1/tools/include/linux/overflow.h#L44)
 compiler builtin, which means they treat the case where an overflow takes 
place explicitly; same for subtraction and multiplication. Chromium also 
mentions using 
[CheckedNumeric](https://chromium.googlesource.com/chromium/src/+/refs/tags/152.0.7925.1/base/numerics/checked_math.h#51)
 in their [style 
guide](https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++.md#types),
 so it looks like they also check if an overflow occurred and treat it 
explicitly. I suppose these target any arithmetic that could overflow with this 
solution, rather than having something targeted for boundary checks.

https://github.com/llvm/llvm-project/pull/206349
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to