Jens Geyer created THRIFT-5355:
----------------------------------
Summary: Do not rely on compiler and check boundaries
Key: THRIFT-5355
URL: https://issues.apache.org/jira/browse/THRIFT-5355
Project: Thrift
Issue Type: Improvement
Components: C++ - Library
Reporter: Jens Geyer
C++ considers the overflow of a signed integer to be an undefined behavior
(even with the "Signed Integers are Two's Complement" update:
[http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r1.html] ).
Instead of relying on tests(1) to discover if a compiler does not handle the
signed integer overflow as we expect it to, we should add an explicit check
before incrementing.
(1) See
[thrift/lib/cpp/src/thrift/async/TConcurrentClientSyncInfo.cpp|https://github.com/apache/thrift/blob/c4e899a6d64aa97430ec9f7608d38db2095f6159/lib/cpp/src/thrift/async/TConcurrentClientSyncInfo.cpp#L33-L34]
Lines 33 to 34 in
[c4e899a|https://github.com/apache/thrift/commit/c4e899a6d64aa97430ec9f7608d38db2095f6159]
| |// test rollover all the time|
| |nextseqid_((std::numeric_limits<int32_t>::max)()-10),|
Quick check on [https://godbolt.org/z/d7W8ds] shows that Clang is able to
optimize and just do the +1 so modern compilers should be able to keep the same
performances where hardware permits.
In addition, some company rules might require code to do just that:
[https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
