mpusz / mp-units

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

Fail to compile v2.0.0 with MSVC #518

Open nniclausse opened 10 months ago

nniclausse commented 10 months ago

Hi

I'm trying to compile mp-units 2.0.0 on windows with Visual Studio (tried with 16 and 17), but it fails. It was working well with v0.8.0

I'm using cmake directly (i'm building a conda package of mp-units btw), and got a lot of compilers errors:

 Building Custom Rule H:/Mambaforge/conda-bld/mp-units_1699864922028/work/example/CMakeLists.txt
ClCompile:
  C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\bin\HostX64\x64\CL.exe /c /I"%SRC_DIR%\src\core-io\include" /I"%SRC_DIR%\src\core\include" /I"%SRC_DIR%\src\systems\si\include" /I"%SRC_DIR%\src\systems
\isq\include" /I"%SRC_DIR%\src\systems\cgs\include" /I"%SRC_DIR%\src\systems\usc\include" /I"%SRC_DIR%\src\systems\international\include" /nologo /W4 /WX- /diagnostics:column /O2 /Ob2 /D _MBCS /D WIN32 /D _WINDOWS /D NDEBUG /D "CMAKE_IN
TDIR=\"Release\"" /Gm- /EHsc /MD /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /std:c++20 /permissive- /Fo"avg_speed.dir\Release\\" /Fd"avg_speed.dir\Release\vc143.pdb" /external:W0 /Gd /TP /errorReport:queue /we4289  /externa
l:I "H:/Mambaforge/conda-bld/mp-units_1699864922028/_h_env/Library/include" /w14062 /w14242 /w14254 /w14263 /w14265 /w14266 /w14287 /w14296 /w14311 /w14545 /w14546 /w14547 /w14549 /w14555 /w14619 /w14640 /w14826 /w14905 /w14906 /w14928 
/utf-8 "%SRC_DIR%\example\avg_speed.cpp"
  avg_speed.cpp
%SRC_DIR%\src\core\include\mp-units/unit.h(410,31): error C7601: the associated constraints are not satisfied [%SRC_DIR%\build\example\avg_speed.vcxproj]
%SRC_DIR%\src\core\include\mp-units/unit.h(405,34): message : the concept 'mp_units::Unit<mp_units::scaled_unit<M{},mp_units::one>>' evaluated to false [%SRC_DIR%\build\example\avg_speed.vcxproj]
          with
          [
              M=mp_units::magnitude<mp_units::power_v<2,-2>{},mp_units::power_v<5,-2>{}>
          ]
%SRC_DIR%\src\core\include\mp-units/bits/unit_concepts.h(46,16): message : the constraint was not satisfied [%SRC_DIR%\build\example\avg_speed.vcxproj]
%SRC_DIR%\src\core\include\mp-units/unit.h(587,70): message : see reference to function template instantiation 'auto mp_units::operator *<_T0,mp_units::one>(M,const U)' being compiled [%SRC_DIR%\build\example\avg_speed.vcxproj]
          with
          [
              _T0=mp_units::magnitude<mp_units::power_v<2,-2>{},mp_units::power_v<5,-2>{}>,
              M=mp_units::magnitude<mp_units::power_v<2,-2>{},mp_units::power_v<5,-2>{}>,
              U=mp_units::one
          ]
%SRC_DIR%\src\core\include\mp-units/unit.h(587,35): error C2504: 'mp_units::named_unit<mp_units::basic_symbol_text<char,1,1>{mp_units::basic_fixed_string<char,1>{CharT37,0},mp_units::basic_fixed_string<CharT,1>{CharT37,0}},mp_units::sca
led_unit<M{},mp_units::one>{}>': base class undefined [%SRC_DIR%\build\example\avg_speed.vcxproj]
          with
          [
              CharT=char,
              M=mp_units::magnitude<mp_units::power_v<2,-2>{},mp_units::power_v<5,-2>{}>
          ]
%SRC_DIR%\src\core\include\mp-units/unit.h(588,37): error C2504: 'mp_units::named_unit<mp_units::basic_symbol_text<char,3,2>{mp_units::basic_fixed_string<char,3>{CharT226,128,176,0},mp_units::basic_fixed_string<CharT,2>{CharT37,111,0}},
mp_units::scaled_unit<M{},mp_units::one>{}>': base class undefined [%SRC_DIR%\build\example\avg_speed.vcxproj]
          with
          [
              CharT=char,
              M=mp_units::magnitude<mp_units::power_v<2,-3>{},mp_units::power_v<5,-3>{}>
          ]
