`__cpp_modules` now defined in latest MSVC (version > 16.9)

[cdacamar]

Hi Niall,

In our latest update to the compiler we now define __cpp_modules as a compiler pre-defined macro value. This causes build issues with Outcome as we see logs like this:

108>F:\gitP\ned14\outcome\test\single-header-test.cpp(24,1): warning C4117: macro name '__cpp_modules' is reserved, '#undef' ignored [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]
   111>ClCompile:
         success-failure.cpp
   108>F:\gitP\ned14\outcome\test\../single-header/outcome.hpp(25,20): error C2230: could not find module 'outcome_v2_0' [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]
   108>F:\gitP\ned14\outcome\test\single-header-test.cpp(29,39): error C2871: 'OUTCOME_V2_NAMESPACE': a namespace with this name does not exist [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]
   108>F:\gitP\ned14\outcome\test\single-header-test.cpp(30,3): error C7568: argument list missing after assumed function template 'result' [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]
   108>F:\gitP\ned14\outcome\test\single-header-test.cpp(31,3): error C7568: argument list missing after assumed function template 'outcome' [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]
   108>F:\gitP\ned14\outcome\test\single-header-test.cpp(31,19): error C7568: argument list missing after assumed function template 'in_place_type' [F:\gitP\ned14\outcome\build_amd64\outcome_hl--single-header-test.vcxproj]

I see that in our CMakeLists.txt there were some lines to try and enable them but because none of the compilers define this macro the actual project can build cleanly. Is there any chance of getting modules working with Outcome via CMake?

In the meantime is there a patch (likely a configuration macro) to disable the unconditional import of the Outcome module? Along with undefinition of __cpp_modules in the unit test.

[ned14]

I patched in that support for Modules eons ago, because me and Gaby had been having a few drinks and he wanted to know how poorly the MSVC Modules implementation would cope with Outcome. Back then it ICEd the compiler. I tried it again about last year, still ICEd the compiler. I think it sounds right that I should give it another attempt this year.

I'll try to get to it soon. I'll rework the support so we no longer fiddle with __cpp_modules. Thanks for reporting this.

[cdacamar]

Feel free to reach out to me if you run into issues. I'm more than happy to iterate with you on the compiler side.

[ned14]

I've fixed your direct issue. Enabling Modules no longer ICEs the compiler, now I get:

