[clang] 87de54d - [clang][Tooling] Fix addTargetAndModeForProgramName to use correct flag names

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-03T11:46:58+02:00
New Revision: 87de54dbb6efa0fc5e304f94b350a39066bc2759

URL: https://github.com/llvm/llvm-project/commit/87de54dbb6efa0fc5e304f94b350a39066bc2759
DIFF: https://github.com/llvm/llvm-project/commit/87de54dbb6efa0fc5e304f94b350a39066bc2759.diff

LOG: [clang][Tooling] Fix addTargetAndModeForProgramName to use correct flag names

The logic was using incorrect flag versions. For example:
- `-target=` can't be a prefix, it must be `--target=`.
- `--driver-mode` can't appear on its own, value must be attached to it.

While fixing those, also changes the append logic to make use of new
`--target=X` format instead of the legacy `-target X` version.

In addition to that makes use of the OPTTable instead of hardcoded strings to
make sure helper also gets updated if clang's options are modified.

Differential Revision: https://reviews.llvm.org/D85076

Added: 
    

Modified: 
    clang/lib/Tooling/Tooling.cpp
    clang/unittests/Tooling/ToolingTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index 40b6cff0d627..0593f0cc1d19 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -245,27 +245,37 @@ std::string getAbsolutePath(StringRef File) {
 
 void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine,
                                     StringRef InvokedAs) {
-  if (!CommandLine.empty() && !InvokedAs.empty()) {
-    bool AlreadyHasTarget = false;
-    bool AlreadyHasMode = false;
-    // Skip CommandLine[0].
-    for (auto Token = ++CommandLine.begin(); Token != CommandLine.end();
-         ++Token) {
-      StringRef TokenRef(*Token);
-      AlreadyHasTarget |=
-          (TokenRef == "-target" || TokenRef.startswith("-target="));
-      AlreadyHasMode |= (TokenRef == "--driver-mode" ||
-                         TokenRef.startswith("--driver-mode="));
-    }
-    auto TargetMode =
-        driver::ToolChain::getTargetAndModeFromProgramName(InvokedAs);
-    if (!AlreadyHasMode && TargetMode.DriverMode) {
-      CommandLine.insert(++CommandLine.begin(), TargetMode.DriverMode);
-    }
-    if (!AlreadyHasTarget && TargetMode.TargetIsValid) {
-      CommandLine.insert(++CommandLine.begin(), {"-target",
-                                                 TargetMode.TargetPrefix});
-    }
+  if (CommandLine.empty() || InvokedAs.empty())
+    return;
+  const auto &Table = driver::getDriverOptTable();
+  // --target=X
+  const std::string TargetOPT =
+      Table.getOption(driver::options::OPT_target).getPrefixedName();
+  // -target X
+  const std::string TargetOPTLegacy =
+      Table.getOption(driver::options::OPT_target_legacy_spelling)
+          .getPrefixedName();
+  // --driver-mode=X
+  const std::string DriverModeOPT =
+      Table.getOption(driver::options::OPT_driver_mode).getPrefixedName();
+  bool AlreadyHasTarget = false;
+  bool AlreadyHasMode = false;
+  // Skip CommandLine[0].
+  for (auto Token = ++CommandLine.begin(); Token != CommandLine.end();
+       ++Token) {
+    StringRef TokenRef(*Token);
+    AlreadyHasTarget |=
+        TokenRef.startswith(TargetOPT) || TokenRef.equals(TargetOPTLegacy);
+    AlreadyHasMode |= TokenRef.startswith(DriverModeOPT);
+  }
+  auto TargetMode =
+      driver::ToolChain::getTargetAndModeFromProgramName(InvokedAs);
+  if (!AlreadyHasMode && TargetMode.DriverMode) {
+    CommandLine.insert(++CommandLine.begin(), TargetMode.DriverMode);
+  }
+  if (!AlreadyHasTarget && TargetMode.TargetIsValid) {
+    CommandLine.insert(++CommandLine.begin(),
+                       TargetOPT + TargetMode.TargetPrefix);
   }
 }
 

diff  --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp
index 5bd2864b5ba1..cc6f453284d7 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -621,7 +621,7 @@ TEST(addTargetAndModeForProgramName, AddsTargetAndMode) {
   addTargetAndModeForProgramName(Args, "");
   EXPECT_EQ((std::vector<std::string>{"clang", "-foo"}), Args);
   addTargetAndModeForProgramName(Args, Target + "-g++");
-  EXPECT_EQ((std::vector<std::string>{"clang", "-target", Target,
+  EXPECT_EQ((std::vector<std::string>{"clang", "--target=" + Target,
                                       "--driver-mode=g++", "-foo"}),
             Args);
 }
@@ -635,7 +635,7 @@ TEST(addTargetAndModeForProgramName, PathIgnored) {
 
   std::vector<std::string> Args = {"clang", "-foo"};
   addTargetAndModeForProgramName(Args, ToolPath);
-  EXPECT_EQ((std::vector<std::string>{"clang", "-target", Target,
+  EXPECT_EQ((std::vector<std::string>{"clang", "--target=" + Target,
                                       "--driver-mode=g++", "-foo"}),
             Args);
 }
@@ -650,10 +650,10 @@ TEST(addTargetAndModeForProgramName, IgnoresExistingTarget) {
                                       "-target", "something"}),
             Args);
 
-  std::vector<std::string> ArgsAlt = {"clang", "-foo", "-target=something"};
+  std::vector<std::string> ArgsAlt = {"clang", "-foo", "--target=something"};
   addTargetAndModeForProgramName(ArgsAlt, Target + "-g++");
   EXPECT_EQ((std::vector<std::string>{"clang", "--driver-mode=g++", "-foo",
-                                      "-target=something"}),
+                                      "--target=something"}),
             ArgsAlt);
 }
 
@@ -663,15 +663,9 @@ TEST(addTargetAndModeForProgramName, IgnoresExistingMode) {
 
   std::vector<std::string> Args = {"clang", "-foo", "--driver-mode=abc"};
   addTargetAndModeForProgramName(Args, Target + "-g++");
-  EXPECT_EQ((std::vector<std::string>{"clang", "-target", Target, "-foo",
+  EXPECT_EQ((std::vector<std::string>{"clang", "--target=" + Target, "-foo",
                                       "--driver-mode=abc"}),
             Args);
-
-  std::vector<std::string> ArgsAlt = {"clang", "-foo", "--driver-mode", "abc"};
-  addTargetAndModeForProgramName(ArgsAlt, Target + "-g++");
-  EXPECT_EQ((std::vector<std::string>{"clang", "-target", Target, "-foo",
-                                      "--driver-mode", "abc"}),
-            ArgsAlt);
 }
 
 #ifndef _WIN32