C++20 named modules support for MSVC, take 2

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<string, 2>& 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<file> ());
+            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<string> (f.state[a].vars[c_module_name])))
-            {
-              dir_path d (f.path ().directory ());
+            s.insert (0, 1, '=');
+            s.insert (0, cast<string> (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<string, bool> 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 <libbuild2/diagnostics.hxx>
 
 #include <libbuild2/cc/types.hxx>
+#include <libbuild2/cc/guess.hxx> // 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,