Skip to content

Conversation

firewave
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator Author

firewave commented Sep 22, 2025

See also #7833 (comment).

Using __int128 it almost fully builds except for the requirement on ValueFlow::Value::equalTo() (suggestions on how to address it in this PR are welcome). The conversion to string for actual 128-bit values does not exist yet but this already exposes overflow issues in the existing code.

Comment on lines +46 to 48
if(USE_INT128 AND USE_BOOST_INT128)
add_definitions(-DHAVE_BOOST_INT128)
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic around USE_BOOST_INT128 is flawed . On is supposed to force the usage of Boost and Auto should provide a fallback to Boost if no built-in type exists but Boost is enabled (i.e. Visual Studio).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm.. how about:

VALUEFLOW_INT128=no
VALUEFLOW_INT128=gcc
VALUEFLOW_INT128=boost

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense to add Builtin and Boost as a configuration options and drop USE_BOOST_INT128 as a whole. That doesn't even require a deprecation since it is experimental and never fully worked.

@firewave firewave marked this pull request as draft September 22, 2025 08:13
@firewave firewave force-pushed the i128 branch 10 times, most recently from 8cea4f1 to 9e1502e Compare September 22, 2025 08:49
@firewave firewave marked this pull request as ready for review September 22, 2025 08:58
Copy link

@firewave
Copy link
Collaborator Author

The conversion to string for actual 128-bit values does not exist yet but this already exposes overflow issues in the existing code.

I filed https://trac.cppcheck.net/ticket/14154 about that.

Copy link
Contributor

@pfultz2 pfultz2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#7658 should be merged first before this pr. This adds another flag that needs to be added every where. #7658 already fixes that so this PR should be based on # 7658.

@firewave firewave marked this pull request as draft October 11, 2025 14:31
Comment on lines +171 to 172
// TODO: requires check fails with GCC using __int128
template<class T, REQUIRES("T must be an arithmetic type", std::is_arithmetic<T> )>
Copy link
Collaborator Author

@firewave firewave Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is fixed in GCC 16.

From https://gcc.gnu.org/gcc-16/changes.html#libstdcxx:

For targets that support [128-bit integers](https://gcc.gnu.org/onlinedocs/gcc/_005f_005fint128.html), std::is_integral<__int128> and similar traits are always true. Previously this was only the case when compiling with GNU dialects (-std=gnu++17, -std=gnu++14, etc.) and not with strict dialects (-std=c++17, etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants