diff --git a/include/llvm/Option/ArgList.h b/include/llvm/Option/ArgList.h index 15d82d6da35..e29cf24661a 100644 --- a/include/llvm/Option/ArgList.h +++ b/include/llvm/Option/ArgList.h @@ -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 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 { diff --git a/include/llvm/Option/OptTable.h b/include/llvm/Option/OptTable.h index 62bfb0852f8..96f51cf3317 100644 --- a/include/llvm/Option/OptTable.h +++ b/include/llvm/Option/OptTable.h @@ -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 Args, - unsigned &MissingArgIndex, unsigned &MissingArgCount, - unsigned FlagsToInclude = 0, - unsigned FlagsToExclude = 0) const; + InputArgList ParseArgs(ArrayRef Args, unsigned &MissingArgIndex, + unsigned &MissingArgCount, unsigned FlagsToInclude = 0, + unsigned FlagsToExclude = 0) const; /// \brief Render the help text for an option table. /// diff --git a/lib/LibDriver/LibDriver.cpp b/lib/LibDriver/LibDriver.cpp index e441fe84c39..9f2b12f75b8 100644 --- a/lib/LibDriver/LibDriver.cpp +++ b/lib/LibDriver/LibDriver.cpp @@ -113,27 +113,27 @@ int llvm::libDriverMain(llvm::ArrayRef ArgsArr) { LibOptTable Table; unsigned MissingIndex; unsigned MissingCount; - std::unique_ptr 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 SearchPaths = getSearchPaths(Args.get(), Saver); + std::vector SearchPaths = getSearchPaths(&Args, Saver); std::vector Members; - for (auto *Arg : Args->filtered(OPT_INPUT)) { + for (auto *Arg : Args.filtered(OPT_INPUT)) { Optional 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 ArgsArr) { llvm::sys::path::filename(Arg->getValue())); } - std::pair Result = llvm::writeArchive( - getOutputPath(Args.get()), Members, /*WriteSymtab=*/true); + std::pair Result = + llvm::writeArchive(getOutputPath(&Args), Members, /*WriteSymtab=*/true); if (Result.second) { if (Result.first.empty()) Result.first = ArgsArr[0]; diff --git a/lib/Option/OptTable.cpp b/lib/Option/OptTable.cpp index cec1717454f..2160b15c1cf 100644 --- a/lib/Option/OptTable.cpp +++ b/lib/Option/OptTable.cpp @@ -247,12 +247,12 @@ Arg *OptTable::ParseOneArg(const ArgList &Args, unsigned &Index, return new Arg(getOption(TheUnknownOptionID), Str, Index++, Str); } -InputArgList *OptTable::ParseArgs(ArrayRef ArgArr, - unsigned &MissingArgIndex, - unsigned &MissingArgCount, - unsigned FlagsToInclude, - unsigned FlagsToExclude) const { - InputArgList *Args = new InputArgList(ArgArr.begin(), ArgArr.end()); +InputArgList OptTable::ParseArgs(ArrayRef 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 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 ArgArr, break; } - Args->append(A); + Args.append(A); } - return Args; + return std::move(Args); } static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) { diff --git a/unittests/Option/OptionParsingTest.cpp b/unittests/Option/OptionParsingTest.cpp index 671cf6de0cb..55cf8a95f35 100644 --- a/unittests/Option/OptionParsingTest.cpp +++ b/unittests/Option/OptionParsingTest.cpp @@ -67,26 +67,26 @@ const char *Args[] = { TEST(Option, OptionParsing) { TestOptTable T; unsigned MAI, MAC; - std::unique_ptr 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 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 Es = AL.getAllArgValues(OPT_E); EXPECT_EQ(Es[0], "apple"); EXPECT_EQ(Es[1], "bloom"); - EXPECT_EQ(AL->getLastArgValue(OPT_F), "42"); - std::vector Gs = AL->getAllArgValues(OPT_G); + EXPECT_EQ(AL.getLastArgValue(OPT_F), "42"); + std::vector 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 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 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 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 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 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 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 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 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], ""); }