Merge topic 'p1689r4-updates'

ccc0466fc5 cmScanDepFormat: P1689R4: Implement source-path related fields
3e5b609547 cmScanDepFormat: P1689R4: Make work-directory optional
aab9a5fc3e cmScanDepFormat: P1689R4: Factor out primary-output field
b0a8fa959c cmScanDepFormat: P1689R4: Diagnose invalid provides/requires json type
82c867ad85 cmScanDepFormat: P1689R4: Flatten future-compile members into parent

Acked-by: Kitware Robot <[email protected]>
Merge-request: !6202

Help/dev/experimental.rst

@@ -43,6 +43,9 @@ by the `P1689r3`_ paper, with the following updates:
 
 * Omit the ``outputs``, ``inputs``, and ``depends`` fields from
   each entry in the ``rules`` array.  They are unused.
+* Flatten ``future-compile`` members directly into each rule.
+* Factor a ``primary-output`` field out of the now-flattened ``outputs``.
+* The ``work-directory`` field is optional.
 
 Compiler writers may try out their scanning functionality using
 the `cxx-modules-sandbox`_ test project, modified to set variables

Source/cmScanDepFormat.cxx

@@ -3,8 +3,14 @@
 
 #include "cmScanDepFormat.h"
 
+#include <cassert>
 #include <cctype>
 #include <cstdio>
+#include <utility>
+
+#include <cm/optional>
+#include <cm/string_view>
+#include <cmext/string_view>
 
 #include <cm3p/json/reader.h>
 #include <cm3p/json/value.h>
@@ -69,8 +75,9 @@ static Json::Value EncodeFilename(std::string const& path)
       return false;                                                           \
     }                                                                         \
                                                                               \
-    if (!work_directory.empty() && !cmSystemTools::FileIsFullPath(res)) {     \
-      res = cmStrCat(work_directory, '/', res);                               \
+    if (work_directory && !work_directory->empty() &&                         \
+        !cmSystemTools::FileIsFullPath(res)) {                                \
+      res = cmStrCat(*work_directory, '/', res);                              \
     }                                                                         \
   } while (0)
 
