search

mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
993 stars 79 forks source link

Basic units fmt example fails to compile on MSVC #445

Open Cazadorro opened 1 year ago

Cazadorro commented 1 year ago

Given the very basic example, compilation fails, and it fails when using fmt::format. for example, everything is fine, up until line 57:

std::cout << UNITS_STD_FMT::format("{}", v3) << '\n';

where I get:

C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1434~1.319\bin\Hostx86\x64\cl.exe  /nologo /TP -DUNITS_DOWNCAST_MODE=1 -DUNITS_USE_LIBFMT=1 -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\gsl-lite\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-io\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\fmt\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\isq\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\isq-iec80000\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\isq-natural\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-cgs\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-fps\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-international\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-hep\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-iau\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-imperial\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-typographic\include -IC:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\systems\si-uscs\include /DWIN32 /D_WINDOWS /EHsc /Zi /Ob0 /Od /RTC1 -MDd /std:c++20 /utf-8 -std:c++20 /showIncludes /FoCMakeFiles\cpptesting.dir\main.cpp.obj /FdCMakeFiles\cpptesting.dir\ /FS -c C:\Users\user\Documents\GitRepositories\cpptesting\main.cpp

C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<fmt::v9::monostate>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<char>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<bool>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<float>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<double>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<long double>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<char const *>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<fmt::v9::basic_string_view<char> >': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<void const *>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(107) : error C4716: 'units::detail::width_checker::operator()<fmt::v9::basic_format_arg<fmt::v9::basic_format_context<fmt::v9::appender,char> >::handle>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<bool>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<fmt::v9::monostate>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<char>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<float>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<double>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<long double>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<fmt::v9::basic_string_view<char> >': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<char const *>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<void const *>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(122) : error C4716: 'units::detail::precision_checker::operator()<fmt::v9::basic_format_arg<fmt::v9::basic_format_context<fmt::v9::appender,char> >::handle>': must return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(231) : warning C4715: 'units::detail::do_parse_arg_id<char const *,char const *,`units::detail::parse_width<char const *,char const *,fmt::v9::formatter<units::quantity<units::isq::si::dim_speed,units::isq::si::kilometre_per_hour,__int64>,char,void>::spec_handler &>'::`2'::width_adapter &>': not all control paths return a value
C:\Users\user\Documents\GitRepositories\cpptesting\external\units\src\core-fmt\include\units\bits\fmt.h(231) : warning C4715: 'units::detail::do_parse_arg_id<char const *,char const *,`units::detail::parse_precision<char const *,char const *,fmt::v9::formatter<units::quantity<units::isq::si::dim_speed,units::isq::si::kilometre_per_hour,__int64>,char,void>::spec_handler &>'::`2'::precision_adapter &>': not all control paths return a value
ninja: build stopped: subcommand failed.

If I look at the relevant portion of code causing the error, I indeed see this: 


struct width_checker {
  template<typename T>
  [[nodiscard]] constexpr unsigned long long operator()(T value) const
  {
    if constexpr (is_integer<T>) {
      if constexpr (std::numeric_limits<T>::is_signed) {
        if (value < 0) UNITS_THROW(UNITS_STD_FMT::format_error("negative width"));
      }
      return static_cast<unsigned long long>(value);
    } else {
      UNITS_THROW(UNITS_STD_FMT::format_error("width is not integer"));
    }
  }
};

which if the errors are correct, mean that it 100% makes sense why there would be errors here, there's floats and chars being put into this thing with no return path at compile time.

It looks like there's some sort of issue with some sort of std::variant based on the monostate present? But it's not letting me see exactly where the issue is (looks like there's some sort of monkey patching going on, making it impossible for me to diagnose on my own).

mpusz commented 1 year ago

Thanks for reporting! Right now, I literally have no time to work on units (both the mainline and the V2 branch). I will try to look into this issue soon.

jcppe commented 1 year ago

It seems like that the msvc compiler is not able to check that ::fmt::detail::do_throw(x) in fmt_hacks.h always throws. The example compiles when the function call is replaced by throw x. No idea what is the best way to fix this.

mpusz commented 8 months ago

I am sorry that it takes so long to address this issue. We will not fix the old version, and unfortunately, MSVC still crashes in multiple places on the V2 implementation, so we can't check and fix that for you. We made some good progress with those crashes on the last CppCon, though, so hopefully, we will enable MSVC compilation soon.

Cazadorro commented 8 months ago

@mpusz we're fine upgrading to the latest version so no need to patch old versions.