%SRC_DIR%\src\core\include\mp-units/reference.h(76,73): error C2059: syntax error: '<end Parse>' [%SRC_DIR%\build\example\avg_speed.vcxproj]
%SRC_DIR%\src\core\include\mp-units/reference.h(156,2): message : see reference to class template instantiation 'mp_units::reference<Q,U>' being compiled [%SRC_DIR%\build\example\avg_speed.vcxproj]
%SRC_DIR%\src\core\include\mp-units/reference.h(76,79): error C2143: syntax error: missing ';' before '{' [%SRC_DIR%\build\example\avg_speed.vcxproj]
%SRC_DIR%\src\core\include\mp-units/reference.h(76,73): error C2143: syntax error: missing '>' before ';' [%SRC_DIR%\build\example\avg_speed.vcxproj]
...
[SKIP a lot of errors]
...
  %SRC_DIR%\src\systems\angular\include\mp-units/systems/angular/angular.h(51,23): error C2737: 'mp_units::angular::unit_symbols::deg2': constexpr object must be initialized [%SRC_DIR%\build\example\unmanned_aerial_vehicle.vcxproj]     
  %SRC_DIR%\src\core\include\mp-units/quantity_spec.h(1344,27): error C2131: expression did not evaluate to a constant [%SRC_DIR%\build\example\unmanned_aerial_vehicle.vcxproj]
  %SRC_DIR%\src\core\include\mp-units/quantity_spec.h(355,90): fatal  error C1903: unable to recover from previous error(s); stopping compilation [%SRC_DIR%\build\example\unmanned_aerial_vehicle.vcxproj]

    0 Warning(s)
    3153 Error(s)

Do you plan to support Visual studio again ? Or did i miss something ? Thanks.

JohelEGP commented 10 months ago

Do you plan to support Visual studio again ? Or did i miss something ? Thanks.

Yeah. This was mentioned in the latest talk: https://github.com/mpusz/mp-units/discussions/451#discussioncomment-7316987.

mpusz commented 10 months ago

@nniclausse, you can find the list of supported compilers at https://mpusz.github.io/mp-units/2.1/getting_started/installation_and_usage/. Unfortunately, MSVC is the worst one with C++20 support now. We submitted many bugs to MSVC teams, and we are waiting for them to be resolved so we may continue our efforts to make the project compile on it.

nniclausse commented 10 months ago

ok, thanks for your feedback, i'll try clang on windows

dpservis commented 6 months ago

Hi, is MSVC still out of the list? (a list of supported compilers in the GitHub page would help).

mpusz commented 6 months ago

Yes, MSVC still does not properly support C++20.

