Merge topic 'swift-compile-commands'

44f29a4291 Swift/Ninja: Fix multifile module compile commands

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Merge-request: !9095

5 files changed, 128 insertions, 20 deletions

diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx
index 7ea479e..2ea893d 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -1916,7 +1916,6 @@ void cmNinjaTargetGenerator::WriteSwiftObjectBuildStatement(
   //  indistinguishable from the old behavior.
   //
   //  FIXME(#25490): Add response file support to Swift object build step
-  //  FIXME(#25491): Include all files in module in compile_commands.json
 
   if (sources.empty()) {
     return;
@@ -2027,15 +2026,7 @@ void cmNinjaTargetGenerator::WriteSwiftObjectBuildStatement(
     std::string const sourceFilePath = this->GetCompiledSourceNinjaPath(sf);
     objBuild.ExplicitDeps.push_back(sourceFilePath);
 
-    if (isSingleOutput) {
-      if (firstForConfig) {
-        this->ExportObjectCompileCommand(
-          language, sourceFilePath, objectDir, targetObjectFilename,
-          cmSystemTools::GetFilenamePath(targetObjectFilename), vars["FLAGS"],
-          vars["DEFINES"], vars["INCLUDES"],
-          /*compile pdb*/ "", /*target pdb*/ "", config, WithScanning::No);
-      }
-    } else {
+    if (!isSingleOutput) {
       // Object outputs
       std::string const objectFilepath =
         this->ConvertToNinjaPath(this->GetObjectFilePath(sf, config));
@@ -2045,16 +2036,6 @@ void cmNinjaTargetGenerator::WriteSwiftObjectBuildStatement(
 
       // Add OFM data
       this->EmitSwiftDependencyInfo(sf, config);
-
-      // Emit compile commands
-      if (firstForConfig) {
-        this->ExportObjectCompileCommand(
-          language, sourceFilePath, objectDir, objectFilepath,
-          cmSystemTools::GetFilenamePath(objectFilepath), vars["FLAGS"],
-          vars["DEFINES"], vars["INCLUDES"],
-          /*compile pdb*/ "",
-          /*target pdb*/ "", config, WithScanning::No);
-      }
     }
   }
 
@@ -2062,6 +2043,12 @@ void cmNinjaTargetGenerator::WriteSwiftObjectBuildStatement(
     this->GenerateSwiftOutputFileMap(config, vars["FLAGS"]);
   }
 
+  if (firstForConfig) {
+    this->ExportSwiftObjectCompileCommand(
+      sources, targetObjectFilename, vars["FLAGS"], vars["DEFINES"],
+      vars["INCLUDES"], config, isSingleOutput);
+  }
+
   for (cmTargetDepend const& dep :
        this->GetGlobalGenerator()->GetTargetDirectDepends(&target)) {
     if (!dep->IsLanguageUsed("Swift", config)) {
@@ -2326,6 +2313,77 @@ void cmNinjaTargetGenerator::ExportObjectCompileCommand(
                                                    objectFileName);
 }
 
+void cmNinjaTargetGenerator::ExportSwiftObjectCompileCommand(
+  std::vector<cmSourceFile const*> const& moduleSourceFiles,
+  std::string const& moduleObjectFilename, std::string const& flags,
+  std::string const& defines, std::string const& includes,
+  std::string const& outputConfig, bool singleOutput)
+{
+  if (!this->GeneratorTarget->GetPropertyAsBool("EXPORT_COMPILE_COMMANDS")) {
+    return;
+  }
+
+  auto escapeSourceFileName = [this](std::string srcFilename) -> std::string {
+    if (!cmSystemTools::FileIsFullPath(srcFilename)) {
+      srcFilename =
+        cmSystemTools::CollapseFullPath(srcFilename,
+                                        this->GetGlobalGenerator()
+                                          ->GetCMakeInstance()
+                                          ->GetHomeOutputDirectory());
+    }
+
+    return this->LocalGenerator->ConvertToOutputFormat(
+      srcFilename, cmOutputConverter::SHELL);
+  };
+
+  cmRulePlaceholderExpander::RuleVariables compileObjectVars;
+  compileObjectVars.Language = "Swift";
+  compileObjectVars.Flags = flags.c_str();
+  compileObjectVars.Defines = defines.c_str();
+  compileObjectVars.Includes = includes.c_str();
+
+  // Build up the list of source files in the module
+  std::vector<std::string> filenames;
+  filenames.reserve(moduleSourceFiles.size());
+  for (cmSourceFile const* sf : moduleSourceFiles) {
+    filenames.emplace_back(
+      escapeSourceFileName(this->GetCompiledSourceNinjaPath(sf)));
+  }
+  // Note that `escapedSourceFilenames` must remain alive until the
+  // compileObjectVars is consumed or Source will be a dangling pointer.
+  std::string const escapedSourceFilenames = cmJoin(filenames, " ");
+  compileObjectVars.Source = escapedSourceFilenames.c_str();
+
+  std::string const& compileCommand =
+    this->Makefile->GetRequiredDefinition("CMAKE_Swift_COMPILE_OBJECT");
+  cmList compileCmds(compileCommand);
+
+  auto rulePlaceholderExpander =
+    this->GetLocalGenerator()->CreateRulePlaceholderExpander();
+
+  for (cmSourceFile const* sf : moduleSourceFiles) {
+    std::string const sourceFilename = this->GetCompiledSourceNinjaPath(sf);
+    std::string objectFilename = moduleObjectFilename;
+
+    if (!singleOutput) {
+      // If it's not single-output, each source file gets a separate object
+      objectFilename = this->GetObjectFilePath(sf, outputConfig);
+    }
+    compileObjectVars.Objects = objectFilename.c_str();
+
+    for (std::string& cmd : compileCmds) {
+      rulePlaceholderExpander->ExpandRuleVariables(this->GetLocalGenerator(),
+                                                   cmd, compileObjectVars);
+    }
+
+    std::string commandLine = this->GetLocalGenerator()->BuildCommandLine(
+      compileCmds, outputConfig, outputConfig);
+
+    this->GetGlobalGenerator()->AddCXXCompileCommand(
+      commandLine, sourceFilename, objectFilename);
+  }
+}
+
 void cmNinjaTargetGenerator::AdditionalCleanFiles(const std::string& config)
 {
   if (cmValue prop_value =
diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h
index b55c460..f081117 100644
--- a/Source/cmNinjaTargetGenerator.h
+++ b/Source/cmNinjaTargetGenerator.h
@@ -195,6 +195,12 @@ protected:
     std::string const& targetCompilePdb, std::string const& targetPdb,
     std::string const& outputConfig, WithScanning withScanning);
 
+  void ExportSwiftObjectCompileCommand(
+    std::vector<cmSourceFile const*> const& moduleSourceFiles,
+    std::string const& moduleObjectFilename, std::string const& flags,
+    std::string const& defines, std::string const& includes,
+    std::string const& outputConfig, bool singleOutput);
+
   void AdditionalCleanFiles(const std::string& config);
 
   cmNinjaDeps GetObjects(const std::string& config) const;
diff --git a/Tests/RunCMake/Swift/CompileCommands-check.cmake b/Tests/RunCMake/Swift/CompileCommands-check.cmake
new file mode 100644
index 0000000..6450745
--- /dev/null
+++ b/Tests/RunCMake/Swift/CompileCommands-check.cmake
@@ -0,0 +1,30 @@
+if(NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/compile_commands.json")
+  set(RunCMake_TEST_FAILED "compile_commands.json not generated")
+  return()
+endif()
+
+# The compile command for both files should contain all Swift source files in
+# the module
+set(expected_compile_commands
+[==[^\[
+{
+  "directory": ".*(/Tests/RunCMake/Swift/CompileCommands-build|\\\\Tests\\\\RunCMake\\\\Swift\\\\CompileCommands-build)",
+  "command": ".*swiftc .* (\\")?.*(/Tests/RunCMake/Swift/E.swift|\\\\Tests\\\\RunCMake\\\\Swift\\\\E.swift)(\\")? (\\")?.*(/Tests/RunCMake/Swift/L.swift|\\\\Tests\\\\RunCMake\\\\Swift\\\\L.swift)(\\")?",
+  "file": ".*(/Tests/RunCMake/Swift/E.swift|\\\\Tests\\\\RunCMake\\\\Swift\\\\E.swift)",
+  "output": "CMakeFiles/CompileCommandLib.dir/E.swift.o|CMakeFiles\\\\CompileCommandLib.dir\\\\E.swift.obj"
+},
+{
+  "directory": ".*(/Tests/RunCMake/Swift/CompileCommands-build|\\\\Tests\\\\RunCMake\\\\Swift\\\\CompileCommands-build)",
+  "command": ".*swiftc .* (\\")?.*(/Tests/RunCMake/Swift/E.swift|\\\\Tests\\\\RunCMake\\\\Swift\\\\E.swift)(\\")? (\\")?.*(/Tests/RunCMake/Swift/L.swift|\\\\Tests\\\\RunCMake\\\\Swift\\\\L.swift)(\\")?",
+  "file": ".*/Tests/RunCMake/Swift/L.swift",
+  "output": "CMakeFiles/CompileCommandLib.dir/L.swift.o|CMakeFiles\\\\CompileCommandLib.dir\\\\L.swift.obj"
+}
+]$]==]
+)
+
+file(READ "${RunCMake_TEST_BINARY_DIR}/compile_commands.json" compile_commands)
+if(NOT compile_commands MATCHES "${expected_compile_commands}")
+  string(REPLACE "\n" "\n  " expected_compile_commands_formatted "${expected_compile_commands}")
+  string(REPLACE "\n" "\n  " compile_commands_formatted "${compile_commands}")
+  string(APPEND RunCMake_TEST_FAILED "Expected compile_commands.json to match:\n  ${expected_compile_commands_formatted}\nActual compile_commands.json:\n  ${compile_commands_formatted}\n")
+endif()
diff --git a/Tests/RunCMake/Swift/CompileCommands.cmake b/Tests/RunCMake/Swift/CompileCommands.cmake
new file mode 100644
index 0000000..f859693
--- /dev/null
+++ b/Tests/RunCMake/Swift/CompileCommands.cmake
@@ -0,0 +1,9 @@
+if(POLICY CMP0157)
+  cmake_policy(SET CMP0157 NEW)
+endif()
+set(CMAKE_Swift_COMPILATION_MODE "singlefile")
+
+enable_language(Swift)
+
+add_library(CompileCommandLib STATIC E.swift L.swift)
+set_target_properties(CompileCommandLib PROPERTIES EXPORT_COMPILE_COMMANDS YES)
diff --git a/Tests/RunCMake/Swift/RunCMakeTest.cmake b/Tests/RunCMake/Swift/RunCMakeTest.cmake
index 184b461..500bf76 100644
--- a/Tests/RunCMake/Swift/RunCMakeTest.cmake
+++ b/Tests/RunCMake/Swift/RunCMakeTest.cmake
@@ -61,6 +61,11 @@ elseif(RunCMake_GENERATOR STREQUAL Ninja)
       run_cmake(CMP0157-WARN)
     endblock()
 
+    block()
+      set(CompileCommands_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CompileCommands-build)
+      run_cmake(CompileCommands)
+      run_cmake_command(CompileCommands-check ${CMAKE_COMMAND} --build ${CompileCommands_TEST_BINARY_DIR})
+    endblock()
   endif()
 elseif(RunCMake_GENERATOR STREQUAL "Ninja Multi-Config")
   if(CMake_TEST_Swift)