@@ -106,10 +113,12 @@ bool cmScanDepFormat_P1689_Parse(std::string const& arg_pp,
     }
 
     for (auto const& rule : rules) {
-      std::string work_directory;
+      cm::optional<std::string> work_directory;
       Json::Value const& workdir = rule["work-directory"];
       if (workdir.isString()) {
-        PARSE_BLOB(workdir, work_directory);
+        std::string wd;
+        PARSE_BLOB(workdir, wd);
+        work_directory = std::move(wd);
       } else if (!workdir.isNull()) {
         cmSystemTools::Error(cmStrCat("-E cmake_ninja_dyndep failed to parse ",
                                       arg_pp,
@@ -117,66 +126,149 @@ bool cmScanDepFormat_P1689_Parse(std::string const& arg_pp,
         return false;
       }
 
-      if (rule.isMember("future-compile")) {
-        Json::Value const& future_compile = rule["future-compile"];
+      if (rule.isMember("primary-output")) {
+        Json::Value const& primary_output = rule["primary-output"];
+        PARSE_FILENAME(primary_output, info->PrimaryOutput);
+      }
+
+      if (rule.isMember("outputs")) {
+        Json::Value const& outputs = rule["outputs"];
+        if (outputs.isArray()) {
+          for (auto const& output : outputs) {
+            std::string extra_output;
+            PARSE_FILENAME(output, extra_output);
+
+            info->ExtraOutputs.emplace_back(extra_output);
+          }
+        }
+      }
+
+      if (rule.isMember("provides")) {
+        Json::Value const& provides = rule["provides"];
+        if (!provides.isArray()) {
+          cmSystemTools::Error(
+            cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                     ": provides is not an array"));
+          return false;
+        }
+
+        for (auto const& provide : provides) {
+          cmSourceReqInfo provide_info;
 
-        if (future_compile.isMember("outputs")) {
-          Json::Value const& outputs = future_compile["outputs"];
-          if (outputs.isArray()) {
-            if (outputs.empty()) {
+          Json::Value const& logical_name = provide["logical-name"];
+          PARSE_BLOB(logical_name, provide_info.LogicalName);
+
+          if (provide.isMember("compiled-module-path")) {
+            Json::Value const& compiled_module_path =
+              provide["compiled-module-path"];
+            PARSE_FILENAME(compiled_module_path,
+                           provide_info.CompiledModulePath);
+          } else {
+            provide_info.CompiledModulePath =
+              cmStrCat(provide_info.LogicalName, ".mod");
+          }
+
+          if (provide.isMember("unique-on-source-path")) {
+            Json::Value const& unique_on_source_path =
+              provide["unique-on-source-path"];
+            if (!unique_on_source_path.isBool()) {
               cmSystemTools::Error(
                 cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
-                         ": expected at least one 1 output"));
+                         ": unique-on-source-path is not a boolean"));
               return false;
             }
+            provide_info.UseSourcePath = unique_on_source_path.asBool();
+          } else {
+            provide_info.UseSourcePath = false;
+          }
 
-            PARSE_FILENAME(outputs[0], info->PrimaryOutput);
+          if (provide.isMember("source-path")) {
+            Json::Value const& source_path = provide["source-path"];
+            PARSE_FILENAME(source_path, provide_info.SourcePath);
+          } else if (provide_info.UseSourcePath) {
+            cmSystemTools::Error(
+              cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                       ": source-path is missing"));
+            return false;
           }
+
+          info->Provides.push_back(provide_info);
         }
+      }
 
-        if (future_compile.isMember("provides")) {
-          Json::Value const& provides = future_compile["provides"];
-          if (provides.isArray()) {
-            for (auto const& provide : provides) {
-              cmSourceReqInfo provide_info;
-
-              Json::Value const& logical_name = provide["logical-name"];
-              PARSE_BLOB(logical_name, provide_info.LogicalName);
-
-              if (provide.isMember("compiled-module-path")) {
-                Json::Value const& compiled_module_path =
-                  provide["compiled-module-path"];
-                PARSE_FILENAME(compiled_module_path,
-                               provide_info.CompiledModulePath);
-              } else {
-                provide_info.CompiledModulePath =
-                  cmStrCat(provide_info.LogicalName, ".mod");
-              }
-
-              info->Provides.push_back(provide_info);
-            }
-          }
+      if (rule.isMember("requires")) {
+        Json::Value const& reqs = rule["requires"];
+        if (!reqs.isArray()) {
+          cmSystemTools::Error(
+            cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                     ": requires is not an array"));
+          return false;
         }
 
-        if (future_compile.isMember("requires")) {
-          Json::Value const& reqs = future_compile["requires"];
-          if (reqs.isArray()) {
-            for (auto const& require : reqs) {
-              cmSourceReqInfo require_info;
+        for (auto const& require : reqs) {
+          cmSourceReqInfo require_info;
 
-              Json::Value const& logical_name = require["logical-name"];
-              PARSE_BLOB(logical_name, require_info.LogicalName);
+          Json::Value const& logical_name = require["logical-name"];
+          PARSE_BLOB(logical_name, require_info.LogicalName);
 
-              if (require.isMember("compiled-module-path")) {
-                Json::Value const& compiled_module_path =
-                  require["compiled-module-path"];
-                PARSE_FILENAME(compiled_module_path,
-                               require_info.CompiledModulePath);
-              }
+          if (require.isMember("compiled-module-path")) {
+            Json::Value const& compiled_module_path =
+              require["compiled-module-path"];
+            PARSE_FILENAME(compiled_module_path,
+                           require_info.CompiledModulePath);
+          }
 
-              info->Requires.push_back(require_info);
+          if (require.isMember("unique-on-source-path")) {
+            Json::Value const& unique_on_source_path =
+              require["unique-on-source-path"];
+            if (!unique_on_source_path.isBool()) {
+              cmSystemTools::Error(
+                cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                         ": unique-on-source-path is not a boolean"));
+              return false;
             }
+            require_info.UseSourcePath = unique_on_source_path.asBool();
+          } else {
+            require_info.UseSourcePath = false;
           }
+
+          if (require.isMember("source-path")) {
+            Json::Value const& source_path = require["source-path"];
+            PARSE_FILENAME(source_path, require_info.SourcePath);
+          } else if (require_info.UseSourcePath) {
+            cmSystemTools::Error(
+              cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                       ": source-path is missing"));
+            return false;
+          }
+
+          if (require.isMember("lookup-method")) {
+            Json::Value const& lookup_method = require["lookup-method"];
+            if (!lookup_method.isString()) {
+              cmSystemTools::Error(
+                cmStrCat("-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                         ": lookup-method is not a string"));
+              return false;
+            }
+
+            std::string lookup_method_str = lookup_method.asString();
+            if (lookup_method_str == "by-name"_s) {
+              require_info.Method = LookupMethod::ByName;
+            } else if (lookup_method_str == "include-angle"_s) {
+              require_info.Method = LookupMethod::IncludeAngle;
+            } else if (lookup_method_str == "include-quote"_s) {
+              require_info.Method = LookupMethod::IncludeQuote;
+            } else {
+              cmSystemTools::Error(cmStrCat(
+                "-E cmake_ninja_dyndep failed to parse ", arg_pp,
+                ": lookup-method is not a valid: ", lookup_method_str));
+              return false;
+            }
+          } else if (require_info.UseSourcePath) {
+            require_info.Method = LookupMethod::ByName;
+          }
+
+          info->Requires.push_back(require_info);
         }
       }
     }
@@ -196,12 +288,14 @@ bool cmScanDepFormat_P1689_Write(std::string const& path,
 
   Json::Value rule(Json::objectValue);
 
-  Json::Value& future_compile = rule["future-compile"] = Json::objectValue;
+  rule["primary-output"] = EncodeFilename(info.PrimaryOutput);
 
-  Json::Value& outputs = future_compile["outputs"] = Json::arrayValue;
-  outputs.append(info.PrimaryOutput);
+  Json::Value& rule_outputs = rule["outputs"] = Json::arrayValue;
+  for (auto const& output : info.ExtraOutputs) {
+    rule_outputs.append(EncodeFilename(output));
+  }
 
-  Json::Value& provides = future_compile["provides"] = Json::arrayValue;
+  Json::Value& provides = rule["provides"] = Json::arrayValue;
   for (auto const& provide : info.Provides) {
     Json::Value provide_obj(Json::objectValue);
     auto const encoded = EncodeFilename(provide.LogicalName);
@@ -213,12 +307,17 @@ bool cmScanDepFormat_P1689_Write(std::string const& path,
         EncodeFilename(provide.CompiledModulePath);
     }
 
-    // TODO: Source file tracking. See below.
+    if (provide.UseSourcePath) {
+      provide_obj["unique-on-source-path"] = true;
+      provide_obj["source-path"] = EncodeFilename(provide.SourcePath);
+    } else if (!provide.SourcePath.empty()) {
+      provide_obj["source-path"] = EncodeFilename(provide.SourcePath);
+    }
 
     provides.append(provide_obj);
   }
 
-  Json::Value& reqs = future_compile["requires"] = Json::arrayValue;
+  Json::Value& reqs = rule["requires"] = Json::arrayValue;
   for (auto const& require : info.Requires) {
     Json::Value require_obj(Json::objectValue);
     auto const encoded = EncodeFilename(require.LogicalName);
@@ -230,8 +329,27 @@ bool cmScanDepFormat_P1689_Write(std::string const& path,
         EncodeFilename(require.CompiledModulePath);
     }
 
-    // TODO: Source filename inclusion. Requires collating with the provides
-    // filenames (as a sanity check if available on both sides).
+    if (require.UseSourcePath) {
+      require_obj["unique-on-source-path"] = true;
+      require_obj["source-path"] = EncodeFilename(require.SourcePath);
+    } else if (!require.SourcePath.empty()) {
+      require_obj["source-path"] = EncodeFilename(require.SourcePath);
+    }
+
+    const char* lookup_method = nullptr;
+    switch (require.Method) {
+      case LookupMethod::ByName:
+        lookup_method = "by-name";
+        break;
+      case LookupMethod::IncludeAngle:
+        lookup_method = "include-angle";
+        break;
+      case LookupMethod::IncludeQuote:
+        lookup_method = "include-quote";
+        break;
+    }
+    assert(lookup_method);
+    require_obj["lookup-method"] = lookup_method;
 
     reqs.append(require_obj);
   }

Source/cmScanDepFormat.h

@@ -5,15 +5,26 @@
 #include <string>
 #include <vector>
 
+enum class LookupMethod
+{
+  ByName,
+  IncludeAngle,
+  IncludeQuote,
+};
+
 struct cmSourceReqInfo
 {
   std::string LogicalName;
+  std::string SourcePath;
   std::string CompiledModulePath;
+  bool UseSourcePath = false;
+  LookupMethod Method = LookupMethod::ByName;
 };
 
 struct cmScanDepInfo
 {
   std::string PrimaryOutput;
+  std::vector<std::string> ExtraOutputs;
 
   // Set of provided and required modules.
   std::vector<cmSourceReqInfo> Provides;