Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support x64-android #371

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FrankXie05
Copy link

Describe the bug

Implicit type conversion rules in C++. In C++, if two values of different types are operated on or compared, the compiler attempts an implicit type conversion to convert one value to the type of the other. This implicit type conversion may result in loss of precision, overflow, or incorrect results。

That's why I used the explicit conversion result.

Environment

OS: Ubuntu 20.04.6 LTS
Compiler: gcc 9.4.0

Use: vcpkg

To reproduce:
Run command:

./vcpkg install rttr:x64-android

@FrankXie05
Copy link
Author

Error log:
install-arm64-android-dbg-out.log

@dg0yt
Copy link

dg0yt commented Mar 13, 2024

Error log: install-arm64-android-dbg-out.log

404 NOT FOUND.

The patch will turn off the compiler message. But will the result still be correct, given different precision of the compared types?

@dg0yt
Copy link

dg0yt commented Mar 13, 2024

Original error from microsoft/vcpkg#37123:

D:/vcpkg/buildtrees/rttr/src/e36f02ecd4-afbbb37f66.clean/src/rttr/../rttr/detail/conversion/number_conversion.h:137:16: error: implicit conversion from 'std::numeric_limits<int>::type' (aka 'int') to 'float' changes value from 2147483647 to 2147483648 [-Werror,-Wimplicit-const-int-float-conversion]
    if (from > std::numeric_limits<T>::max())
             ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/vcpkg/buildtrees/rttr/src/e36f02ecd4-afbbb37f66.clean/src/rttr/../rttr/detail/variant/variant_data_converter.h:1060:16: note: in instantiation of function template specialization 'rttr::detail::convert_to<float, int>' requested here
        return convert_to(from, to);
               ^
...
D:/vcpkg/buildtrees/rttr/src/e36f02ecd4-afbbb37f66.clean/src/rttr/../rttr/detail/conversion/number_conversion.h:139:21: error: implicit conversion from 'std::numeric_limits<int>::type' (aka 'int') to 'float' changes value from -2147483647 to -2147483648 [-Werror,-Wimplicit-const-int-float-conversion]
    else if (from < -std::numeric_limits<T>::max())
                  ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/vcpkg/buildtrees/rttr/src/e36f02ecd4-afbbb37f66.clean/src/rttr/../rttr/detail/conversion/number_conversion.h:137:16: error: implicit conversion from 'std::numeric_limits<long>::type' (aka 'long') to 'float' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-const-int-float-conversion]
    if (from > std::numeric_limits<T>::max())
             ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/vcpkg/buildtrees/rttr/src/e36f02ecd4-afbbb37f66.clean/src/rttr/../rttr/detail/variant/variant_data_converter.h:1065:16: note: in instantiation of function template specialization 'rttr::detail::convert_to<float, long>' requested here
        return convert_to(from, to);
               ^
...

Note the changed values.

@FrankXie05
Copy link
Author

The patch will turn off the compiler message. But will the result still be correct, given different precision of the compared types?

There is a legitimate concern that the comparison of integers and floats is itself an implicit problem.

The solution I can think of is:

  1. Convert them all to the same type to ensure the same accuracy.(double is slightly better than float because the range is larger)
  2. Use epsilon for comparison。

@FrankXie05
Copy link
Author

Like this:

    double max_val = static_cast<double>(std::numeric_limits<T>::max());
    double min_val = static_cast<double>(std::numeric_limits<T>::min());

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.

None yet

2 participants