[OptTable] Reapply Improve error message output for grouped short options

This reapplies 71d7fed3bc which was
reverted by 3e2bd82f02. 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
This commit is contained in:
gbreynoo 2021-09-03 11:08:39 +01:00
parent 9e3f86e273
commit e28cd75a50
3 changed files with 49 additions and 13 deletions

View File

@ -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);
}

View File

@ -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

View File

@ -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]);
}