From d50eb9cde4def38384c03d60a7265de30f96fad6 Mon Sep 17 00:00:00 2001 From: ras0219 <533828+ras0219@users.noreply.github.com> Date: Mon, 6 Jul 2020 11:03:38 -0700 Subject: [PATCH] [vcpkg] Remove use of std::variant and std::visit to fix VS2015. (#12242) Fixes #12220 Co-authored-by: Robert Schumacher --- include/vcpkg/base/stringview.h | 15 +- src/vcpkg/base/stringview.cpp | 17 +- src/vcpkg/platform-expression.cpp | 282 +++++++++++++----------------- src/vcpkg/vcpkgcmdarguments.cpp | 6 +- 4 files changed, 140 insertions(+), 180 deletions(-) diff --git a/include/vcpkg/base/stringview.h b/include/vcpkg/base/stringview.h index aee70d697ea832..6a5503e1c3e29d 100644 --- a/include/vcpkg/base/stringview.h +++ b/include/vcpkg/base/stringview.h @@ -43,20 +43,7 @@ namespace vcpkg std::string to_string() const; void to_string(std::string& out) const; - constexpr StringView substr(size_t pos, size_t count = std::numeric_limits::max()) const - { - if (pos > m_size) - { - return StringView(); - } - - if (count > m_size - pos) - { - return StringView(m_ptr + pos, m_size - pos); - } - - return StringView(m_ptr + pos, count); - } + StringView substr(size_t pos, size_t count = std::numeric_limits::max()) const; constexpr char byte_at_index(size_t pos) const { return m_ptr[pos]; } diff --git a/src/vcpkg/base/stringview.cpp b/src/vcpkg/base/stringview.cpp index 326b0219ce35de..34798219898f0b 100644 --- a/src/vcpkg/base/stringview.cpp +++ b/src/vcpkg/base/stringview.cpp @@ -71,11 +71,26 @@ namespace vcpkg return result.front(); } - StringView::StringView(const std::string& s) : m_ptr(s.data()), m_size(s.size()) { } + StringView::StringView(const std::string& s) : m_ptr(s.data()), m_size(s.size()) {} std::string StringView::to_string() const { return std::string(m_ptr, m_size); } void StringView::to_string(std::string& s) const { s.append(m_ptr, m_size); } + StringView StringView::substr(size_t pos, size_t count) const + { + if (pos > m_size) + { + return StringView(); + } + + if (count > m_size - pos) + { + return StringView(m_ptr + pos, m_size - pos); + } + + return StringView(m_ptr + pos, count); + } + bool operator==(StringView lhs, StringView rhs) noexcept { return lhs.size() == rhs.size() && memcmp(lhs.data(), rhs.data(), lhs.size()) == 0; diff --git a/src/vcpkg/platform-expression.cpp b/src/vcpkg/platform-expression.cpp index 35cfd4cbc1ab86..9b83372e1aef75 100644 --- a/src/vcpkg/platform-expression.cpp +++ b/src/vcpkg/platform-expression.cpp @@ -3,10 +3,10 @@ #include #include #include +#include #include #include -#include #include namespace vcpkg::PlatformExpression @@ -61,68 +61,43 @@ namespace vcpkg::PlatformExpression namespace detail { - struct ExprIdentifier + enum class ExprKind { - std::string identifier; - }; - struct ExprNot - { - std::unique_ptr expr; - }; - struct ExprAnd - { - std::vector exprs; - }; - struct ExprOr - { - std::vector exprs; + identifier, + op_not, + op_and, + op_or }; struct ExprImpl { - std::variant underlying; + ExprImpl(ExprKind k, std::string i, std::vector> es) + : kind(k), identifier(std::move(i)), exprs(std::move(es)) + { + } - explicit ExprImpl(ExprIdentifier e) : underlying(std::move(e)) { } - explicit ExprImpl(ExprNot e) : underlying(std::move(e)) { } - explicit ExprImpl(ExprAnd e) : underlying(std::move(e)) { } - explicit ExprImpl(ExprOr e) : underlying(std::move(e)) { } + ExprImpl(ExprKind k, std::string i) : kind(k), identifier(std::move(i)) {} + ExprImpl(ExprKind k, std::unique_ptr a) : kind(k) { exprs.push_back(std::move(a)); } + ExprImpl(ExprKind k, std::vector> es) : kind(k), exprs(std::move(es)) {} + + ExprKind kind; + std::string identifier; + std::vector> exprs; - ExprImpl clone() const + std::unique_ptr clone() const { - struct Visitor - { - ExprImpl operator()(const ExprIdentifier& e) { return ExprImpl(e); } - ExprImpl operator()(const ExprNot& e) - { - return ExprImpl(ExprNot{std::make_unique(e.expr->clone())}); - } - ExprImpl operator()(const ExprAnd& e) - { - ExprAnd res; - for (const auto& expr : e.exprs) - { - res.exprs.push_back(expr.clone()); - } - return ExprImpl(std::move(res)); - } - ExprImpl operator()(const ExprOr& e) - { - ExprOr res; - for (const auto& expr : e.exprs) - { - res.exprs.push_back(expr.clone()); - } - return ExprImpl(std::move(res)); - } - }; - return std::visit(Visitor{}, underlying); + return std::make_unique( + ExprImpl{kind, identifier, Util::fmap(exprs, [](auto&& p) { return p->clone(); })}); } }; class ExpressionParser : public Parse::ParserBase { public: - ExpressionParser(StringView str, MultipleBinaryOperators multiple_binary_operators) : Parse::ParserBase(str, "CONTROL"), multiple_binary_operators(multiple_binary_operators) { } + ExpressionParser(StringView str, MultipleBinaryOperators multiple_binary_operators) + : Parse::ParserBase(str, "CONTROL"), multiple_binary_operators(multiple_binary_operators) + { + } MultipleBinaryOperators multiple_binary_operators; @@ -142,7 +117,7 @@ namespace vcpkg::PlatformExpression add_error("invalid logic expression, unexpected character"); } - return Expr(std::make_unique(std::move(res))); + return Expr(std::move(res)); } private: @@ -153,16 +128,13 @@ namespace vcpkg::PlatformExpression // // | - static bool is_identifier_char(char32_t ch) - { - return is_lower_alpha(ch) || is_ascii_digit(ch); - } + static bool is_identifier_char(char32_t ch) { return is_lower_alpha(ch) || is_ascii_digit(ch); } // : // // // - ExprImpl expr() + std::unique_ptr expr() { auto result = expr_not(); @@ -170,15 +142,11 @@ namespace vcpkg::PlatformExpression { case '|': { - ExprOr e; - e.exprs.push_back(std::move(result)); - return expr_binary<'|', '&'>(std::move(e)); + return expr_binary<'|', '&'>(std::make_unique(ExprKind::op_or, std::move(result))); } case '&': { - ExprAnd e; - e.exprs.push_back(std::move(result)); - return expr_binary<'&', '|'>(std::move(e)); + return expr_binary<'&', '|'>(std::make_unique(ExprKind::op_and, std::move(result))); } default: return result; } @@ -187,7 +155,7 @@ namespace vcpkg::PlatformExpression // : // ( ) // - ExprImpl expr_simple() + std::unique_ptr expr_simple() { if (cur() == '(') { @@ -209,7 +177,7 @@ namespace vcpkg::PlatformExpression // : // A lowercase alpha-numeric string - ExprImpl expr_identifier() + std::unique_ptr expr_identifier() { std::string name = match_zero_or_more(is_identifier_char).to_string(); @@ -219,26 +187,26 @@ namespace vcpkg::PlatformExpression } skip_whitespace(); - return ExprImpl{ExprIdentifier{name}}; + return std::make_unique(ExprKind::identifier, std::move(name)); } // : // // ! - ExprImpl expr_not() + std::unique_ptr expr_not() { if (cur() == '!') { next(); skip_whitespace(); - return ExprImpl(ExprNot{std::make_unique(expr_simple())}); + return std::make_unique(ExprKind::op_not, expr_simple()); } return expr_simple(); } - template - ExprImpl expr_binary(ExprKind&& seed) + template + std::unique_ptr expr_binary(std::unique_ptr&& seed) { do { @@ -249,7 +217,7 @@ namespace vcpkg::PlatformExpression } while (allow_multiple_binary_operators() && cur() == oper); skip_whitespace(); - seed.exprs.push_back(expr_not()); + seed->exprs.push_back(expr_not()); } while (cur() == oper); if (cur() == other) @@ -258,7 +226,7 @@ namespace vcpkg::PlatformExpression } skip_whitespace(); - return ExprImpl(std::move(seed)); + return std::move(seed); } }; } @@ -273,21 +241,14 @@ namespace vcpkg::PlatformExpression { if (other.underlying_) { - underlying_ = std::make_unique(other.underlying_->clone()); + this->underlying_ = other.underlying_->clone(); } } Expr& Expr::operator=(const Expr& other) { if (other.underlying_) { - if (this->underlying_) - { - *this->underlying_ = other.underlying_->clone(); - } - else - { - this->underlying_ = std::make_unique(other.underlying_->clone()); - } + this->underlying_ = other.underlying_->clone(); } else { @@ -297,36 +258,28 @@ namespace vcpkg::PlatformExpression return *this; } - Expr::Expr(std::unique_ptr&& e) : underlying_(std::move(e)) { } + Expr::Expr(std::unique_ptr&& e) : underlying_(std::move(e)) {} Expr::~Expr() = default; Expr Expr::Identifier(StringView id) { - return Expr(std::make_unique(ExprImpl{ExprIdentifier{id.to_string()}})); + return Expr(std::make_unique(ExprKind::identifier, id.to_string())); } - Expr Expr::Not(Expr&& e) { return Expr(std::make_unique(ExprImpl{ExprNot{std::move(e.underlying_)}})); } + Expr Expr::Not(Expr&& e) { return Expr(std::make_unique(ExprKind::op_not, std::move(e.underlying_))); } Expr Expr::And(std::vector&& exprs) { - std::vector impls; - for (auto& e : exprs) - { - impls.push_back(std::move(*e.underlying_)); - } - return Expr(std::make_unique(ExprAnd{std::move(impls)})); + return Expr(std::make_unique( + ExprKind::op_and, Util::fmap(exprs, [](Expr& expr) { return std::move(expr.underlying_); }))); } Expr Expr::Or(std::vector&& exprs) { - std::vector impls; - for (auto& e : exprs) - { - impls.push_back(std::move(*e.underlying_)); - } - return Expr(std::make_unique(ExprOr{std::move(impls)})); + return Expr(std::make_unique( + ExprKind::op_or, Util::fmap(exprs, [](Expr& expr) { return std::move(expr.underlying_); }))); } bool Expr::evaluate(const Context& context) const { - if (!underlying_) + if (!this->underlying_) { return true; // empty expression is always true } @@ -369,89 +322,96 @@ namespace vcpkg::PlatformExpression return iter->second == value; } - bool visit(const ExprImpl& e) const { return std::visit(*this, e.underlying); } - - bool operator()(const ExprIdentifier& expr) const + bool visit(const ExprImpl& expr) const { - if (!override_ctxt.empty()) + if (expr.kind == ExprKind::identifier) { - auto override_id = override_ctxt.find(expr.identifier); - if (override_id != override_ctxt.end()) + if (!override_ctxt.empty()) { - return override_id->second; + auto override_id = override_ctxt.find(expr.identifier); + if (override_id != override_ctxt.end()) + { + return override_id->second; + } + // Fall through to use the cmake logic if the id does not have an override } - // Fall through to use the cmake logic if the id does not have an override - } - auto id = string2identifier(expr.identifier); - switch (id) + auto id = string2identifier(expr.identifier); + switch (id) + { + case Identifier::invalid: + // Point out in the diagnostic that they should add to the override list because that is + // what most users should do, however it is also valid to update the built in identifiers to + // recognize the name. + System::printf( + System::Color::error, + "Error: Unrecognized identifer name %s. Add to override list in triplet file.\n", + expr.identifier); + return false; + case Identifier::x64: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "x64"); + case Identifier::x86: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "x86"); + case Identifier::arm: + // For backwards compatability arm is also true for arm64. + // This is because it previously was only checking for a substring. + return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm") || + true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm64"); + case Identifier::arm64: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm64"); + case Identifier::windows: + return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "") || + true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"); + case Identifier::linux: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Linux"); + case Identifier::osx: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Darwin"); + case Identifier::uwp: + return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"); + case Identifier::android: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Android"); + case Identifier::emscripten: + return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Emscripten"); + case Identifier::wasm32: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "wasm32"); + case Identifier::static_link: + return true_if_exists_and_equal("VCPKG_LIBRARY_LINKAGE", "static"); + default: + Checks::exit_with_message( + VCPKG_LINE_INFO, + "vcpkg bug: string2identifier returned a value that we don't recognize: %d\n", + static_cast(id)); + } + } + else if (expr.kind == ExprKind::op_not) { - case Identifier::invalid: - // Point out in the diagnostic that they should add to the override list because that is what - // most users should do, however it is also valid to update the built in identifiers to - // recognize the name. - System::printf(System::Color::error, - "Error: Unrecognized identifer name %s. Add to override list in triplet file.\n", - expr.identifier); - return false; - case Identifier::x64: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "x64"); - case Identifier::x86: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "x86"); - case Identifier::arm: - // For backwards compatability arm is also true for arm64. - // This is because it previously was only checking for a substring. - return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm") || - true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm64"); - case Identifier::arm64: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "arm64"); - case Identifier::windows: - return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "") || - true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"); - case Identifier::linux: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Linux"); - case Identifier::osx: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Darwin"); - case Identifier::uwp: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"); - case Identifier::android: return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Android"); - case Identifier::emscripten: - return true_if_exists_and_equal("VCPKG_CMAKE_SYSTEM_NAME", "Emscripten"); - case Identifier::wasm32: return true_if_exists_and_equal("VCPKG_TARGET_ARCHITECTURE", "wasm32"); - case Identifier::static_link: return true_if_exists_and_equal("VCPKG_LIBRARY_LINKAGE", "static"); - default: - Checks::exit_with_message( - VCPKG_LINE_INFO, - "vcpkg bug: string2identifier returned a value that we don't recognize: %d\n", - static_cast(id)); + return !visit(*expr.exprs.at(0)); } - } - - bool operator()(const ExprNot& expr) const { - bool res = visit(*expr.expr); - return !res; - } + else if (expr.kind == ExprKind::op_and) + { + bool valid = true; - bool operator()(const ExprAnd& expr) const - { - bool valid = true; + // we want to print errors in all expressions, so we check all of the expressions all the time + for (const auto& e : expr.exprs) + { + valid &= visit(*e); + } - // we want to print errors in all expressions, so we check all of the expressions all the time - for (const auto& e : expr.exprs) - { - valid &= visit(e); + return valid; } + else if (expr.kind == ExprKind::op_or) + { + bool valid = false; - return valid; - } + // we want to print errors in all expressions, so we check all of the expressions all the time + for (const auto& e : expr.exprs) + { + valid |= visit(*e); + } - bool operator()(const ExprOr& expr) const - { - bool valid = false; - // we want to print errors in all expressions, so we check all of the expressions all the time - for (const auto& e : expr.exprs) + return valid; + } + else { - valid |= visit(e); + Checks::unreachable(VCPKG_LINE_INFO); } - return valid; } }; - return Visitor{context, override_ctxt}.visit(*underlying_); + return Visitor{context, override_ctxt}.visit(*this->underlying_); } ExpectedS parse_platform_expression(StringView expression, MultipleBinaryOperators multiple_binary_operators) diff --git a/src/vcpkg/vcpkgcmdarguments.cpp b/src/vcpkg/vcpkgcmdarguments.cpp index 45db0c2e7c4fb7..934b375a3b8148 100644 --- a/src/vcpkg/vcpkgcmdarguments.cpp +++ b/src/vcpkg/vcpkgcmdarguments.cpp @@ -578,7 +578,7 @@ namespace vcpkg void VcpkgCmdArguments::append_common_options(HelpTableFormatter& table) { - constexpr static auto opt = [](StringView arg, StringView joiner, StringView value) { + static auto opt = [](StringView arg, StringView joiner, StringView value) { return Strings::format("--%s%s%s", arg, joiner, value); }; @@ -683,9 +683,7 @@ namespace vcpkg { StringView flag; bool enabled; - } flags[] = { - {BINARY_CACHING_FEATURE, binary_caching_enabled()} - }; + } flags[] = {{BINARY_CACHING_FEATURE, binary_caching_enabled()}}; for (const auto& flag : flags) {