[clang][deps] Fix handling of -MT in module command-line

[benlangmuir]

Summary

Follow-up to 6626f6fec3d3, this fixes the handling of -MT

• If no targets are provided, we need to invent one since cc1 expects the driver to have handled it. The default is to use -o, quoting as necessary for a make target.
• Fix the splitting for empty string, which was incorrectly treated as {""} instead of {}.
• Add a way to test this behaviour in clang-scan-deps.

[jansvoboda11]

LGTM!

clang/include/clang/Basic/MakeSupport.h

+//===- MakeSupport.h - Make Utilities ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_MAKESUPPORT_H
+#define LLVM_CLANG_BASIC_MAKESUPPORT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+
+/// Quote target names for inclusion in GNU Make dependency files.
+/// Only the characters '$', '#', ' ', '\t' are quoted.
+void quoteMakeTarget(StringRef Target, SmallVectorImpl<char> &Res);
+
+} // namespace clang
+
+#endif // LLVM_CLANG_BASIC_MAKESUPPORT_H

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

 
 /// An output from a module compilation, such as the path of the module file.
 enum class ModuleOutputKind {
   /// The module file (.pcm). Required.
   ModuleFile,
   /// The path of the dependency file (.d), if any.
   DependencyFile,
   /// The null-separated list of names to use as the targets in the dependency
-  /// file, if any.
+  /// file, if any. Defaults to the value of \c ModuleFile, as in the driver.
   DependencyTargets,
   /// The path of the serialized diagnostic file (.dia), if any.
   DiagnosticSerializationFile,
 };
 
 struct ModuleDeps {
   /// The identifier of the module.
   ModuleID ID;

clang/lib/Basic/CMakeLists.txt

add_clang_library(clangBasic
   DiagnosticOptions.cpp
   ExpressionTraits.cpp
   FileEntry.cpp
   FileManager.cpp
   FileSystemStatCache.cpp
   IdentifierTable.cpp
   LangOptions.cpp
   LangStandards.cpp
+  MakeSupport.cpp
   Module.cpp
   ObjCRuntime.cpp
   OpenCLOptions.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
   ProfileList.cpp
   NoSanitizeList.cpp
   SanitizerSpecialCaseList.cpp

clang/lib/Basic/MakeSupport.cpp

+//===-- MakeSuport.cpp --------------------------------------------------*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/MakeSupport.h"
+
+void clang::quoteMakeTarget(StringRef Target, SmallVectorImpl<char> &Res) {
+  for (unsigned i = 0, e = Target.size(); i != e; ++i) {
+    switch (Target[i]) {
+    case ' ':
+    case '\t':
+      // Escape the preceding backslashes
+      for (int j = i - 1; j >= 0 && Target[j] == '\\'; --j)
+        Res.push_back('\\');
+
+      // Escape the space/tab
+      Res.push_back('\\');
+      break;
+    case '$':
+      Res.push_back('$');
+      break;
+    case '#':
+      Res.push_back('\\');
+      break;
+    default:
+      break;
+    }
+
+    Res.push_back(Target[i]);
+  }
+}
 No newline at end of file

clang/lib/Driver/ToolChains/Clang.cpp

 #include "CommonArgs.h"
 #include "Hexagon.h"
 #include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CLWarnings.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MakeSupport.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
 #include "clang/Driver/Options.h"
for (; *Arg; ++Arg) {
     case '\\':
       Res.push_back('\\');
       break;
     }
     Res.push_back(*Arg);
   }
 }
 
