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

Clang Compiler warning: [-Wfloat-equal] #2848

Closed
abdes opened this issue Mar 31, 2022 · 6 comments
Closed

Clang Compiler warning: [-Wfloat-equal] #2848

abdes opened this issue Mar 31, 2022 · 6 comments

Comments

@abdes
Copy link

abdes commented Mar 31, 2022

third_party_deps/fmtlib-src/include/fmt/format.h:2299:53: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
  if (is_constant_evaluated()) return value - value == 0;
                                      ~~~~~~~~~~~~~ ^  ~

please consider disabling the warning in fmtlib itself or defining a float_equal_no_warning function or similar, or using std::equal_to, or comparing to epsilon, etc.

@vitaut
Copy link
Contributor

vitaut commented Mar 31, 2022

A PR to silence this warning would be welcome.

@abdes
Copy link
Author

abdes commented Mar 31, 2022

As it stands today in the current implementation, it does not look like you are doing very accurate floating point comparison. Such comparison is much more complicated, and would require a lot more code, similar to what boost.Math is doing for example.

Let me know if just silencing the warning is ok, or if you intend to have very accurate floating point comparison.

@vitaut
Copy link
Contributor

vitaut commented Apr 1, 2022

It is not a comparison but a fallback implementation of isfinite. The warning is meaningless and should simply be suppressed.

abdes added a commit to abdes/fmt that referenced this issue Apr 1, 2022
@vitaut
Copy link
Contributor

vitaut commented Apr 1, 2022

Suppressed this false positive in ef54f9a.

@vitaut vitaut closed this as completed Apr 1, 2022
@abdes
Copy link
Author

abdes commented Apr 2, 2022

Sorry, but you only partially solved the problem:

[ 33%] Building CXX object test/CMakeFiles/format-test.dir/format-test.cc.o
In file included from /home/xxx/projects/fmt/test/format-test.cc:13:
/home/xxx/projects/fmt/include/fmt/format.h: In instantiation of ‘constexpr bool fmt::v8::detail::isfinite(T) [with T = __float128; typename std::enable_if<std::is_same<T, __float128>::value, int>::type <anonymous> = 0]’:
/home/xxx/projects/fmt/test/format-test.cc:80:3:   required from ‘void check_isfinite() [with Float = __float128]’
/home/xxx/projects/fmt/test/format-test.cc:95:40:   required from here
/home/xxx/projects/fmt/include/fmt/format.h:2308:24: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Wfloat-equal]
 2308 |   return value - value == 0;  // std::isfinite doesn't support __float128.
      |          ~~~~~~~~~~~~~~^~~~

Additionally, not in the API, but in a test file:

[ 83%] Building CXX object test/CMakeFiles/xchar-test.dir/xchar-test.cc.o
/home/xxx/projects/fmt/test/xchar-test.cc: In member function ‘typename FormatContext::iterator fmt::v8::formatter<std::complex<double>, charT>::format(const std::complex<double>&, FormatContext&)’:
/home/xxx/projects/fmt/test/xchar-test.cc:483:31: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Wfloat-equal]
  483 |                      c.real() != 0 ? fmt::format("({}+{}i)", real, imag)
      |                      ~~~~~~~~~^~~~
/home/xxx/projects/fmt/test/xchar-test.cc: In instantiation of ‘typename FormatContext::iterator fmt::v8::formatter<std::complex<double>, charT>::format(const std::complex<double>&, FormatContext&) [with FormatContext = fmt::v8::basic_format_context<fmt::v8::appender, char>; charT = char; typename FormatContext::iterator = fmt::v8::appender]’:
/home/xxx/projects/fmt/include/fmt/core.h:1266:28:   required from ‘static void fmt::v8::detail::value<Context>::format_custom_arg(void*, typename Context::parse_context_type&, Context&) [with T = std::complex<double>; Formatter = fmt::v8::formatter<std::complex<double>, char, void>; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; typename Context::parse_context_type = fmt::v8::basic_format_parse_context<char>]’
/home/xxx/projects/fmt/include/fmt/core.h:1245:19:   required from ‘fmt::v8::detail::value<Context>::value(T&) [with T = const std::complex<double>; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>]’
/home/xxx/projects/fmt/include/fmt/core.h:1727:14:   required from ‘fmt::v8::detail::value<Context> fmt::v8::detail::make_value(T&&) [with Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; T = std::complex<double>&]’
/home/xxx/projects/fmt/include/fmt/core.h:1744:29:   required from ‘fmt::v8::detail::value<Context> fmt::v8::detail::make_arg(T&&) [with bool IS_PACKED = true; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; fmt::v8::detail::type <anonymous> = fmt::v8::detail::type::custom_type; T = std::complex<double>&; typename std::enable_if<IS_PACKED, int>::type <anonymous> = 0]’
/home/xxx/projects/fmt/include/fmt/core.h:1868:77:   required from ‘fmt::v8::format_arg_store<Context, Args>::format_arg_store(T&& ...) [with T = {std::complex<double>&}; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; Args = {std::complex<double>}]’
/home/xxx/projects/fmt/include/fmt/core.h:1885:38:   required from ‘constexpr fmt::v8::format_arg_store<Context, typename std::remove_cv<typename std::remove_reference<Args>::type>::type ...> fmt::v8::make_format_args(Args&& ...) [with Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; Args = {std::complex<double>&}]’
/home/xxx/projects/fmt/include/fmt/core.h:3167:44:   required from ‘std::string fmt::v8::format(fmt::v8::format_string<T ...>, T&& ...) [with T = {std::complex<double>}; std::string = std::__cxx11::basic_string<char>; fmt::v8::format_string<T ...> = fmt::v8::basic_format_string<char, std::complex<double> >]’
/home/xxx/projects/fmt/test/xchar-test.cc:490:30:   required from here
/home/xxx/projects/fmt/test/xchar-test.cc:483:31: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Wfloat-equal]

@vitaut
Copy link
Contributor

vitaut commented Apr 3, 2022

In general fixing all possible warnings is a non-goal but a PR similar to ef54f9a would be welcome. You can also suppress warnings with FMT_SYSTEM_HEADERS.

@vitaut vitaut closed this as completed Apr 3, 2022
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

No branches or pull requests

2 participants