diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -30,21 +30,28 @@ class Multilib { public: using flags_list = std::vector; + using print_options_callback = flags_list (*)(const flags_list &); private: std::string GCCSuffix; std::string OSSuffix; std::string IncludeSuffix; flags_list Flags; - int Priority; + flags_list PrintOptions; + print_options_callback PrintOptionsCallback; public: /// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the /// sysroot string so they must either be empty or begin with a '/' character. /// This is enforced with an assert in the constructor. + /// PrintOptions and PrintOptionsCallback are mutually exclusive. + /// If PrintOptionsCallback is non-null then it will be used when needed for + /// -print-multi-lib functionality. Otherwise -print-multi-lib will use the + /// content from PrintOptions. Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {}, - StringRef IncludeSuffix = {}, int Priority = 0, - const flags_list &Flags = flags_list()); + StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(), + const flags_list &PrintOptions = flags_list(), + print_options_callback PrintOptionsCallback = nullptr); /// Get the detected GCC installation path suffix for the multi-arch /// target variant. Always starts with a '/', unless empty @@ -58,13 +65,14 @@ /// empty const std::string &includeSuffix() const { return IncludeSuffix; } - /// Get the flags that indicate or contraindicate this multilib's use - /// All elements begin with either '+' or '-' + /// Get the set of flags that indicate this multilib's use. + /// Flags are arbitrary strings although typically they will look similar to + /// command line options. A multilib is considered compatible if its flags are + /// a subset of the flags derived from the Clang command line options. const flags_list &flags() const { return Flags; } - /// Returns the multilib priority. When more than one multilib matches flags, - /// the one with the highest priority is selected, with 0 being the default. - int priority() const { return Priority; } + /// Returns the options that should be used for clang -print-multi-lib + flags_list getPrintOptions() const; LLVM_DUMP_METHOD void dump() const; /// print summary of the Multilib @@ -108,6 +116,9 @@ const_iterator begin() const { return Multilibs.begin(); } const_iterator end() const { return Multilibs.end(); } + /// Select compatible variants + multilib_list select(const Multilib::flags_list &Flags) const; + /// Pick the best multilib in the set, \returns false if none are compatible bool select(const Multilib::flags_list &Flags, Multilib &M) const; @@ -129,13 +140,6 @@ } const IncludeDirsFunc &filePathsCallback() const { return FilePathsCallback; } - -private: - /// Apply the filter to Multilibs and return the subset that remains - static multilib_list filterCopy(FilterCallback F, const multilib_list &Ms); - - /// Apply the filter to the multilib_list, removing those that don't match - static void filterInPlace(FilterCallback F, multilib_list &Ms); }; raw_ostream &operator<<(raw_ostream &OS, const MultilibSet &MS); diff --git a/clang/include/clang/Driver/MultilibBuilder.h b/clang/include/clang/Driver/MultilibBuilder.h --- a/clang/include/clang/Driver/MultilibBuilder.h +++ b/clang/include/clang/Driver/MultilibBuilder.h @@ -28,11 +28,10 @@ std::string OSSuffix; std::string IncludeSuffix; flags_list Flags; - int Priority; public: MultilibBuilder(StringRef GCCSuffix, StringRef OSSuffix, - StringRef IncludeSuffix, int Priority = 0); + StringRef IncludeSuffix); /// Initializes GCCSuffix, OSSuffix & IncludeSuffix to the same value. MultilibBuilder(StringRef Suffix = {}); @@ -75,10 +74,6 @@ const flags_list &flags() const { return Flags; } flags_list &flags() { return Flags; } - /// Returns the multilib priority. When more than one multilib matches flags, - /// the one with the highest priority is selected, with 0 being the default. - int priority() const { return Priority; } - /// Add a flag to the flags list /// \p Flag must be a flag accepted by the driver with its leading '-' /// removed, diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp --- a/clang/lib/Driver/Multilib.cpp +++ b/clang/lib/Driver/Multilib.cpp @@ -26,16 +26,28 @@ using namespace llvm::sys; Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix, - StringRef IncludeSuffix, int Priority, - const flags_list &Flags) + StringRef IncludeSuffix, const flags_list &Flags, + const flags_list &PrintOptions, + print_options_callback PrintOptionsCallback) : GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix), - Flags(Flags), Priority(Priority) { + Flags(Flags), PrintOptions(PrintOptions), + PrintOptionsCallback(PrintOptionsCallback) { assert(GCCSuffix.empty() || (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1)); assert(OSSuffix.empty() || (StringRef(OSSuffix).front() == '/' && OSSuffix.size() > 1)); assert(IncludeSuffix.empty() || (StringRef(IncludeSuffix).front() == '/' && IncludeSuffix.size() > 1)); + // PrintOptionsCallback should not be provided at the same time as a non-empty + // PrintOptions. They are mutually exlcusive. + assert(PrintOptions.empty() || PrintOptionsCallback == nullptr); +} + +Multilib::flags_list Multilib::getPrintOptions() const { + if (PrintOptionsCallback) { + return (*PrintOptionsCallback)(Flags); + } + return PrintOptions; } LLVM_DUMP_METHOD void Multilib::dump() const { @@ -45,14 +57,11 @@ void Multilib::print(raw_ostream &OS) const { if (GCCSuffix.empty()) OS << "."; - else { + else OS << StringRef(GCCSuffix).drop_front(); - } OS << ";"; - for (StringRef Flag : Flags) { - if (Flag.front() == '+') - OS << "@" << Flag.substr(1); - } + for (StringRef Option : getPrintOptions()) + OS << "@" << Option.substr(1); } bool Multilib::operator==(const Multilib &Other) const { @@ -84,56 +93,36 @@ } MultilibSet &MultilibSet::FilterOut(FilterCallback F) { - filterInPlace(F, Multilibs); + llvm::erase_if(Multilibs, F); return *this; } void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); } -static bool isFlagEnabled(StringRef Flag) { - char Indicator = Flag.front(); - assert(Indicator == '+' || Indicator == '-'); - return Indicator == '+'; +MultilibSet::multilib_list +MultilibSet::select(const Multilib::flags_list &Flags) const { + llvm::StringSet<> FlagSet; + for (const auto &Flag : Flags) + FlagSet.insert(Flag); + + multilib_list Result; + llvm::copy_if(Multilibs, std::back_inserter(Result), + [&FlagSet](const Multilib &M) { + for (const std::string &F : M.flags()) + if (!FlagSet.contains(F)) + return false; + return true; + }); + return Result; } -bool MultilibSet::select(const Multilib::flags_list &Flags, Multilib &M) const { - llvm::StringMap FlagSet; - - // Stuff all of the flags into the FlagSet such that a true mappend indicates - // the flag was enabled, and a false mappend indicates the flag was disabled. - for (StringRef Flag : Flags) - FlagSet[Flag.substr(1)] = isFlagEnabled(Flag); - - multilib_list Filtered = filterCopy([&FlagSet](const Multilib &M) { - for (StringRef Flag : M.flags()) { - llvm::StringMap::const_iterator SI = FlagSet.find(Flag.substr(1)); - if (SI != FlagSet.end()) - if (SI->getValue() != isFlagEnabled(Flag)) - return true; - } - return false; - }, Multilibs); - - if (Filtered.empty()) +bool MultilibSet::select(const Multilib::flags_list &Flags, + Multilib &Selected) const { + multilib_list Result = select(Flags); + if (Result.empty()) return false; - if (Filtered.size() == 1) { - M = Filtered[0]; - return true; - } - - // Sort multilibs by priority and select the one with the highest priority. - llvm::sort(Filtered, [](const Multilib &a, const Multilib &b) -> bool { - return a.priority() > b.priority(); - }); - - if (Filtered[0].priority() > Filtered[1].priority()) { - M = Filtered[0]; - return true; - } - - // TODO: We should consider returning llvm::Error rather than aborting. - assert(false && "More than one multilib with the same priority"); - return false; + Selected = Result.back(); + return true; } LLVM_DUMP_METHOD void MultilibSet::dump() const { @@ -145,17 +134,6 @@ OS << M << "\n"; } -MultilibSet::multilib_list MultilibSet::filterCopy(FilterCallback F, - const multilib_list &Ms) { - multilib_list Copy(Ms); - filterInPlace(F, Copy); - return Copy; -} - -void MultilibSet::filterInPlace(FilterCallback F, multilib_list &Ms) { - llvm::erase_if(Ms, F); -} - raw_ostream &clang::driver::operator<<(raw_ostream &OS, const MultilibSet &MS) { MS.print(OS); return OS; diff --git a/clang/lib/Driver/MultilibBuilder.cpp b/clang/lib/Driver/MultilibBuilder.cpp --- a/clang/lib/Driver/MultilibBuilder.cpp +++ b/clang/lib/Driver/MultilibBuilder.cpp @@ -41,9 +41,8 @@ } } -MultilibBuilder::MultilibBuilder(StringRef GCC, StringRef OS, StringRef Include, - int Priority) - : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include), Priority(Priority) { +MultilibBuilder::MultilibBuilder(StringRef GCC, StringRef OS, StringRef Include) + : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include) { normalizePathSegment(GCCSuffix); normalizePathSegment(OSSuffix); normalizePathSegment(IncludeSuffix); @@ -86,8 +85,23 @@ return true; } +static Multilib::flags_list getPrintOptions(const Multilib::flags_list &Flags) { + // Derive print options from flags. + // In general, flags in the Multilib class are not required to be valid + // command line options, but for the MultilibBuilder class flags are expected + // to form valid command line options when their first character is replaced + // with '-'. + Multilib::flags_list PrintOptions; + for (StringRef Flag : Flags) { + if (Flag.front() == '+') + PrintOptions.push_back(("-" + Flag.substr(1)).str()); + } + return PrintOptions; +} + Multilib MultilibBuilder::makeMultilib() const { - return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Priority, Flags); + return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Flags, + Multilib::flags_list(), &::getPrintOptions); } MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) { diff --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp b/clang/lib/Driver/ToolChains/Fuchsia.cpp --- a/clang/lib/Driver/ToolChains/Fuchsia.cpp +++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -263,33 +263,33 @@ Multilibs.push_back(Multilib()); // Use the noexcept variant with -fno-exceptions to avoid the extra overhead. - Multilibs.push_back(MultilibBuilder("noexcept", {}, {}, 1) + Multilibs.push_back(MultilibBuilder("noexcept", {}, {}) .flag("-fexceptions") .flag("+fno-exceptions") .makeMultilib()); // ASan has higher priority because we always want the instrumentated version. - Multilibs.push_back(MultilibBuilder("asan", {}, {}, 2) + Multilibs.push_back(MultilibBuilder("asan", {}, {}) .flag("+fsanitize=address") .makeMultilib()); // Use the asan+noexcept variant with ASan and -fno-exceptions. - Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {}, 3) + Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {}) .flag("+fsanitize=address") .flag("-fexceptions") .flag("+fno-exceptions") .makeMultilib()); // HWASan has higher priority because we always want the instrumentated // version. - Multilibs.push_back(MultilibBuilder("hwasan", {}, {}, 4) + Multilibs.push_back(MultilibBuilder("hwasan", {}, {}) .flag("+fsanitize=hwaddress") .makeMultilib()); // Use the hwasan+noexcept variant with HWASan and -fno-exceptions. - Multilibs.push_back(MultilibBuilder("hwasan+noexcept", {}, {}, 5) + Multilibs.push_back(MultilibBuilder("hwasan+noexcept", {}, {}) .flag("+fsanitize=hwaddress") .flag("-fexceptions") .flag("+fno-exceptions") .makeMultilib()); // Use Itanium C++ ABI for the compat multilib. - Multilibs.push_back(MultilibBuilder("compat", {}, {}, 6) + Multilibs.push_back(MultilibBuilder("compat", {}, {}) .flag("+fc++-abi=itanium") .makeMultilib()); @@ -299,9 +299,10 @@ }); Multilib::flags_list Flags; - addMultilibFlag( - Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true), - "fexceptions", Flags); + bool Exceptions = + Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true); + addMultilibFlag(Exceptions, "fexceptions", Flags); + addMultilibFlag(!Exceptions, "fno-exceptions", Flags); addMultilibFlag(getSanitizerArgs(Args).needsAsanRt(), "fsanitize=address", Flags); addMultilibFlag(getSanitizerArgs(Args).needsHwasanRt(), "fsanitize=hwaddress", diff --git a/clang/unittests/Driver/MultilibBuilderTest.cpp b/clang/unittests/Driver/MultilibBuilderTest.cpp --- a/clang/unittests/Driver/MultilibBuilderTest.cpp +++ b/clang/unittests/Driver/MultilibBuilderTest.cpp @@ -207,3 +207,14 @@ << "Selection picked " << Selection << " which was not expected "; } } + +TEST(MultilibBuilderTest, PrintOptions) { + Multilib M = MultilibBuilder() + .flag("+x") + .flag("-y") + .flag("+a") + .flag("-b") + .flag("+c") + .makeMultilib(); + ASSERT_EQ(Multilib::flags_list({"-x", "-a", "-c"}), M.getPrintOptions()); +} diff --git a/clang/unittests/Driver/MultilibTest.cpp b/clang/unittests/Driver/MultilibTest.cpp --- a/clang/unittests/Driver/MultilibTest.cpp +++ b/clang/unittests/Driver/MultilibTest.cpp @@ -33,14 +33,14 @@ } TEST(MultilibTest, OpEqReflexivity3) { - Multilib M1({}, {}, {}, 0, {"+foo"}); - Multilib M2({}, {}, {}, 0, {"+foo"}); + Multilib M1({}, {}, {}, {"+foo"}); + Multilib M2({}, {}, {}, {"+foo"}); ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same"; } TEST(MultilibTest, OpEqInequivalence1) { - Multilib M1({}, {}, {}, 0, {"+foo"}); - Multilib M2({}, {}, {}, 0, {"-foo"}); + Multilib M1({}, {}, {}, {"+foo"}); + Multilib M2({}, {}, {}, {"-foo"}); ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same"; ASSERT_FALSE(M2 == M1) << "Multilibs with conflicting flags are not the same (commuted)"; @@ -48,7 +48,7 @@ TEST(MultilibTest, OpEqInequivalence2) { Multilib M1; - Multilib M2({}, {}, {}, 0, {"+foo"}); + Multilib M2({}, {}, {}, {"+foo"}); ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different"; } @@ -124,7 +124,7 @@ } TEST(MultilibTest, Construction3) { - Multilib M({}, {}, {}, 0, {"+f1", "+f2", "-f3"}); + Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"}); for (Multilib::flags_list::const_iterator I = M.flags().begin(), E = M.flags().end(); I != E; ++I) { @@ -149,8 +149,8 @@ TEST(MultilibTest, SetPriority) { MultilibSet MS({ - Multilib("/foo", {}, {}, 1, {"+foo"}), - Multilib("/bar", {}, {}, 2, {"+bar"}), + Multilib("/foo", {}, {}, {"+foo"}), + Multilib("/bar", {}, {}, {"+bar"}), }); Multilib::flags_list Flags1 = {"+foo", "-bar"}; Multilib Selection1; @@ -166,3 +166,24 @@ ASSERT_TRUE(Selection2.gccSuffix() == "/bar") << "Selection picked " << Selection2 << " which was not expected"; } + +TEST(MultilibTest, SelectMultiple) { + MultilibSet MS({ + Multilib("/a", {}, {}, {"x"}), + Multilib("/b", {}, {}, {"y"}), + }); + std::vector Selection; + + Selection = MS.select({"x"}); + ASSERT_EQ(1u, Selection.size()); + EXPECT_EQ("/a", Selection[0].gccSuffix()); + + Selection = MS.select({"y"}); + ASSERT_EQ(1u, Selection.size()); + EXPECT_EQ("/b", Selection[0].gccSuffix()); + + Selection = MS.select({"y", "x"}); + ASSERT_EQ(2u, Selection.size()); + EXPECT_EQ("/a", Selection[0].gccSuffix()); + EXPECT_EQ("/b", Selection[1].gccSuffix()); +}