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

Fail to compile v2.0.0 with MSVC #518

Open
nniclausse opened this issue Nov 15, 2023 · 26 comments
Open

Fail to compile v2.0.0 with MSVC #518

nniclausse opened this issue Nov 15, 2023 · 26 comments
Labels
gated Something else is blocking the resolution of this issue

Comments

@nniclausse
Copy link

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
Copy link
Collaborator

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

Yeah. This was mentioned in the latest talk: #451 (comment).

@mpusz
Copy link
Owner

mpusz commented Nov 15, 2023

@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
Copy link
Author

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

@dpservis
Copy link

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

@mpusz
Copy link
Owner

mpusz commented Mar 31, 2024

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
Copy link

dpservis commented Apr 1, 2024

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
Copy link

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

@mpusz
Copy link
Owner

mpusz commented Jun 21, 2024

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
Copy link
Owner

mpusz commented Jun 21, 2024

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.

@mpusz mpusz added the gated Something else is blocking the resolution of this issue label Jun 22, 2024
@connorjak
Copy link

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

@connorjak
Copy link

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:

template<typename Q2, typename U2>
[[nodiscard]] friend consteval detail::reference_t<Q{} * Q2{}, U{} * U2{}> operator*(reference, reference<Q2, U2>)
{
return {};
}

  • Visual C++ 19.40 (MSVC v143) (Visual Studio 2022 17.10)
  • x64
  • Windows 11

@czjhoppe
Copy link
Contributor

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
Copy link
Owner

mpusz commented Aug 30, 2024

@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
Copy link
Contributor

czjhoppe commented Sep 2, 2024

@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
Copy link
Owner

mpusz commented Sep 2, 2024

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
Copy link
Owner

mpusz commented Sep 10, 2024

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 mpusz closed this as completed Sep 10, 2024
@mpusz mpusz reopened this Sep 16, 2024
@mpusz
Copy link
Owner

mpusz commented Sep 16, 2024

@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
Copy link
Contributor

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
Copy link
Owner

mpusz commented Sep 17, 2024

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
Copy link
Contributor

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

@mpusz
Copy link
Owner

mpusz commented Sep 17, 2024

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

@mpusz mpusz closed this as completed Sep 17, 2024
@fabiorossetto
Copy link

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

  • with modules disabled, latest master of mp-units compiles on MSVC
  • tests are not compiled on MSVC, so its a bit risky to adopt mp-units with MSVC in production at the moment

@mpusz
Copy link
Owner

mpusz commented Sep 18, 2024

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 mpusz reopened this Sep 18, 2024
@mpusz
Copy link
Owner

mpusz commented Sep 18, 2024

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

@fabiorossetto
Copy link

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

@mpusz
Copy link
Owner

mpusz commented Sep 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gated Something else is blocking the resolution of this issue
Projects
None yet
Development

No branches or pull requests

8 participants