The list of supported compilers is provided on the main page of our docs (https://mpusz.github.io/mp-units/2.2).

dpservis commented 6 months ago

Yes, MSVC still does not properly support C++20.

The list of supported compilers is provided on the main page of our docs (https://mpusz.github.io/mp-units/2.2).

Thanks a lot. It seems to me there is no way to bypass the problem, I have significant issues with the concepts that are not properly evaluated...

aber-code commented 3 months ago

Is there a list with corresponding bugs you submitted? Having this list would enable me to track them and give them a vote.

mpusz commented 3 months ago

Unfortunately, there is no list, and I did not submit the bugs. Also, many bugs are still not submitted as no one had time to investigate and isolate them. Diagnostics (error messages) suck in MSVC, so it is not easy to find the root cause of the problem.

mpusz commented 3 months ago

However, one of the MSVC developers promised that they would try to enable mp-units internally for their CI. Hopefully, this will push things forward.

connorjak commented 1 month ago

My team is also blocked by this; using Unreal Engine 5.4 locks us into MSVC for Windows.

connorjak commented 1 month ago

Here's the relevant part of my error log:

mp-units\framework\reference.h(90): error C2275: 'Q': expected an expression instead of a type
mp-units/framework/reference.h(90): note: the template instantiation context (the oldest one first) is
mp-units/framework/reference.h(76): note: while compiling class template 'mp_units::reference'
... then a lot of syntax errors from the {} not 

Referenced line: https://github.com/mpusz/mp-units/blob/ab51b47328c2dadace2ab934dbaedc4e275a4cda/src/core/include/mp-units/framework/reference.h#L89-L93

czjhoppe commented 1 month ago

I have created a fork of mp-units where I fixed all the bugs that occur with MSVC. These fixes are for version 2.2 and can be found in the branch 2.2-msvc-194: mp-units branch.

Most of the fixes are due to bugs in the MSVC compiler. Whenever I was able to reproduce the behavior in a small snippet, I created an issue to report it. Here are the links to the issues:

  1. Discrepancy in Behavior of operator= and operator for Multiplying int and double at compile time
  2. Syntax error when using non-type template parameters in templated class member function
  3. Type always prefered over value when using qualified identifiers

However, there are still some bugs that I could not reproduce and do not understand well enough to report an issue.

Additionally, I have found some issues in mp-units that will not be solved with a new MSVC compiler version. Should we discuss these issues here, or should I open separate issues?"

mpusz commented 1 month ago

@czjhoppe, WOW, that is excellent news and huge work! Thank you!

Could you please provide a PR with those? It would be great if MSVC workarounds could be somehow marked with a preprocessor macro (like others already provided in hacks.h) so they would be easier to remove when MSVC catches up and also easier for other developers and contributors to understand when they read the code (some workarounds are not the best coding practices šŸ˜‰).

Regarding "issues in mp-units that will not be solved with a new MSVC compiler version", I would like to understand it a bit more. Is it a bug in mp-units, or is it just a feature that is not in the MSVC roadmap for now? If those are mp-units bugs, you can open PRs right away. We may discuss the rest here if needed.

Thanks again!

czjhoppe commented 1 month ago

@mpusz Happy to contribute to this project :)

Of course, I can create a PR with those changes. However, I am not quite sure if it makes sense to include the changes in the test folder. Most of the changes involve a alot of brackets and they would look really ugly when replaced with a macro. My suggestion would be to exclude those changes in the PR. The PR should include examples or at least a few of them to document the usage with MSVC?

Those issues can be considered as 'bugs' in mp-units. I will open PRs for those.

mpusz commented 1 month ago

Right, we do not have to make all the repo look really bad šŸ˜‰ Maybe we should just scope on the library part and leave tests and examples without those workarounds. Hopefully, MSVC will improve soon and then we will be able to compile the entire repo with it as well.

mpusz commented 3 weeks ago

The main library compiles with MSVC now. Examples and unit tests will need to wait for MSVC to fix their bugs. I listed them on the compiler conformance page in our docs.

mpusz commented 2 weeks ago

@czjhoppe, did you rebase your MSVC branches to the mp-units master? I tried to compile a simple application on the latest MSVC with the current master, but there were still too many issues for it to build completely. Maybe we need to provide more workarounds in order for mp-units to compile successfully on MSVC?

BTW, I hoped to work with MSVC devs at CppCon but they did not arrive here :-(

czjhoppe commented 2 weeks ago

Hi @mpusz, just to clarify, I made changes only to the library part as we had discussed earlier. Perhaps there was a misunderstanding? You can review all the changes that would have been required otherwise by checking out the following branch on my fork: https://github.com/czjhoppe/mp-units/tree/master-msvc-194. This branch successfully compiles all examples and unit tests.

mpusz commented 2 weeks ago

Sure, this is what we have agreed. However, I tried to write a simplest application, compile and link it but it didn't work. I had many new errors. For example about using enum and others.

I will review your branch today. I just wanted to check if it is already rebased on the master.

czjhoppe commented 2 weeks ago

Maybe reviewing my branch will help. Otherwise feel free to reach out to me once more.

mpusz commented 2 weeks ago

OK, I found the issue. With modules disabled, my compilation works. Enabling modules surfaces another massive set of issues :-(

fabiorossetto commented 2 weeks ago

Hello everyone. Thank you for the amazing work. Do I understand correctly that:

mpusz commented 2 weeks ago

I would say there is no risk. mp-units is always tested on all other compilers. @czjhoppe also proved that when the workarounds are applied to tests, they will also compile and pass. If you want to have tests on your side, you can take the tests with workarounds applied from @czjhoppe repository.

Hopefully, the MSVC team will fix those 3 bugs soon, and then our entire master branch will compile as well.

mpusz commented 2 weeks ago

I decided to keep this issue open until all the MSVC bugs, including the modules ones, are fixed.

fabiorossetto commented 1 week ago

That's great! Will you proceed with a new release anytime soon, now that MSVC is (somewhat) supported?

mpusz commented 1 week ago

Yes, I plan to release mp-units 2.3 soon.