hamzasood updated this revision to Diff 163098.
hamzasood added a comment.

I've addressed most of your comments and resolved the merge conflicts 
introduced in r340838.


https://reviews.llvm.org/D51321

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -14,6 +14,8 @@
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Path.h"
+
+#define GTEST_HAS_COMBINE 1
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -644,15 +646,53 @@
   }
 };
 
-class InterpolateTest : public ::testing::Test {
+enum class DriverExe { Clang, ClangCL };
+enum class DriverMode { Implicit, GCC, CL };
+
+class InterpolateTest
+    : public ::testing::TestWithParam<std::tuple<DriverExe, DriverMode>> {
+
+  StringRef getClangExe() const {
+    switch (std::get<0>(GetParam())) {
+    case DriverExe::Clang:   return "clang";
+    case DriverExe::ClangCL: return "clang-cl";
+    }
+    llvm_unreachable("Invalid DriverExe");
+  }
+
+  StringRef getDriverModeFlag() const {
+    switch (std::get<1>(GetParam())) {
+    case DriverMode::Implicit: return StringRef();
+    case DriverMode::GCC: return "--driver-mode=gcc";
+    case DriverMode::CL:  return "--driver-mode=cl";
+    }
+    llvm_unreachable("Invalid DriverMode");
+  }
+
 protected:
+  bool isTestingCL() const {
+    switch (std::get<1>(GetParam())) {
+    case DriverMode::Implicit:
+      return std::get<0>(GetParam()) == DriverExe::ClangCL;
+    case DriverMode::GCC: return false;
+    case DriverMode::CL:  return true;
+    }
+    llvm_unreachable("Invalid DriverMode");
+  }
+
   // Adds an entry to the underlying compilation database.
   // A flag is injected: -D <File>, so the command used can be identified.
   void add(llvm::StringRef File, llvm::StringRef Flags = "") {
-    llvm::SmallVector<StringRef, 8> Argv = {"clang", File, "-D", File};
+    llvm::SmallVector<StringRef, 8> Argv;
+    Argv.push_back(getClangExe());
+    if (!getDriverModeFlag().empty())
+      Argv.push_back(getDriverModeFlag());
+    Argv.append({"-D", File});
     llvm::SplitString(Flags, Argv);
+
     llvm::SmallString<32> Dir;
     llvm::sys::path::system_temp_directory(false, Dir);
+
     Entries[path(File)].push_back(
         {Dir, path(File), {Argv.begin(), Argv.end()}, "foo.o"});
   }
@@ -675,69 +715,124 @@
             ->getCompileCommands(path(F));
     if (Results.empty())
       return "none";
-    // drop the input file argument, so tests don't have to deal with path().
-    EXPECT_EQ(Results[0].CommandLine.back(), path(F))
-        << "Last arg should be the file";
-    Results[0].CommandLine.pop_back();
-    return llvm::join(Results[0].CommandLine, " ");
+
+    // To simply the test checks, we're building a new command line that doesn't
+    // contain the uninteresting stuff (exe name etc.).
+    ArrayRef<std::string> CmdLine = Results[0].CommandLine;
+
+    // Drop the clang executable path.
+    EXPECT_EQ(CmdLine.front(), getClangExe()) << "First arg should be clang";
+    CmdLine = CmdLine.drop_front();
+
+    // Drop the --driver-mode flag.
+    if (!getDriverModeFlag().empty()) {
+      EXPECT_EQ(CmdLine.front(), getDriverModeFlag())
+        << "Incorrect driver mode flag";
+      CmdLine = CmdLine.drop_front();
+    }
+
+    // Drop the input file.
+    EXPECT_EQ(CmdLine.back(), path(F)) << "Last arg should be the file";
+    CmdLine = CmdLine.drop_back();
+
+    return llvm::join(CmdLine, " ");
   }
 
   MemCDB::EntryMap Entries;
 };
 
