[PATCH] D56901: [Support] Fix Windows Command Shell Command line parsing for quoted arguments

[MyDeveloperDay via Phabricator via llvm-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: chandlerc, Bigcheese, jdenny, probinson, JonasToth.
Herald added a subscriber: kristina.

PR38471 - Check readability-identifier-naming is not working on Windows 10

clang-tidy command line parsing fails to read single quoted -checks when using windows cmd.exe prompt

The following will fail to read the values of -checks due argv not removing the single quote (') on windows cmd shell (unlike unix variants & cygwin)
see (https://bugs.llvm.org/show_bug.cgi?id=38471) for more details

  clang-tidy.exe -checks='-*,readability-identifier-naming' test.cpp -- -std=c++11

this results in Windows only supporting double quote usage and this is confusing with lots of the examples using single quotes

  clang-tidy.exe -checks="-*,readability-identifier-naming" test.cpp -- -std=c++11

As I'm fairly new to LLVM this may not be the best place to do this, whilst this was reported up in clang-tidy I suspect it could have an effect on any of the llvm tools being run in a windows command shell (but not inside bash.exe)

If you think this can be done better elsewhere or is unnecessary I'll be happy to change/abandon it

Picking reviewers from code owners and common reviewers to changes in this area feel free to adjust as necessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D56901

Files:
  lib/Support/CommandLine.cpp
  unittests/Support/CommandLineTest.cpp


Index: unittests/Support/CommandLineTest.cpp
===================================================================
--- unittests/Support/CommandLineTest.cpp
+++ unittests/Support/CommandLineTest.cpp
@@ -914,4 +914,30 @@
   EXPECT_TRUE(MacroDefs.front().compare("HAVE_FOO") == 0);
 }
 
+TEST(CommandLineTest, WindowsCmdShellQuotesArg) {
+  if (!Triple(sys::getProcessTriple()).isOSWindows())
+    return;
+
+  StackOption<std::string, cl::opt<std::string>> Checks(
+      "checks", cl::desc("The checks"), cl::init(""));
+
+  const char *argsCmdShell[] = {"clang-tidy.exe",
+                                "-checks='-*,readability-identifier-naming'"};
+
+  EXPECT_TRUE(cl::ParseCommandLineOptions(2, argsCmdShell, StringRef(),
+                                          &llvm::nulls()));
+
+  EXPECT_TRUE(Checks.compare("-*,readability-identifier-naming") == 0);
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *argsCmdShell2[] = {"clang-tidy.exe",
+                                 "-checks=-*,readability-identifier-naming"};
+
+  EXPECT_TRUE(cl::ParseCommandLineOptions(2, argsCmdShell2, StringRef(),
+                                          &llvm::nulls()));
+
+  EXPECT_TRUE(Checks.compare("-*,readability-identifier-naming") == 0);
+}
+
 } // anonymous namespace
Index: lib/Support/CommandLine.cpp
===================================================================
--- lib/Support/CommandLine.cpp
+++ lib/Support/CommandLine.cpp
@@ -439,6 +439,14 @@
 
   Value = Arg.substr(EqualPos + 1);
   Arg = Arg.substr(0, EqualPos);
+
+  // Windows Cmd Shell will not remove the '' from a --checks='abc' type command
+  // when passed via argv, unlike Linux or cygwin shells.
+  if (Triple(sys::getProcessTriple()).isOSWindows()) {
+    // Pull off the leading and trailing 's.
+    if (Value.size() > 1 && Value[0] == '\'' && Value[Value.size() - 1] == '\'')
+      Value = Value.substr(1, Value.size() - 2);
+  }
   return I->second;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56901.182470.patch
Type: text/x-patch
Size: 1945 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190118/a7b75399/attachment.bin>