[llvm] e28cd75 - [OptTable] Reapply Improve error message output for grouped short options
via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 3 03:14:17 PDT 2021
Author: gbreynoo
Date: 2021-09-03T11:13:52+01:00
New Revision: e28cd75a5039893db5861f49eeea9eb5b59bdcdc
URL: https://github.com/llvm/llvm-project/commit/e28cd75a5039893db5861f49eeea9eb5b59bdcdc
DIFF: https://github.com/llvm/llvm-project/commit/e28cd75a5039893db5861f49eeea9eb5b59bdcdc.diff
LOG: [OptTable] Reapply Improve error message output for grouped short options
This reapplies 71d7fed3bc2ad6c22729d446526a59fcfd99bd03 which was
reverted by 3e2bd82f02c6cbbfb0544897c7645867f04b3a7e. This change
includes the fix for breaking the sanitizer bots.
As seen in https://bugs.llvm.org/show_bug.cgi?id=48880 the current
implementation for parsing grouped short options can return unclear
error messages. This change fixes the example given in the ticket in
which a flag is incorrectly given an argument. Also when parsing a
group we now keep reading past the first incorrect option and output
errors for all incorrect options in the group.
Differential Revision: https://reviews.llvm.org/D108770
Added:
Modified:
llvm/lib/Option/OptTable.cpp
llvm/test/tools/llvm-objcopy/tool-help-message.test
llvm/unittests/Option/OptionParsingTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index a994a42639ed9..1bf63a75e31f5 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -375,15 +375,24 @@ Arg *OptTable::parseOneArgGrouped(InputArgList &Args, unsigned &Index) const {
}
if (Fallback) {
Option Opt(Fallback, this);
+ // Check that the last option isn't a flag wrongly given an argument.
+ if (Str[2] == '=')
+ return new Arg(getOption(TheUnknownOptionID), Str, Index++, CStr);
+
if (Arg *A = Opt.accept(Args, Str.substr(0, 2), true, Index)) {
- if (Str.size() == 2)
- ++Index;
- else
- Args.replaceArgString(Index, Twine('-') + Str.substr(2));
+ Args.replaceArgString(Index, Twine('-') + Str.substr(2));
return A;
}
}
+ // In the case of an incorrect short option extract the character and move to
+ // the next one.
+ if (Str[1] != '-') {
+ CStr = Args.MakeArgString(Str.substr(0, 2));
+ Args.replaceArgString(Index, Twine('-') + Str.substr(2));
+ return new Arg(getOption(TheUnknownOptionID), CStr, Index, CStr);
+ }
+
return new Arg(getOption(TheUnknownOptionID), Str, Index++, CStr);
}
diff --git a/llvm/test/tools/llvm-objcopy/tool-help-message.test b/llvm/test/tools/llvm-objcopy/tool-help-message.test
index c51221b3d3a80..7e72acbbe5073 100644
--- a/llvm/test/tools/llvm-objcopy/tool-help-message.test
+++ b/llvm/test/tools/llvm-objcopy/tool-help-message.test
@@ -2,31 +2,31 @@
# RUN: llvm-objcopy --help | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
# RUN: not llvm-objcopy 2>&1 | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
# RUN: not llvm-objcopy -- 2>&1 | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
-# RUN: not llvm-objcopy -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
-# RUN: not llvm-objcopy --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
+# RUN: not llvm-objcopy -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-SHORT %s
+# RUN: not llvm-objcopy --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
# RUN: not llvm-objcopy --strip-debug 2>&1 | FileCheck %s --check-prefix=NO-INPUT-FILES
# RUN: llvm-strip -h | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
# RUN: llvm-strip --help | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
# RUN: not llvm-strip 2>&1 | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
# RUN: not llvm-strip -- 2>&1 | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
-# RUN: not llvm-strip -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
-# RUN: not llvm-strip --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
+# RUN: not llvm-strip -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-SHORT %s
+# RUN: not llvm-strip --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
# RUN: not llvm-strip --strip-debug 2>&1 | FileCheck %s --check-prefix=NO-INPUT-FILES
# RUN: llvm-install-name-tool -h | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
# RUN: llvm-install-name-tool --help | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
# RUN: not llvm-install-name-tool 2>&1 | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
-# RUN: not llvm-install-name-tool -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
-# RUN: not llvm-install-name-tool --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
+# RUN: not llvm-install-name-tool -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
+# RUN: not llvm-install-name-tool --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
# RUN: not llvm-install-name-tool -add_rpath @executable 2>&1 | FileCheck %s --check-prefix=NO-INPUT-FILES
# RUN: not llvm-install-name-tool -add_rpath @executable f1 f2 2>&1 | FileCheck %s --check-prefix=MULTIPLE-INPUT-FILES
# RUN: llvm-bitcode-strip -h | FileCheck --check-prefix=BITCODE-STRIP-USAGE %s --match-full-lines
# RUN: llvm-bitcode-strip --help | FileCheck --check-prefix=BITCODE-STRIP-USAGE %s --match-full-lines
# RUN: not llvm-bitcode-strip 2>&1 | FileCheck --check-prefix=BITCODE-STRIP-USAGE %s --match-full-lines
-# RUN: not llvm-bitcode-strip -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
-# RUN: not llvm-bitcode-strip --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
+# RUN: not llvm-bitcode-strip -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
+# RUN: not llvm-bitcode-strip --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG-LONG %s
# RUN: not llvm-bitcode-strip f1 f2 2>&1 | FileCheck %s --check-prefix=MULTIPLE-INPUT-FILES
# OBJCOPY-USAGE: USAGE: llvm-objcopy [options] input [output]
@@ -41,6 +41,7 @@
# BITCODE-STRIP-USAGE: USAGE: llvm-bitcode-strip [options] input
# BITCODE-STRIP-USAGE: Pass @FILE as argument to read options from FILE.
-# UNKNOWN-ARG: unknown argument '{{-+}}abcabc'
+# UNKNOWN-ARG-SHORT: unknown argument '-a'
+# UNKNOWN-ARG-LONG: unknown argument '{{-+}}abcabc'
# NO-INPUT-FILES: no input file specified
# MULTIPLE-INPUT-FILES: expects a single input file
diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp
index f0299d2d58922..520801d5aba72 100644
--- a/llvm/unittests/Option/OptionParsingTest.cpp
+++ b/llvm/unittests/Option/OptionParsingTest.cpp
@@ -376,3 +376,29 @@ TEST(Option, UnknownOptions) {
EXPECT_EQ("--long", Unknown[1]);
}
}
+
+TEST(Option, FlagsWithoutValues) {
+ TestOptTable T;
+ T.setGroupedShortOptions(true);
+ unsigned MAI, MAC;
+ const char *Args[] = {"-A=1", "-A="};
+ InputArgList AL = T.ParseArgs(Args, MAI, MAC);
+ const std::vector<std::string> Unknown = AL.getAllArgValues(OPT_UNKNOWN);
+ ASSERT_EQ((size_t)2, Unknown.size());
+ EXPECT_EQ("-A=1", Unknown[0]);
+ EXPECT_EQ("-A=", Unknown[1]);
+}
+
+TEST(Option, UnknownGroupedShortOptions) {
+ TestOptTable T;
+ T.setGroupedShortOptions(true);
+ unsigned MAI, MAC;
+ const char *Args[] = {"-AuzK", "-AuzK"};
+ InputArgList AL = T.ParseArgs(Args, MAI, MAC);
+ const std::vector<std::string> Unknown = AL.getAllArgValues(OPT_UNKNOWN);
+ ASSERT_EQ((size_t)4, Unknown.size());
+ EXPECT_EQ("-u", Unknown[0]);
+ EXPECT_EQ("-z", Unknown[1]);
+ EXPECT_EQ("-u", Unknown[2]);
+ EXPECT_EQ("-z", Unknown[3]);
+}
More information about the llvm-commits
mailing list