maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Modify ParseArgs to return the InputArgList by value - there's no need for dynamic allocation/ownership here 

The one caller that does anything other than keep this variable on the
stack is the single use of DerivedArgList in Clang, which is a bit more
interesting but can probably be cleaned up/simplified a bit further
(have DerivedArgList take ownership of the InputArgList rather than
needing to reference its Args indirectly) which I'll try to after this.

llvm-svn: 240345
This commit is contained in:
David Blaikie 2015-06-22 22:06:37 +00:00
parent 69533a5a85
commit db3d31d0be
5 changed files with 115 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
llvm/include/llvm/Option/ArgList.h
View File

@ -92,10 +92,6 @@ public:
/// check for the presence of Arg instances for a particular Option
/// and to iterate over groups of arguments.
class ArgList {
private:
ArgList(const ArgList &) = delete;
void operator=(const ArgList &) = delete;
public:
typedef SmallVector<Arg*, 16> arglist_type;
typedef arglist_type::iterator iterator;
@ -108,9 +104,21 @@ private:
arglist_type Args;
protected:
// Default ctor provided explicitly as it is not provided implicitly due to
// the presence of the (deleted) copy ctor above.
// Make the default special members protected so they won't be used to slice
// derived objects, but can still be used by derived objects to implement
// their own special members.
ArgList() = default;
// Explicit move operations to ensure the container is cleared post-move
// otherwise it could lead to a double-delete in the case of moving of an
// InputArgList which deletes the contents of the container. If we could fix
// up the ownership here (delegate storage/ownership to the derived class so
// it can be a container of unique_ptr) this would be simpler.
ArgList(ArgList &&RHS) : Args(std::move(RHS.Args)) { RHS.Args.clear(); }
ArgList &operator=(ArgList &&RHS) {
Args = std::move(RHS.Args);
RHS.Args.clear();
return *this;
}
// Protect the dtor to ensure this type is never destroyed polymorphically.
~ArgList() = default;
@ -319,6 +327,19 @@ private:
public:
InputArgList(const char* const *ArgBegin, const char* const *ArgEnd);
// Default move operations implemented for the convenience of MSVC. Nothing
// special here.
InputArgList(InputArgList &&RHS)
: ArgList(std::move(RHS)), ArgStrings(std::move(RHS.ArgStrings)),
SynthesizedStrings(std::move(RHS.SynthesizedStrings)),
NumInputArgStrings(RHS.NumInputArgStrings) {}
InputArgList &operator=(InputArgList &&RHS) {
ArgList::operator=(std::move(RHS));
ArgStrings = std::move(RHS.ArgStrings);
SynthesizedStrings = std::move(RHS.SynthesizedStrings);
NumInputArgStrings = RHS.NumInputArgStrings;
return *this;
}
~InputArgList();
const char *getArgString(unsigned Index) const override {

7
llvm/include/llvm/Option/OptTable.h
View File

@ -151,10 +151,9 @@ public:
/// is the default and means exclude nothing.
/// \return An InputArgList; on error this will contain all the options
/// which could be parsed.
InputArgList *ParseArgs(ArrayRef<const char *> Args,
unsigned &MissingArgIndex, unsigned &MissingArgCount,
unsigned FlagsToInclude = 0,
unsigned FlagsToExclude = 0) const;
InputArgList ParseArgs(ArrayRef<const char *> Args, unsigned &MissingArgIndex,
unsigned &MissingArgCount, unsigned FlagsToInclude = 0,
unsigned FlagsToExclude = 0) const;
/// \brief Render the help text for an option table.
///

20
llvm/lib/LibDriver/LibDriver.cpp
View File

@ -113,27 +113,27 @@ int llvm::libDriverMain(llvm::ArrayRef<const char*> ArgsArr) {
LibOptTable Table;
unsigned MissingIndex;
unsigned MissingCount;
std::unique_ptr<llvm::opt::InputArgList> Args(
Table.ParseArgs(ArgsArr.slice(1), MissingIndex, MissingCount));
llvm::opt::InputArgList Args =
Table.ParseArgs(ArgsArr.slice(1), MissingIndex, MissingCount);
if (MissingCount) {
llvm::errs() << "missing arg value for \""
<< Args->getArgString(MissingIndex)
<< "\", expected " << MissingCount
<< Args.getArgString(MissingIndex) << "\", expected "
<< MissingCount
<< (MissingCount == 1 ? " argument.\n" : " arguments.\n");
return 1;
}
for (auto *Arg : Args->filtered(OPT_UNKNOWN))
for (auto *Arg : Args.filtered(OPT_UNKNOWN))
llvm::errs() << "ignoring unknown argument: " << Arg->getSpelling() << "\n";
if (Args->filtered_begin(OPT_INPUT) == Args->filtered_end()) {
if (Args.filtered_begin(OPT_INPUT) == Args.filtered_end()) {
llvm::errs() << "no input files.\n";
return 1;
}
std::vector<StringRef> SearchPaths = getSearchPaths(Args.get(), Saver);
std::vector<StringRef> SearchPaths = getSearchPaths(&Args, Saver);
std::vector<llvm::NewArchiveIterator> Members;
for (auto *Arg : Args->filtered(OPT_INPUT)) {
for (auto *Arg : Args.filtered(OPT_INPUT)) {
Optional<std::string> Path = findInputFile(Arg->getValue(), SearchPaths);
if (!Path.hasValue()) {
llvm::errs() << Arg->getValue() << ": no such file or directory\n";
@ -143,8 +143,8 @@ int llvm::libDriverMain(llvm::ArrayRef<const char*> ArgsArr) {
llvm::sys::path::filename(Arg->getValue()));
}
std::pair<StringRef, std::error_code> Result = llvm::writeArchive(
getOutputPath(Args.get()), Members, /*WriteSymtab=*/true);
std::pair<StringRef, std::error_code> Result =
llvm::writeArchive(getOutputPath(&Args), Members, /*WriteSymtab=*/true);
if (Result.second) {
if (Result.first.empty())
Result.first = ArgsArr[0];

22
llvm/lib/Option/OptTable.cpp
View File

@ -247,12 +247,12 @@ Arg *OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
return new Arg(getOption(TheUnknownOptionID), Str, Index++, Str);
}
InputArgList *OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
unsigned &MissingArgIndex,
unsigned &MissingArgCount,
unsigned FlagsToInclude,
unsigned FlagsToExclude) const {
InputArgList *Args = new InputArgList(ArgArr.begin(), ArgArr.end());
InputArgList OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
unsigned &MissingArgIndex,
unsigned &MissingArgCount,
unsigned FlagsToInclude,
unsigned FlagsToExclude) const {
InputArgList Args(ArgArr.begin(), ArgArr.end());
// FIXME: Handle '@' args (or at least error on them).
@ -260,19 +260,19 @@ InputArgList *OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
unsigned Index = 0, End = ArgArr.size();
while (Index < End) {
// Ingore nullptrs, they are response file's EOL markers
if (Args->getArgString(Index) == nullptr) {
if (Args.getArgString(Index) == nullptr) {
++Index;
continue;
}
// Ignore empty arguments (other things may still take them as arguments).
StringRef Str = Args->getArgString(Index);
StringRef Str = Args.getArgString(Index);
if (Str == "") {
++Index;
continue;
}
unsigned Prev = Index;
Arg *A = ParseOneArg(*Args, Index, FlagsToInclude, FlagsToExclude);
Arg *A = ParseOneArg(Args, Index, FlagsToInclude, FlagsToExclude);
assert(Index > Prev && "Parser failed to consume argument.");
// Check for missing argument error.
@ -284,10 +284,10 @@ InputArgList *OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
break;
}
Args->append(A);
Args.append(A);
}
return Args;
return std::move(Args);
}
static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) {

129
llvm/unittests/Option/OptionParsingTest.cpp
View File

@ -67,26 +67,26 @@ const char *Args[] = {
TEST(Option, OptionParsing) {
TestOptTable T;
unsigned MAI, MAC;
std::unique_ptr<InputArgList> AL(T.ParseArgs(Args, MAI, MAC));
InputArgList AL = T.ParseArgs(Args, MAI, MAC);
// Check they all exist.
EXPECT_TRUE(AL->hasArg(OPT_A));
EXPECT_TRUE(AL->hasArg(OPT_B));
EXPECT_TRUE(AL->hasArg(OPT_C));
EXPECT_TRUE(AL->hasArg(OPT_D));
EXPECT_TRUE(AL->hasArg(OPT_E));
EXPECT_TRUE(AL->hasArg(OPT_F));
EXPECT_TRUE(AL->hasArg(OPT_G));
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_TRUE(AL.hasArg(OPT_B));
EXPECT_TRUE(AL.hasArg(OPT_C));
EXPECT_TRUE(AL.hasArg(OPT_D));
EXPECT_TRUE(AL.hasArg(OPT_E));
EXPECT_TRUE(AL.hasArg(OPT_F));
EXPECT_TRUE(AL.hasArg(OPT_G));
// Check the values.
EXPECT_EQ(AL->getLastArgValue(OPT_B), "hi");
EXPECT_EQ(AL->getLastArgValue(OPT_C), "bye");
EXPECT_EQ(AL->getLastArgValue(OPT_D), "adena");
std::vector<std::string> Es = AL->getAllArgValues(OPT_E);
EXPECT_EQ(AL.getLastArgValue(OPT_B), "hi");
EXPECT_EQ(AL.getLastArgValue(OPT_C), "bye");
EXPECT_EQ(AL.getLastArgValue(OPT_D), "adena");
std::vector<std::string> Es = AL.getAllArgValues(OPT_E);
EXPECT_EQ(Es[0], "apple");
EXPECT_EQ(Es[1], "bloom");
EXPECT_EQ(AL->getLastArgValue(OPT_F), "42");
std::vector<std::string> Gs = AL->getAllArgValues(OPT_G);
EXPECT_EQ(AL.getLastArgValue(OPT_F), "42");
std::vector<std::string> Gs = AL.getAllArgValues(OPT_G);
EXPECT_EQ(Gs[0], "chuu");
EXPECT_EQ(Gs[1], "2");
@ -97,11 +97,11 @@ TEST(Option, OptionParsing) {
EXPECT_NE(Help.find("-A"), std::string::npos);
// Test aliases.
arg_iterator Cs = AL->filtered_begin(OPT_C);
ASSERT_NE(Cs, AL->filtered_end());
arg_iterator Cs = AL.filtered_begin(OPT_C);
ASSERT_NE(Cs, AL.filtered_end());
EXPECT_EQ(StringRef((*Cs)->getValue()), "desu");
ArgStringList ASL;
(*Cs)->render(*AL, ASL);
(*Cs)->render(AL, ASL);
ASSERT_EQ(ASL.size(), 2u);
EXPECT_EQ(StringRef(ASL[0]), "-C");
EXPECT_EQ(StringRef(ASL[1]), "desu");
@ -110,30 +110,29 @@ TEST(Option, OptionParsing) {
TEST(Option, ParseWithFlagExclusions) {
TestOptTable T;
unsigned MAI, MAC;
std::unique_ptr<InputArgList> AL;
// Exclude flag3 to avoid parsing as OPT_SLASH_C.
AL.reset(T.ParseArgs(Args, MAI, MAC,
/*FlagsToInclude=*/0,
/*FlagsToExclude=*/OptFlag3));
EXPECT_TRUE(AL->hasArg(OPT_A));
EXPECT_TRUE(AL->hasArg(OPT_C));
EXPECT_FALSE(AL->hasArg(OPT_SLASH_C));
InputArgList AL = T.ParseArgs(Args, MAI, MAC,
/*FlagsToInclude=*/0,
/*FlagsToExclude=*/OptFlag3);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_TRUE(AL.hasArg(OPT_C));
EXPECT_FALSE(AL.hasArg(OPT_SLASH_C));
// Exclude flag1 to avoid parsing as OPT_C.
AL.reset(T.ParseArgs(Args, MAI, MAC,
/*FlagsToInclude=*/0,
/*FlagsToExclude=*/OptFlag1));
EXPECT_TRUE(AL->hasArg(OPT_B));
EXPECT_FALSE(AL->hasArg(OPT_C));
EXPECT_TRUE(AL->hasArg(OPT_SLASH_C));
AL = T.ParseArgs(Args, MAI, MAC,
/*FlagsToInclude=*/0,
/*FlagsToExclude=*/OptFlag1);
EXPECT_TRUE(AL.hasArg(OPT_B));
EXPECT_FALSE(AL.hasArg(OPT_C));
EXPECT_TRUE(AL.hasArg(OPT_SLASH_C));
const char *NewArgs[] = { "/C", "foo", "--C=bar" };
AL.reset(T.ParseArgs(NewArgs, MAI, MAC));
EXPECT_TRUE(AL->hasArg(OPT_SLASH_C));
EXPECT_TRUE(AL->hasArg(OPT_C));
EXPECT_EQ(AL->getLastArgValue(OPT_SLASH_C), "foo");
EXPECT_EQ(AL->getLastArgValue(OPT_C), "bar");
AL = T.ParseArgs(NewArgs, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_SLASH_C));
EXPECT_TRUE(AL.hasArg(OPT_C));
EXPECT_EQ(AL.getLastArgValue(OPT_SLASH_C), "foo");
EXPECT_EQ(AL.getLastArgValue(OPT_C), "bar");
}
TEST(Option, ParseAliasInGroup) {
@ -141,8 +140,8 @@ TEST(Option, ParseAliasInGroup) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-I" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_TRUE(AL->hasArg(OPT_H));
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_H));
}
TEST(Option, AliasArgs) {
@ -150,10 +149,10 @@ TEST(Option, AliasArgs) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-J", "-Joo" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_TRUE(AL->hasArg(OPT_B));
EXPECT_EQ(AL->getAllArgValues(OPT_B)[0], "foo");
EXPECT_EQ(AL->getAllArgValues(OPT_B)[1], "bar");
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_B));
EXPECT_EQ(AL.getAllArgValues(OPT_B)[0], "foo");
EXPECT_EQ(AL.getAllArgValues(OPT_B)[1], "bar");
}
TEST(Option, IgnoreCase) {
@ -161,9 +160,9 @@ TEST(Option, IgnoreCase) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-a", "-joo" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_TRUE(AL->hasArg(OPT_A));
EXPECT_TRUE(AL->hasArg(OPT_B));
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_TRUE(AL.hasArg(OPT_B));
}
TEST(Option, DoNotIgnoreCase) {
@ -171,9 +170,9 @@ TEST(Option, DoNotIgnoreCase) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-a", "-joo" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_FALSE(AL->hasArg(OPT_A));
EXPECT_FALSE(AL->hasArg(OPT_B));
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_FALSE(AL.hasArg(OPT_A));
EXPECT_FALSE(AL.hasArg(OPT_B));
}
TEST(Option, SlurpEmpty) {
@ -181,10 +180,10 @@ TEST(Option, SlurpEmpty) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-A", "-slurp" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_TRUE(AL->hasArg(OPT_A));
EXPECT_TRUE(AL->hasArg(OPT_Slurp));
EXPECT_EQ(AL->getAllArgValues(OPT_Slurp).size(), 0U);
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_TRUE(AL.hasArg(OPT_Slurp));
EXPECT_EQ(AL.getAllArgValues(OPT_Slurp).size(), 0U);
}
TEST(Option, Slurp) {
@ -192,15 +191,15 @@ TEST(Option, Slurp) {
unsigned MAI, MAC;
const char *MyArgs[] = { "-A", "-slurp", "-B", "--", "foo" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_EQ(AL->size(), 2U);
EXPECT_TRUE(AL->hasArg(OPT_A));
EXPECT_FALSE(AL->hasArg(OPT_B));
EXPECT_TRUE(AL->hasArg(OPT_Slurp));
EXPECT_EQ(AL->getAllArgValues(OPT_Slurp).size(), 3U);
EXPECT_EQ(AL->getAllArgValues(OPT_Slurp)[0], "-B");
EXPECT_EQ(AL->getAllArgValues(OPT_Slurp)[1], "--");
EXPECT_EQ(AL->getAllArgValues(OPT_Slurp)[2], "foo");
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_EQ(AL.size(), 2U);
EXPECT_TRUE(AL.hasArg(OPT_A));
EXPECT_FALSE(AL.hasArg(OPT_B));
EXPECT_TRUE(AL.hasArg(OPT_Slurp));
EXPECT_EQ(AL.getAllArgValues(OPT_Slurp).size(), 3U);
EXPECT_EQ(AL.getAllArgValues(OPT_Slurp)[0], "-B");
EXPECT_EQ(AL.getAllArgValues(OPT_Slurp)[1], "--");
EXPECT_EQ(AL.getAllArgValues(OPT_Slurp)[2], "foo");
}
TEST(Option, FlagAliasToJoined) {
@ -209,9 +208,9 @@ TEST(Option, FlagAliasToJoined) {
// Check that a flag alias provides an empty argument to a joined option.
const char *MyArgs[] = { "-K" };
std::unique_ptr<InputArgList> AL(T.ParseArgs(MyArgs, MAI, MAC));
EXPECT_EQ(AL->size(), 1U);
EXPECT_TRUE(AL->hasArg(OPT_B));
EXPECT_EQ(AL->getAllArgValues(OPT_B).size(), 1U);
EXPECT_EQ(AL->getAllArgValues(OPT_B)[0], "");
InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
EXPECT_EQ(AL.size(), 1U);
EXPECT_TRUE(AL.hasArg(OPT_B));
EXPECT_EQ(AL.getAllArgValues(OPT_B).size(), 1U);
EXPECT_EQ(AL.getAllArgValues(OPT_B)[0], "");
}