6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(23,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(24,4): error C2913: explicit specialization; 'std::_Abs' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(27,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(29,62): error C2913: explicit specialization; 'std::_Safe_mult' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(31,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(34,2): error C2913: explicit specialization; 'std::_Safe_mult' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(37,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(38,4): error C2913: explicit specialization; 'std::_Sign_of' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(41,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(42,62): error C2913: explicit specialization; 'std::_Safe_addX' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(44,20): error C2061: syntax error: identifier 'intmax_t'
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(47,2): error C2913: explicit specialization; 'std::_Safe_addX' is not a specialization of a class template
6>C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\ratio(49,20): error C2061: syntax error: identifier 'intmax_t'
...

I'm probably using it wrong.

[cdacamar]

I'm probably using it wrong.

Do you happen to have a branch and build instructions I can try?

[ned14]

cmake configure with -DOUTCOME_ENABLE_CXX_MODULES=On should do it. Note the hacky implementation warns with Debug config.

I suspect to fix this, I'll need a macro for every #include of a standard library header to make it into an import. I had kinda been hoping that if Modules are enabled, #include <ratio> would equal import std.ratio.

[cdacamar]

OK, I was able to build the code using the latest version of Visual Studio (16.9) with the following changes:

1. include\outcome.ixx

   module;
   +#include <__msvc_all_public_headers.hpp>
   // Tell the headers we are generating the interface for the library
   #define GENERATING_OUTCOME_MODULE_INTERFACE

   The reason for this is that the various headers throughout the Outcome project include STL headers and these headers need to be included within the context of the global module fragment in order for the project to link properly. If you do not do this then any header within the module purview (declarations after export module outcome_v2_0;) will have module linkage--and that's not right for STL objects. Ideally you wouldn't use the Microsoft STL-specific header __msvc_all_public_headers.hpp but rather list out the STL headers you need explicitly--this header is a hammer.

2. include\outcome\detail\value_storage.hpp

   
   #include <cassert>

-OUTCOME_V2_NAMESPACE_BEGIN +OUTCOME_V2_NAMESPACE_EXPORT_BEGIN

namespace detail

This one is actually a compiler bug.  Apparently the compiler is bad about binding names in non-static data member initializers--so this fix is a workaround.

3. `test\single-header-test.cpp`
```diff
 */

+#include <utility>
+
 #include "../single-header/outcome.hpp"

 int main()
 {
-  using namespace OUTCOME_V2_NAMESPACE;
+  //using namespace OUTCOME_V2_NAMESPACE;
+  using namespace outcome_v2_10f1a124;
   result<int> f(5);
-  outcome<void> m(in_place_type<void>);
+  outcome<void> m(std::in_place_type<void>);
   (void) f;
   (void) m;
   return 0;

The reason you need <utility> is because in_place_type is not exported within the outcome_v2 namespace (that could be fixed by also exporting it). The reason you cannot use the macro OUTCOME_V2_NAMESPACE is because named modules do not export macros :).

The way I tested all the changes above was to navigate to the root of the project, configure the project as per your instructions and create two files: interface.rsp:

/std:c++latest /Zi /EHsc /MDd /fp:precise /c include/outcome.ixx -Ioutcome/../quickcpplib/include -Ibuild/quickcpplib/include

build.rsp:

/Iinclude /Ibuild\quickcpplib\include /Zi /nologo /W3 /WX- /diagnostics:column /Od /Ob0 /D WIN32 /D _WINDOWS /D __cpp_modules=202001L /D QUICKCPPLIB_ENABLE_VALGRIND=1 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /std:c++latest /experimental:module /Gd /errorReport:prompt test\single-header-test.cpp

Open a VS command prompt and use the following:

$ cl @interface.rsp
$ cl @build.rsp outcome.obj

You can also get timings by adding the /Bt+ flag. I obtained the following timings (without modules/with) when compiling test\single-header-test.cpp after the interface was already built:

without modules	with modules
0.73316s	0.19093s
0.88877s	0.19652s
0.73316s	0.19161s
averages
0.78503s	0.19302s

Pretty decent speedup :). Imagine if we could also get rid of the #include <utility> once in_place_type is properly exported.

A few more observations:

1. I noticed that the building of the interface did not properly pass /MDd (it passes /MD) which is why you saw the incompatible environment warning appear.
2. The interface building needs to have different output directories depending on configuration (right now I believe they output to the same directory). To do this you can use the /ifcOutput flag.
3. You no longer need /experimental:module if you're not using std.core.
4. re: your idea about translating #include header-name -> import std.name; while that transformation, specifically, does not happen, the compiler can translate #include header-name -> import header-name; if the /translateInclude option is specified and the header unit is a /headerUnit reference on the command line. We'll talk more about this in some upcoming blogs.

Please let me know if you have any other questions/concerns. Please keep trying out modules and report bugs--I'm off to fix the bug I found!

[ned14]

Firstly, this is enormously helpful, thank you. You're right that I need a less MSVC-specific solution so clang and GCC can one day be supported, but you've certainly advanced me a lot above. Some notes:

The reason for this is that the various headers throughout the Outcome project include STL headers and these headers need to be included within the context of the global module fragment in order for the project to link properly.

I'm thinking if I have cmake match every #include <(.+)> in the single header edition, and prepend them to outcome.ixx, it'll do for now.

I obtained the following timings (without modules/with) when compiling test\single-header-test.cpp after the interface was already built:

I think that column also needs a "with precompiled headers", and then "with modules" won't look so good. Also, from my best understanding, Modules may improve build times, but resolving linkage into a linked binary can't be fast for large links. By which I mean I can't see it can be fast resolving ODR etc etc, so sure, you can have faster builds, but linking large complex binaries already takes a long time, and it can only be slower, as far as I know, with Modules.

Also, single-header/outcome-basic.hpp is the "fast build" edition, you should find it includes so little that it benefits little from any acceleration technologies. I use that edition in all my own code.

Please let me know if you have any other questions/concerns. Please keep trying out modules and report bugs--I'm off to fix the bug I found!

Is there any way I can dump everything a .ifc exports? It would be very useful to check for leakages from the interface.

[cdacamar]

I think that column also needs a "with precompiled headers", and then "with modules" won't look so good.

If the goal is to have the fastest time possible then modules will not beat PCH, but PCH is far less flexible than modules. With a PCH the code author needs to include all of their external libraries in a single header for the majority of a project so they have very poor isolation of 3rd party components. Modules provide a solution that is both fast and sanitary. Another plus for modules is that the generated IFC (the MSVC modules BMI) is that they are relocable, unlike PCH which is machine dependent.

Also, from my best understanding, Modules may improve build times, but resolving linkage into a linked binary can't be fast for large links. By which I mean I can't see it can be fast resolving ODR etc etc, so sure, you can have faster builds, but linking large complex binaries already takes a long time, and it can only be slower, as far as I know, with Modules.

Not necessarily true. The strong ownership model that MSVC uses allows the linker to easily resolve symbols materialized through a module and discard unnecessary, unreferenced, symbols. In many scenarios we have observed the input to the linker is reduced due to the deduplication of COMDAT symbols.

Is there any way I can dump everything a .ifc exports? It would be very useful to check for leakages from the interface.

Not just yet... We are working on tooling like this for a future release. Stay tuned.

[ned14]

I have more bugs for you to fix in MSVC :)

As of the latest commits to develop, most of the test suite compiles with Modules. However there are a few ICEs in there, a few places where it gets confused about ambiguous template resolution which I can probably work around using more preprocessor work, but also some link errors which I think are not my fault. It also seems to not export, and therefore not find, pure constexpr functions, which is weird, though maybe that's the detail namespace.

I'll try to find some more time to work on this next week, specifically to stop try.hpp including success_failure.hpp if a C++ Module is being used (this would separate the macros being defined from C++ being defined).

Thanks for your help in all his.

[cdacamar]

It is good to hear that you've made more progress!

I will sync to the latest repo myself and try some of your suggestions. I look forward to hearing back on progress next week.

[ned14]

Closing as immediate problem is fixed and next Boost release process has begun. Thanks for reporting this issue!