-// Quote target names for inclusion in GNU Make dependency files.
-// Only the characters '$', '#', ' ', '\t' are quoted.
-static void QuoteTarget(StringRef Target, SmallVectorImpl<char> &Res) {
-  for (unsigned i = 0, e = Target.size(); i != e; ++i) {
-    switch (Target[i]) {
-    case ' ':
-    case '\t':
-      // Escape the preceding backslashes
-      for (int j = i - 1; j >= 0 && Target[j] == '\\'; --j)
-        Res.push_back('\\');
-
-      // Escape the space/tab
-      Res.push_back('\\');
-      break;
-    case '$':
-      Res.push_back('$');
-      break;
-    case '#':
-      Res.push_back('\\');
-      break;
-    default:
-      break;
-    }
-
-    Res.push_back(Target[i]);
-  }
-}
-
 /// Apply \a Work on the current tool chain \a RegularToolChain and any other
 /// offloading tool chain that is associated with the current action \a JA.
 static void
 forAllAssociatedToolChains(Compilation &C, const JobAction &JA,
                            const ToolChain &RegularToolChain,
                            llvm::function_ref<void(const ToolChain &)> Work) {
   // Apply Work on the current/regular tool chain.
   Work(RegularToolChain);
if (ArgM) {
     for (const Arg *A : Args.filtered(options::OPT_MT, options::OPT_MQ)) {
       HasTarget = true;
       A->claim();
       if (A->getOption().matches(options::OPT_MT)) {
         A->render(Args, CmdArgs);
       } else {
         CmdArgs.push_back("-MT");
         SmallString<128> Quoted;
-        QuoteTarget(A->getValue(), Quoted);
+        quoteMakeTarget(A->getValue(), Quoted);
         CmdArgs.push_back(Args.MakeArgString(Quoted));
       }
     }
 
     // Add a default target if one wasn't specified.
     if (!HasTarget) {
       const char *DepTarget;
 
       // If user provided -o, that is the dependency target, except
       // when we are only generating a dependency file.
       Arg *OutputOpt = Args.getLastArg(options::OPT_o);
       if (OutputOpt && Output.getType() != types::TY_Dependencies) {
         DepTarget = OutputOpt->getValue();
       } else {
         // Otherwise derive from the base input.
         //
         // FIXME: This should use the computed output file location.
         SmallString<128> P(Inputs[0].getBaseInput());
         llvm::sys::path::replace_extension(P, "o");
         DepTarget = Args.MakeArgString(llvm::sys::path::filename(P));
       }
 
       CmdArgs.push_back("-MT");
       SmallString<128> Quoted;
-      QuoteTarget(DepTarget, Quoted);
+      quoteMakeTarget(DepTarget, Quoted);
       CmdArgs.push_back(Args.MakeArgString(Quoted));
     }
 
     if (ArgM->getOption().matches(options::OPT_M) ||
         ArgM->getOption().matches(options::OPT_MD))
       CmdArgs.push_back("-sys-header-deps");
     if ((isa<PrecompileJobAction>(JA) &&
          !Args.hasArg(options::OPT_fno_module_file_deps)) ||

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

 //===- ModuleDepCollector.cpp - Callbacks to collect deps -------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 
+#include "clang/Basic/MakeSupport.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "llvm/Support/StringSaver.h"
 
 using namespace clang;
 using namespace tooling;
 using namespace dependencies;
serializeCompilerInvocation(const CompilerInvocation &CI) {
   CI.generateCC1CommandLine(Args, SA);
 
   // Convert arguments to the return type.
   return std::vector<std::string>{Args.begin(), Args.end()};
 }
 
 static std::vector<std::string> splitString(std::string S, char Separator) {
   SmallVector<StringRef> Segments;
-  StringRef(S).split(Segments, Separator);
+  StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false);
   std::vector<std::string> Result;
   Result.reserve(Segments.size());
   for (StringRef Segment : Segments)
     Result.push_back(Segment.str());
   return Result;
 }
 
 std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
     llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>
         LookupModuleOutput) const {
   CompilerInvocation CI(BuildInvocation);
   FrontendOptions &FrontendOpts = CI.getFrontendOpts();
 
   InputKind ModuleMapInputKind(FrontendOpts.DashX.getLanguage(),
                                InputKind::Format::ModuleMap);
   FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind);
   FrontendOpts.OutputFile =
       LookupModuleOutput(ID, ModuleOutputKind::ModuleFile);
   if (HadSerializedDiagnostics)
     CI.getDiagnosticOpts().DiagnosticSerializationFile =
         LookupModuleOutput(ID, ModuleOutputKind::DiagnosticSerializationFile);
   if (HadDependencyFile) {
-    CI.getDependencyOutputOpts().OutputFile =
+    DependencyOutputOptions &DepOpts = CI.getDependencyOutputOpts();
+    DepOpts.OutputFile =
         LookupModuleOutput(ID, ModuleOutputKind::DependencyFile);
-    CI.getDependencyOutputOpts().Targets = splitString(
+    DepOpts.Targets = splitString(
         LookupModuleOutput(ID, ModuleOutputKind::DependencyTargets), '\0');
+    if (!DepOpts.OutputFile.empty() && DepOpts.Targets.empty()) {
+      // Fallback to -o as dependency target, as in the driver.
+      SmallString<128> Target;
+      quoteMakeTarget(FrontendOpts.OutputFile, Target);
+      DepOpts.Targets.push_back(std::string(Target));
+    }
   }
 
   for (ModuleID MID : ClangModuleDeps)
     FrontendOpts.ModuleFiles.push_back(
         LookupModuleOutput(MID, ModuleOutputKind::ModuleFile));
 
   return serializeCompilerInvocation(CI);
 }

clang/test/ClangScanDeps/generate-modules-path-args.c

 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
 // RUN: sed "s|DIR|%/t|g" %t/cdb_without.json.template > %t/cdb_without.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json \
 // RUN:   -format experimental-full -generate-modules-path-args > %t/deps.json
 // RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN:   -format experimental-full -generate-modules-path-args -dependency-target foo > %t/deps_mt1.json
+// RUN: cat %t/deps_mt1.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT1
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN:   -format experimental-full -generate-modules-path-args -dependency-target foo -dependency-target bar > %t/deps_mt2.json
+// RUN: cat %t/deps_mt2.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT2
 // RUN: clang-scan-deps -compilation-database %t/cdb_without.json \
 // RUN:   -format experimental-full -generate-modules-path-args > %t/deps_without.json
 // RUN: cat %t/deps_without.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t -check-prefix=WITHOUT %s
 
 // CHECK:      {
 // CHECK-NEXT:   "modules": [
 // CHECK-NEXT:     {
 // CHECK:            "command-line": [
 // CHECK-NEXT:         "-cc1"
 // CHECK:              "-serialize-diagnostic-file"
 // CHECK-NEXT:         "[[PREFIX]]{{.*}}Mod{{.*}}.diag"
+// CHECK:              "-MT"
+// CHECK-NEXT:         "[[PREFIX]]{{.*}}Mod{{.*}}.pcm"
 // CHECK:              "-dependency-file"
 // CHECK-NEXT:         "[[PREFIX]]{{.*}}Mod{{.*}}.d"
 // CHECK:            ],
 
+// DEPS_MT1:      "-MT"
+// DEPS_MT1-NEXT: "foo"
+
+// DEPS_MT2:      "-MT"
+// DEPS_MT2-NEXT: "foo"
+// DEPS_MT2-NEXT: "-MT"
+// DEPS_MT2-NEXT: "bar"
+
 // WITHOUT:      {
 // WITHOUT-NEXT:   "modules": [
 // WITHOUT-NEXT:     {
 // WITHOUT:            "command-line": [
 // WITHOUT-NEXT:         "-cc1"
 // WITHOUT-NOT:          "-serialize-diagnostic-file"
 // WITHOUT-NOT:          "-dependency-file"
+// WITHOUT-NOT:          "-MT"
 // WITHOUT:            ],
 
 //--- cdb.json.template
 [{
   "directory": "DIR",
   "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -serialize-diagnostics DIR/tu.diag -MD -MT tu -MF DIR/tu.d",
   "file": "DIR/tu.c"
 }]

clang/tools/clang-scan-deps/ClangScanDeps.cpp

llvm::cl::opt<bool> ReuseFileManager(
     llvm::cl::desc("Reuse the file manager and its cache between invocations."),
     llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt<std::string> ModuleName(
     "module-name", llvm::cl::Optional,
     llvm::cl::desc("the module of which the dependencies are to be computed"),
     llvm::cl::cat(DependencyScannerCategory));
 
+llvm::cl::list<std::string> ModuleDepTargets(
+    "dependency-target",
+    llvm::cl::desc("With '-generate-modules-path-args', the names of "
+                   "dependency targets for the dependency file"),
+    llvm::cl::cat(DependencyScannerCategory));
+
 enum ResourceDirRecipeKind {
   RDRK_ModifyCompilerPath,
   RDRK_InvokeCompiler,
 };
 
 static llvm::cl::opt<ResourceDirRecipeKind> ResourceDirRecipe(
     "resource-dir-recipe",
     llvm::cl::desc("How to produce missing '-resource-dir' argument"),
std::string lookupModuleOutput(const ModuleID &MID, ModuleOutputKind MOK) {
     if (PCMPath.second)
       PCMPath.first->second = constructPCMPath(MID);
     switch (MOK) {
     case ModuleOutputKind::ModuleFile:
       return PCMPath.first->second;
     case ModuleOutputKind::DependencyFile:
       return PCMPath.first->second + ".d";
     case ModuleOutputKind::DependencyTargets:
-      return ""; // Will get the default target name.
+      // Null-separate the list of targets.
+      return join(ModuleDepTargets, StringRef("\0", 1));
     case ModuleOutputKind::DiagnosticSerializationFile:
       return PCMPath.first->second + ".diag";
     }
   }
 
   /// Construct a path for the explicitly built PCM.
   std::string constructPCMPath(ModuleID MID) const {
     auto MDIt = Modules.find(IndexedModuleID{MID, 0});