[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

[Shivanshu Goyal via Phabricator via llvm-commits]

shivanshu3 created this revision.
shivanshu3 added reviewers: aeubanks, kadircet.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
shivanshu3 requested review of this revision.
Herald added a subscriber: ormris.

MSVC's cl.exe has a few command line arguments which start with -M such as "-MD", "-MDd", "-MT", "-MTd", "-MP".
These arguments are not dependency file generation related, and these arguments were being removed by getClangStripDependencyFileAdjuster() which was wrong.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86999

Files:
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp


Index: clang/unittests/Tooling/ToolingTest.cpp
===================================================================
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -563,6 +563,39 @@
   EXPECT_TRUE(HasFlag("-c"));
 }
 
+// Check getClangStripDependencyFileAdjuster doesn't strip args when using the
+// MSVC cl.exe driver
+TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) {
+  FixedCompilationDatabase Compilations(
+      "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-MTd", "-MP"});
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr<FrontendActionFactory> Action(
+      newFrontendActionFactory<SyntaxOnlyAction>());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+      [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+        FinalArgs = Args;
+        return Args;
+      };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+    return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_TRUE(HasFlag("-MD"));
+  EXPECT_TRUE(HasFlag("-MDd"));
+  EXPECT_TRUE(HasFlag("-MT"));
+  EXPECT_TRUE(HasFlag("-MTd"));
+  EXPECT_TRUE(HasFlag("-MP"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===================================================================
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,6 +21,17 @@
 namespace clang {
 namespace tooling {
 
+static Optional<StringRef> getDriverMode(const CommandLineArguments &Args) {
+  for (const auto &Arg : Args) {
+    StringRef ArgRef = Arg;
+    if (ArgRef.startswith("--driver-mode=")) {
+      auto EqualSignIndex = ArgRef.find('=');
+      return StringRef(ArgRef.data() + EqualSignIndex + 1);
+    }
+  }
+  return StringRef();
+}
+
 /// Add -fsyntax-only option and drop options that triggers output generation.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
@@ -93,13 +104,27 @@
 
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+    auto DriverMode = getDriverMode(Args);
+    auto UsingClDriver = (DriverMode.hasValue() && *DriverMode == "cl");
+
     CommandLineArguments AdjustedArgs;
     for (size_t i = 0, e = Args.size(); i < e; ++i) {
       StringRef Arg = Args[i];
       // All dependency-file options begin with -M. These include -MM,
       // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-      if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-          !Arg.startswith("-showIncludes")) {
+      // Dependency-file options in MSVC do not begin with -M, so we
+      // do not remove those when using the cl driver.
+      bool IsDependencyFileArg;
+      if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
+        IsDependencyFileArg = true;
+      else if (UsingClDriver)
+        IsDependencyFileArg = false;
+      else if (Arg.startswith("-M"))
+        IsDependencyFileArg = true;
+      else
+        IsDependencyFileArg = false;
+
+      if (!IsDependencyFileArg) {
         AdjustedArgs.push_back(Args[i]);
         continue;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86999.289344.patch
Type: text/x-patch
Size: 3632 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200902/13778459/attachment.bin>