-TEST_F(InterpolateTest, Nearby) {
+TEST_P(InterpolateTest, Nearby) {
   add("dir/foo.cpp");
   add("dir/bar.cpp");
   add("an/other/foo.cpp");
 
   // great: dir and name both match (prefix or full, case insensitive)
-  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
-  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/f.cpp"), "-D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "-D dir/foo.cpp");
   // no name match. prefer matching dir, break ties by alpha
-  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  EXPECT_EQ(getCommand("dir/a.cpp"), "-D dir/bar.cpp");
   // an exact name match beats one segment of directory match
   EXPECT_EQ(getCommand("some/other/bar.h"),
-            "clang -D dir/bar.cpp -x c++-header");
+            isTestingCL()
+                ? "-D dir/bar.cpp /TP"
+                : "-D dir/bar.cpp -x c++-header");
   // two segments of directory match beat a prefix name match
-  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "-D an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
   EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-            "clang -D an/other/foo.cpp");
+            "-D an/other/foo.cpp");
 }
 
-TEST_F(InterpolateTest, Language) {
-  add("dir/foo.cpp", "-std=c++17");
+TEST_P(InterpolateTest, Language) {
+  add("dir/foo.cpp", isTestingCL() ? "/std:c++17" : "-std=c++17");
   add("dir/bar.c", "");
-  add("dir/baz.cee", "-x c");
+  add("dir/baz.cee", isTestingCL() ? "/TC" : "-x c");
 
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
-            "clang -D dir/foo.cpp -x c++-header -std=c++17");
+            isTestingCL()
+                ? "-D dir/foo.cpp /TP /std:c++17"
+                : "-D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
-  EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
+  EXPECT_EQ(getCommand("foo.hpp"),
+                       isTestingCL()
+                           ? "-D dir/foo.cpp /std:c++17"
+                           : "-D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
-  EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
+  EXPECT_EQ(getCommand("baz.h"),
+                       isTestingCL()
+                           ? "-D dir/baz.cee /TC"
+                           : "-D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
-  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  EXPECT_EQ(getCommand("foo.c"), "-D dir/bar.c");
   // make sure we don't crash on queries with invalid extensions.
-  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("foo.cce"), "-D dir/foo.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
-  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("foo.c"), "-D dir/foo.cpp");
 }
 
-TEST_F(InterpolateTest, Strip) {
-  add("dir/foo.cpp", "-o foo.o -Wall");
-  // the -o option and the input file are removed, but -Wall is preserved.
-  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+TEST_P(InterpolateTest, Strip) {
+  // The input and output are removed, but the warning level is preserved.
+  if (isTestingCL()) {
+    add("dir/foo.cpp", "/Fofoo.o /W3");
+    EXPECT_EQ(getCommand("dir/bar.cpp"), "-D dir/foo.cpp /W3");
+  } else {
+    add("dir/foo.cpp", "-o foo.o -Wall");
+    EXPECT_EQ(getCommand("dir/bar.cpp"), "-D dir/foo.cpp -Wall");
+  }
 }
 
-TEST_F(InterpolateTest, Case) {
+TEST_P(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
   // Case mismatches are completely ignored, so we choose the name match.
-  EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
-}
+  EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "-D FOO/BAR/BAZ/SHOUT.cc");
+}
+
+INSTANTIATE_TEST_CASE_P(DriverModes, InterpolateTest,
+  ::testing::Combine(
+    ::testing::Values(DriverExe::Clang, DriverExe::ClangCL),
+    ::testing::Values(DriverMode::Implicit, DriverMode::GCC, DriverMode::CL)),
+
+  [](const ::testing::TestParamInfo<std::tuple<DriverExe, DriverMode>> &Info) {
+    std::string Name;
+    {
+      llvm::raw_string_ostream OS(Name);
+
+      switch (std::get<0>(Info.param)) {
+      case DriverExe::Clang:   OS << "Clang"; break;
+      case DriverExe::ClangCL: OS << "ClangCL"; break;
+      }
+
+      switch (std::get<1>(Info.param)) {
+      case DriverMode::Implicit: break;
+      case DriverMode::GCC: OS << "WithModeGCC"; break;
+      case DriverMode::CL:  OS << "WithModeCL"; break;
+      }
+    }
+    return Name;
+  });
 
 TEST(CompileCommandTest, EqualityOperator) {
   CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -48,6 +48,7 @@
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
@@ -127,47 +128,22 @@
   Optional<types::ID> Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
+  // Whether the command is for the CL driver.
+  bool IsCL = false;
 
   TransferableCommand(CompileCommand C)
       : Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
-    std::vector<std::string> NewArgs = {Cmd.CommandLine.front()};
-    // Parse the old args in order to strip out and record unwanted flags.
-    auto OptTable = clang::driver::createDriverOptTable();
-    std::vector<const char *> Argv;
-    for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
-      Argv.push_back(Cmd.CommandLine[I].c_str());
-    unsigned MissingI, MissingC;
-    auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-    for (const auto *Arg : ArgList) {
-      const auto &option = Arg->getOption();
-      // Strip input and output files.
-      if (option.matches(clang::driver::options::OPT_INPUT) ||
-          option.matches(clang::driver::options::OPT_o)) {
-        continue;
-      }
-      // Strip -x, but record the overridden language.
-      if (option.matches(clang::driver::options::OPT_x)) {
-        for (const char *Value : Arg->getValues())
-          Type = types::lookupTypeForTypeSpecifier(Value);
-        continue;
-      }
-      // Strip --std, but record the value.
-      if (option.matches(clang::driver::options::OPT_std_EQ)) {
-        for (const char *Value : Arg->getValues()) {
-          Std = llvm::StringSwitch<LangStandard::Kind>(Value)
-#define LANGSTANDARD(id, name, lang, desc, features)                           \
-  .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
-                    .Default(Std);
-        }
-        continue;
-      }
-      llvm::opt::ArgStringList ArgStrs;
-      Arg->render(ArgList, ArgStrs);
-      NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
+    std::vector<std::string> OldArgs = std::move(Cmd.CommandLine);
+    Cmd.CommandLine.clear();
+
+    // Copy over the old arguments.
+    {
+      SmallVector<const char *, 8> TmpArgv;
+      TmpArgv.reserve(OldArgs.size());
+      llvm::transform(OldArgs, std::back_inserter(TmpArgv),
+                      [](const std::string &S) { return S.c_str(); });
+      processInputCommandLine(TmpArgv);
     }
-    Cmd.CommandLine = std::move(NewArgs);
 
     if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
       Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
@@ -188,21 +164,43 @@
       TargetType = types::onlyPrecompileType(TargetType) // header?
                        ? types::lookupHeaderTypeForSourceType(*Type)
                        : *Type;
-      Result.CommandLine.push_back("-x");
-      Result.CommandLine.push_back(types::getTypeName(TargetType));
+      if (IsCL) {
+        StringRef Flag = toCLFlag(TargetType);
+        if (!Flag.empty())
+          Result.CommandLine.push_back(Flag);
+      } else {
+        Result.CommandLine.push_back("-x");
+        Result.CommandLine.push_back(types::getTypeName(TargetType));
+      }
     }
     // --std flag may only be transferred if the language is the same.
     // We may consider "translating" these, e.g. c++11 -> c11.
     if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
-      Result.CommandLine.push_back(
-          "-std=" +
-          std::string(LangStandard::getLangStandardForKind(Std).getName()));
+      Result.CommandLine.emplace_back((
+          llvm::Twine(IsCL ? "/std:" : "-std=") +
+          LangStandard::getLangStandardForKind(Std).getName()).str());
     }
     Result.CommandLine.push_back(Filename);
     return Result;
   }
 
 private:
