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

[SUGGESTION] Start using import std; in cpp2util.h with MSVC and re-enable -import-std in CI builds #950

Closed
bluetarpmedia opened this issue Jan 19, 2024 · 5 comments

Comments

@bluetarpmedia
Copy link
Contributor

Now that MSVC (with /std:c++latest) supports import std; it's possible to remove the Microsoft-only experimental import std.core; etc lines from cpp2util.h and replace them with import std; or import std.compat;.

As described in #943, the import std.core; code was not working in the GitHub-hosted CI runner, so the MSVC regression tests have been updated to run cppfront with -include-std to #include headers instead of importing any modules.

I experimented with import std; and GitHub-hosted runners and documented two methods to get them building correctly:

  • running msbuild for a Visual Studio solution
  • using cl.exe in a shell, which is applicable for cppfront's GitHub CI workflow

https://github.com/bluetarpmedia/msvc-import-std-github

So this suggestion involves 2 changes:

  1. Update cpp2util.h to use import std; (where available)
  2. Update the GitHub CI workflow to restore the default -import-std behaviour, and modify the MSVC build job to compile import std; modules support first before then compiling the tests (more info in my msvc-import-std-github repo)
@hsutter
Copy link
Owner

hsutter commented Jan 19, 2024

I was thinking the same. Note that this will require:

  1. Build { std, std.compat } . { ifc, obj} with the identical switches we will use to build the test. The files probably should just stay in the MSVC build current directory since they don't need to be shared, and then we don't need to add a /reference switch on the command line. Here's what I used FWIW: cl /std:c++latest /EHsc /nologo /W4 /MD /c "%VCToolsInstallDir%\modules\std.ixx" and cl /std:c++latest /EHsc /nologo /W4 /MD /c "%VCToolsInstallDir%\modules\std.compat.ixx".
    • Note: If the MSVC environment's regression-run cleanup does del *.obj (as I do in mine), it should be modified to not erase the std*.obj files... I changed my cleanup step to del pure2-*.obj mixed-*.obj.
  2. Change /experimental-module to std.obj std.compat.obj to the MSVC command line (including those obj's seems to work fine even for mixed-* files that use #include and don't use them)

I can go ahead and make all these changes in one commit, including checking in my std*.ifc and std*.obj files in the MSVC regression test directory, and then everything should be consistent and working except for the CI flow... could you then make a PR to apply the same to the CI environment?

@hsutter
Copy link
Owner

hsutter commented Jan 19, 2024

Apologies in advance that checking in the std* modules files added 35M. They should not change often though, if that helps.

If that causes issues and I should remove them again, please speak up and let me know!

@JohelEGP
Copy link
Contributor

JohelEGP commented Jan 19, 2024

See the discussion at 3b78cf1#commitcomment-112286068.
The correct thing to do in this case
is to have CI build the module, and
cache it and restore it using https://github.com/actions/cache/.

hsutter added a commit that referenced this issue Jan 20, 2024
@bluetarpmedia
Copy link
Contributor Author

Nice work @hsutter

One thing I noticed is that it’s not necessary to import std; as well as import std.compat; because the latter includes the former.

(Shame about the name because it doesn’t seem to suggest that; something like import std+; makes more sense to me.)

@hsutter
Copy link
Owner

hsutter commented Jan 20, 2024

Thanks! STL (the person) just mentioned that too.

I've removed import std; since it's not needed, and added some build setup notes: 72cfb62

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants