From f5e6c30650dbb135cb758431944f8d350eeee61e Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 7 Dec 2023 09:24:08 +0200 Subject: C++20 named modules support for MSVC, take 2 --- libbuild2/c/init.cxx | 3 +- libbuild2/cc/common.hxx | 6 +- libbuild2/cc/compile-rule.cxx | 178 ++++++++++++++++++------------------------ libbuild2/cc/parser.cxx | 22 ++++-- libbuild2/cc/parser.hxx | 11 ++- libbuild2/cc/parser.test.cxx | 2 +- libbuild2/cxx/init.cxx | 39 +++++---- 7 files changed, 126 insertions(+), 135 deletions(-) diff --git a/libbuild2/c/init.cxx b/libbuild2/c/init.cxx index cb0469c..d8d7488 100644 --- a/libbuild2/c/init.cxx +++ b/libbuild2/c/init.cxx @@ -390,8 +390,7 @@ namespace build2 "c.link", "c.install", - cm.x_info->id.type, - cm.x_info->id.variant, + cm.x_info->id, cm.x_info->class_, cm.x_info->version.major, cm.x_info->version.minor, diff --git a/libbuild2/cc/common.hxx b/libbuild2/cc/common.hxx index d7caeb9..6347005 100644 --- a/libbuild2/cc/common.hxx +++ b/libbuild2/cc/common.hxx @@ -165,6 +165,7 @@ namespace build2 // Cached values for some commonly-used variables/values. // + const compiler_id& cid; // x.id compiler_type ctype; // x.id.type const string& cvariant; // x.id.variant compiler_class cclass; // x.class @@ -278,8 +279,7 @@ namespace build2 const char* compile, const char* link, const char* install, - compiler_type ct, - const string& cv, + const compiler_id& ci, compiler_class cl, uint64_t mj, uint64_t mi, uint64_t vmj, uint64_t vmi, @@ -305,7 +305,7 @@ namespace build2 x_compile (compile), x_link (link), x_install (install), - ctype (ct), cvariant (cv), cclass (cl), + cid (ci), ctype (ci.type), cvariant (ci.variant), cclass (cl), cmaj (mj), cmin (mi), cvmaj (vmj), cvmin (vmi), cpath (path), cmode (mode), diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 3b37cad..3c081f2 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -354,6 +354,35 @@ namespace build2 case lang::c: o1 = "/TC"; break; case lang::cxx: o1 = "/TP"; break; } + + // Note: /interface and /internalPartition are in addition to /TP. + // + switch (md.type) + { + case unit_type::non_modular: + case unit_type::module_impl: + { + break; + } + case unit_type::module_intf: + case unit_type::module_intf_part: + { + o2 = "/interface"; + break; + } + case unit_type::module_impl_part: + { + o2 = "/internalPartition"; + break; + } + case unit_type::module_header: + { + //@@ MODHDR TODO: /exportHeader + assert (false); + break; + } + } + break; } case compiler_class::gcc: @@ -385,6 +414,7 @@ namespace build2 case lang::cxx: o2 = obj ? "objective-c++" : "c++"; break; } } + break; } case unit_type::module_intf: @@ -424,9 +454,11 @@ namespace build2 default: assert (false); } + break; } } + break; } } @@ -1456,24 +1488,6 @@ namespace build2 extract_modules (a, bs, t, li, tts, src, md, move (tu.module_info), dd, u); - - // Currently in VC module interface units must be compiled from - // the original source (something to do with having to detect and - // store header boundaries in the .ifc files). - // - // @@ MODHDR MSVC: should we do the same for header units? I guess - // we will figure it out when MSVC supports header units. - // - // @@ TMP: probably outdated. Probably the same for partitions. - // - // @@ See also similar check in extract_headers(), existing entry - // case. - // - if (ctype == compiler_type::msvc) - { - if (ut == unit_type::module_intf) - psrc.second = false; - } } } @@ -4125,10 +4139,7 @@ namespace build2 // If modules are enabled, then we keep the preprocessed output // around (see apply() for details). // - // See apply() for details on the extra MSVC check. - // - if (modules && (ctype != compiler_type::msvc || - md.type != unit_type::module_intf)) + if (modules) { result.first = ctx.fcache->create_existing (t.path () + pext); result.second = true; @@ -5361,7 +5372,7 @@ namespace build2 fdstream_mode::binary | fdstream_mode::skip); parser p; - p.parse (is, path_name (*sp), tu); + p.parse (is, path_name (*sp), tu, cid); is.close (); @@ -5401,18 +5412,6 @@ namespace build2 ut = md.type; mi.name = src.path ().string (); } - - // Prior to 15.5 (19.12) VC was not using the 'export module M;' - // syntax so we use the preprequisite type to distinguish - // between interface and implementation units. - // - // @@ TMP: probably outdated. - // - if (ctype == compiler_type::msvc && cmaj == 19 && cmin <= 11) - { - if (ut == unit_type::module_impl && src.is_a (*x_mod)) - ut = unit_type::module_intf; - } } // If we were forced to reprocess, assume the checksum is not @@ -6979,7 +6978,7 @@ namespace build2 // options). // void compile_rule:: - append_module_options (environment& env, + append_module_options (environment&, cstrings& args, small_vector& stor, action a, @@ -6990,8 +6989,6 @@ namespace build2 unit_type ut (md.type); const module_positions& ms (md.modules); - dir_path stdifc; // See the VC case below. - switch (ctype) { case compiler_type::gcc: @@ -7079,6 +7076,9 @@ namespace build2 if (ms.start == 0) return; + // MSVC requires a transitive set of interfaces, including + // implementation partitions. + // auto& pts (t.prerequisite_targets[a]); for (size_t i (ms.start), n (pts.size ()); i != n; ++i) { @@ -7091,34 +7091,14 @@ namespace build2 // of these are bmi's. // const file& f (pt->as ()); + string s (relative (f.path ()).string ()); - // In VC std.* modules can only come from a single directory - // specified with the IFCPATH environment variable or the - // /module:stdIfcDir option. - // - if (std_module (cast (f.state[a].vars[c_module_name]))) - { - dir_path d (f.path ().directory ()); + s.insert (0, 1, '='); + s.insert (0, cast (f.state[a].vars[c_module_name])); - if (stdifc.empty ()) - { - // Go one directory up since /module:stdIfcDir will look in - // either Release or Debug subdirectories. Keeping the result - // absolute feels right. - // - stor.push_back ("/module:stdIfcDir"); - stor.push_back (d.directory ().string ()); - stdifc = move (d); - } - else if (d != stdifc) // Absolute and normalized. - fail << "multiple std.* modules in different directories"; - } - else - { - stor.push_back ("/module:reference"); - stor.push_back (relative (f.path ()).string ()); - } + stor.push_back (move (s)); } + break; } case compiler_type::icc: @@ -7129,25 +7109,11 @@ namespace build2 // into storage? Because of potential reallocations. // for (const string& a: stor) - args.push_back (a.c_str ()); - - if (getenv ("IFCPATH")) - { - // VC's IFCPATH takes precedence over /module:stdIfcDir so unset it if - // we are using our own std modules. Note: IFCPATH saved in guess.cxx. - // - if (!stdifc.empty ()) - env.push_back ("IFCPATH"); - } - else if (stdifc.empty ()) { - // Add the VC's default directory (should be only one). - // - if (sys_mod_dirs != nullptr && !sys_mod_dirs->empty ()) - { - args.push_back ("/module:stdIfcDir"); - args.push_back (sys_mod_dirs->front ().string ().c_str ()); - } + if (ctype == compiler_type::msvc) + args.push_back ("/reference"); + + args.push_back (a.c_str ()); } } @@ -7386,9 +7352,8 @@ namespace build2 // Note also that what we are doing here appears to be incompatible // with PCH (/Y* options) and /Gm (minimal rebuild). // - // @@ MOD: TODO deal with absent relo (sidebuild). - // - if (find_options ({"/Zi", "/ZI", "-Zi", "-ZI"}, args)) + if (!relo.empty () && + find_options ({"/Zi", "/ZI", "-Zi", "-ZI"}, args)) { if (fc) args.push_back ("/Fd:"); @@ -7401,27 +7366,38 @@ namespace build2 args.push_back (out1.c_str ()); } - if (fc) - { - args.push_back ("/Fo:"); - args.push_back (relo.string ().c_str ()); - } - else + if (ut == unit_type::module_intf || + ut == unit_type::module_intf_part || + ut == unit_type::module_impl_part || + ut == unit_type::module_header) { - out = "/Fo" + relo.string (); - args.push_back (out.c_str ()); - } + assert (ut != unit_type::module_header); // @@ MODHDR - // @@ MODHDR MSVC - // @@ MODPART MSVC - // - if (ut == unit_type::module_intf) - { relm = relative (tp); - args.push_back ("/module:interface"); - args.push_back ("/module:output"); + args.push_back ("/ifcOutput"); args.push_back (relm.string ().c_str ()); + + if (relo.empty ()) + args.push_back ("/ifcOnly"); + else + { + args.push_back ("/Fo:"); + args.push_back (relo.string ().c_str ()); + } + } + else + { + if (fc) + { + args.push_back ("/Fo:"); + args.push_back (relo.string ().c_str ()); + } + else + { + out = "/Fo" + relo.string (); + args.push_back (out.c_str ()); + } } // Note: no way to indicate that the source if already preprocessed. @@ -7642,7 +7618,7 @@ namespace build2 } case compiler_type::clang: { - assert (ut != unit_type::module_header); + assert (ut != unit_type::module_header); // @@ MODHDR relm = relative (tp); diff --git a/libbuild2/cc/parser.cxx b/libbuild2/cc/parser.cxx index 3e84edb..f62847e 100644 --- a/libbuild2/cc/parser.cxx +++ b/libbuild2/cc/parser.cxx @@ -15,8 +15,10 @@ namespace build2 using type = token_type; void parser:: - parse (ifdstream& is, const path_name& in, unit& u) + parse (ifdstream& is, const path_name& in, unit& u, const compiler_id& cid) { + cid_ = &cid; + lexer l (is, in, true /* preprocessed */); l_ = &l; u_ = &u; @@ -82,6 +84,12 @@ namespace build2 // to call it __import) or it can have a special attribute (GCC // currently marks it with [[__translated]]). // + // Similarly, MSVC drops the `module;` marker and replaces all + // other `module` keywords with `__preprocessed_module`. + // + // Clang doesn't appear to rewrite anything, at least as of + // version 18. + // if (bb == 0 && t.first) { const string& id (t.value); // Note: tracks t. @@ -102,7 +110,9 @@ namespace build2 // Fall through. } - if (id == "module") + if (id == "module" || + (cid_->type == compiler_type::msvc && + id == "__preprocessed_module")) { location_value l (get_location (t)); l_->next (t); @@ -113,7 +123,9 @@ namespace build2 else n = false; } - else if (id == "import" /*|| id == "__import"*/) + else if (id == "import" /* || + (cid_->type == compiler_type::gcc && + id == "__import")*/) { l_->next (t); @@ -181,7 +193,7 @@ namespace build2 // pair np (parse_module_name (t, true /* partition */)); - // Should be {}-balanced. + // Skip attributes (should be {}-balanced). // for (; t.type != type::eos && t.type != type::semi && !t.first; @@ -262,7 +274,7 @@ namespace build2 return; } - // Should be {}-balanced. + // Skip attributes (should be {}-balanced). // for (; t.type != type::eos && t.type != type::semi && !t.first; diff --git a/libbuild2/cc/parser.hxx b/libbuild2/cc/parser.hxx index 1fbf1a3..0c2eb2d 100644 --- a/libbuild2/cc/parser.hxx +++ b/libbuild2/cc/parser.hxx @@ -10,6 +10,7 @@ #include #include +#include // compiler_id namespace build2 { @@ -23,16 +24,19 @@ namespace build2 class parser { public: + // The compiler_id argument should identify the compiler that has done + // the preprocessing. + // unit - parse (ifdstream& is, const path_name& n) + parse (ifdstream& is, const path_name& n, const compiler_id& cid) { unit r; - parse (is, n, r); + parse (is, n, r, cid); return r; } void - parse (ifdstream&, const path_name&, unit&); + parse (ifdstream&, const path_name&, unit&, const compiler_id&); private: void @@ -54,6 +58,7 @@ namespace build2 string checksum; // Translation unit checksum. private: + const compiler_id* cid_; lexer* l_; unit* u_; diff --git a/libbuild2/cc/parser.test.cxx b/libbuild2/cc/parser.test.cxx index 1d5930a..2270d32 100644 --- a/libbuild2/cc/parser.test.cxx +++ b/libbuild2/cc/parser.test.cxx @@ -44,7 +44,7 @@ namespace build2 } parser p; - unit u (p.parse (is, in)); + unit u (p.parse (is, in, compiler_id (compiler_type::gcc, ""))); switch (u.type) { diff --git a/libbuild2/cxx/init.cxx b/libbuild2/cxx/init.cxx index 626c795..8a2ad88 100644 --- a/libbuild2/cxx/init.cxx +++ b/libbuild2/cxx/init.cxx @@ -388,28 +388,24 @@ namespace build2 { case compiler_type::msvc: { - // While modules are supported in VC 15.0 (19.10), there is a - // bug in the separate interface/implementation unit support - // which makes them pretty much unusable. This has been fixed in - // 15.3 (19.11). And 15.5 (19.12) supports the `export module - // M;` syntax. And 16.4 (19.24) supports the global module - // fragment. And in 16.8 all the modules-related options have - // been changed. Seeing that the whole thing is unusable anyway, - // we disable it for 16.8 or later for now. + // Modules are enabled by default in /std:c++20 and + // /std:c++latest with both defining __cpp_modules to 201907 + // (final C++20 module), at least as of 17.6 (LTS). // - if ((mj > 19 || (mj == 19 && mi >= (modules.value ? 10 : 12))) && - (mj < 19 || (mj == 19 && mi < 28) || modules.value)) + // @@ Should we enable modules by default? + // + if (modules.value) { - prepend ( - mj > 19 || mi >= 24 ? - "/D__cpp_modules=201810" : // p1103 (merged modules) - mj == 19 || mi >= 12 ? - "/D__cpp_modules=201704" : // p0629r0 (export module M;) - "/D__cpp_modules=201703"); // n4647 ( module M;) - - prepend ("/experimental:module"); + if (mj < 19 || (mj == 19 && mi < 36)) + { + fail << "support for C++ modules requires MSVC 17.6 or later" << + info << "C++ compiler is " << ci.signature << + info << "required by " << project (rs) << '@' << rs; + } + modules = true; } + break; } case compiler_type::gcc: @@ -419,6 +415,10 @@ namespace build2 // generated headers via the mapper, we require the user to // explicitly request modules. // + // @@ Actually, now that we pre-generate headers by default, + // this is probably no longer the reason. But GCC modules being + // unusable due to bugs is a reason enough. + // if (mj >= 11 && modules.value) { // Defines __cpp_modules: @@ -870,8 +870,7 @@ namespace build2 "cxx.link", "cxx.install", - cm.x_info->id.type, - cm.x_info->id.variant, + cm.x_info->id, cm.x_info->class_, cm.x_info->version.major, cm.x_info->version.minor, -- cgit v1.1