+  // Determine whether the given command line is intended for the CL driver.
+  static bool checkIsCLMode(ArrayRef<const char *> CmdLine,
+                            const llvm::opt::OptTable &OptTable) {
+    const auto DriverModeName =
+      OptTable.getOption(driver::options::OPT_driver_mode).getPrefixedName();
+
+    // First check for a --driver-mode override.
+    for (StringRef S : llvm::reverse(CmdLine)) {
+      if (S.startswith(DriverModeName))
+        return S.drop_front(DriverModeName.length()) == "cl";
+    }
+
+    // Otherwise just check the clang executable file name.
+    return llvm::sys::path::stem(CmdLine.front()).endswith_lower("cl");
+  }
+
   // Map the language from the --std flag to that of the -x flag.
   static types::ID toType(InputKind::Language Lang) {
     switch (Lang) {
@@ -218,6 +216,105 @@
       return types::TY_INVALID;
     }
   }
+
+  // Convert a file type to the matching CL-style type flag.
+  static StringRef toCLFlag(types::ID Type) {
+    switch (Type) {
+    case types::TY_C:
+    case types::TY_CHeader:
+      return "/TC";
+    case types::TY_CXX:
+    case types::TY_CXXHeader:
+      return "/TP";
+    default:
+      return StringRef();
+    }
+  }
+
+  // Try to interpret the argument as a type specifier, e.g. '-x'.
+  Optional<types::ID> tryParseTypeArg(const llvm::opt::Arg &Arg) {
+    const llvm::opt::Option &Opt = Arg.getOption();
+    using namespace driver::options;
+    if (IsCL) {
+      if (Opt.matches(OPT__SLASH_TC) || Opt.matches(OPT__SLASH_Tc))
+        return types::TY_C;
+      if (Opt.matches(OPT__SLASH_TP) || Opt.matches(OPT__SLASH_Tp))
+        return types::TY_CXX;
+    } else {
+      if (Opt.matches(driver::options::OPT_x))
+        return types::lookupTypeForTypeSpecifier(Arg.getValue());
+    }
+    return None;
+  }
+
+  // Try to interpret the argument as '-std='.
+  Optional<LangStandard::Kind> tryParseStdArg(const llvm::opt::Arg &Arg) {
+    using namespace driver::options;
+    if (Arg.getOption().matches(IsCL ? OPT__SLASH_std : OPT_std_EQ)) {
+      return llvm::StringSwitch<LangStandard::Kind>(Arg.getValue())
+#define LANGSTANDARD(id, name, lang, ...) .Case(name, LangStandard::lang_##id)
+#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
+#include "clang/Frontend/LangStandards.def"
+#undef LANGSTANDARD_ALIAS
+#undef LANGSTANDARD
+                 .Default(LangStandard::lang_unspecified);
+    }
+    return None;
+  }
+
+  // Store a copy of the input arguments, making changes as appropriate.
+  void processInputCommandLine(ArrayRef<const char *> OldArgv) {
+    assert(Cmd.CommandLine.empty() &&
+           "Trying to overwrite an existing command line!");
+
+    auto OptTable = driver::createDriverOptTable();
+    IsCL = checkIsCLMode(OldArgv, *OptTable);
+
+    // Parse the old args in order to strip out and record unwanted flags.
+    // We parse each argument individually so that we can retain the exact
+    // spelling of each argument; re-rendering is lossy for aliased flags.
+    // E.g. in CL mode, /W4 maps to -Wall.
+    llvm::opt::InputArgList ArgList(OldArgv.begin(), OldArgv.end());
+    Cmd.CommandLine.push_back(OldArgv.front());
+    for (unsigned Pos = 1; Pos < OldArgv.size();) {
+      using namespace driver::options;
+
+      const unsigned OldPos = Pos;
+      std::unique_ptr<llvm::opt::Arg> Arg(
+        OptTable->ParseOneArg(ArgList, Pos,
+                              /* Include */IsCL ? CoreOption | CLOption : 0,
+                              /* Exclude */IsCL ? 0 : driver::options::CLOption));
+
+      if (!Arg)
+        continue;
+
+      const llvm::opt::Option &Opt = Arg->getOption();
+
+      // Strip input and output files.
+      if (Opt.matches(OPT_INPUT) || Opt.matches(OPT_o) ||
+          (IsCL && (Opt.matches(OPT__SLASH_Fa) ||
+                    Opt.matches(OPT__SLASH_Fe) ||
+                    Opt.matches(OPT__SLASH_Fi) ||
+                    Opt.matches(OPT__SLASH_Fo))))
+        continue;
+
+      // Strip -x, but record the overridden language.
+      if (const auto GivenType = tryParseTypeArg(*Arg)) {
+        Type = *GivenType;
+        continue;
+      }
+
+      // Strip -std, but record the value.
+      if (const auto GivenStd = tryParseStdArg(*Arg)) {
+        if (*GivenStd != LangStandard::lang_unspecified)
+          Std = *GivenStd;
+        continue;
+      }
+
+      Cmd.CommandLine.insert(Cmd.CommandLine.end(),
+                             &OldArgv[OldPos], &OldArgv[Pos]);
+    }
+  }
 };
 
 // Given a filename, FileIndex picks the best matching file from the underlying
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to