hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.27k stars 225 forks source link

[BUG] MSVC CI regression-test job doesn't have modules support, meaning `pure2` tests succeed without running #943

Closed bluetarpmedia closed 6 months ago

bluetarpmedia commented 6 months ago

CC @jarzec and @JohelEGP who are aware of this problem. The same problem happens here: https://github.com/modern-cmake/cppfront/issues/68

Describe the bug

The CI regression-tests for MSVC are not able to run the pure2 tests. The tests succeed which means real bugs or regressions can be hidden, unless they're exposed on other compilers.

There seems to be a problem with the GitHub-hosted runner for windows-latest (currently an alias for windows-2022) with Visual Studio Enterprise. The machine image is documented to have the experimental modules component installed for Visual Studio. (See Microsoft.VisualStudio.Component.VC.Modules.x86.x64)

But when cl.exe runs with -std:c++latest -experimental:module it reports fatal error C1011:

cannot locate standard module interface. Did you install the library part of the C++ modules feature in VS setup?

Strangely, this would mean that __cpp_lib_modules is defined to be true so that the import lines are enabled in cpp2util.h.

I updated the GitHub action in my own branch to try and manually install the C++ modules support before proceeding to run the tests, but that had no effect.

"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vs_installer.exe" modify --channelId VisualStudio.17.Release --productId Microsoft.VisualStudio.Product.Enterprise -q --add "Microsoft.VisualStudio.Component.VC.Modules.x86.x64"

So it would appear that the C++ modules component is correctly installed in the GitHub-hosted runner machine image, but for some reason MSVC still fails.

The CI run-tests.sh script detects the C1011 error and reports this to stdout: Skipping further checks due to missing std modules support

Until we can resolve the problem, I think it's better to change the MSVC tests to use cppfront with -include-std so that the tests can actually run rather than silently succeeding.

To Reproduce

Here is a recent MSVC regression-tests job: https://github.com/hsutter/cppfront/actions/runs/7547757353/job/20548383595

Expand the Run regression tests - Windows version section and scroll down to the pure2 tests. You'll see something like:

Testing pure Cpp2 code: pure2-bounds-safety-span.cpp2
        Generating Cpp1 code
        Compiling the generated Cpp1 code
            Skipping further checks due to missing std modules support

But above, you'll notice that the mixed tests are executed correctly.

hsutter commented 6 months ago

Until we can resolve the problem, I think it's better to change the MSVC tests to use cppfront with -include-std so that the tests can actually run rather than silently succeeding.

That sounds right. I'd be happy to merge a PR to do this.

hsutter commented 6 months ago

Has someone filed a bug with GitHub to report this problem with modules?

hsutter commented 6 months ago

On further investigation, I think part of the issue is that MSVC now has support for C++23 import std;, and the old pre-standard import std.core; style (which also requires the -experimental:modules switch) are now getting in the way.

I tried updating cpp2util.h to remove the pre-standard ones and go straight to import std;, and build without the -experimental:modules switch, but encountered failures and couldn't get further so I undid the change.

hsutter commented 6 months ago

Please let us know if the merged PR fixes this issue. If it does, we can close it. Thanks!

bluetarpmedia commented 6 months ago

Please let us know if the merged PR fixes this issue. If it does, we can close it. Thanks!

Yeah, the #944 PR has fixed the problem, meaning the pure2 tests are now executing for MSVC in the CI job.