test-features: Suggest cascade lines with support expressions

include/vcpkg/base/message-data.inc.h

@@ -2974,6 +2974,7 @@ DECLARE_MESSAGE(
     "",
     "{feature_spec} was unexpectedly a cascading failure because the following dependencies are unavailable:")
 DECLARE_MESSAGE(UnexpectedStateCascadePortNote, (), "", "consider changing this to =cascade instead")
+DECLARE_MESSAGE(UnexpectedStateCascadeSuggestLine, (), "", "consider adding the following line:")
 DECLARE_MESSAGE(UnexpectedSwitch,
                 (msg::option),
                 "Switch is a command line switch like --switch",

include/vcpkg/platform-expression.h

@@ -64,6 +64,10 @@ namespace vcpkg::PlatformExpression
 
         static const Expr always_true;
 
+        Expr& negate();
+
+        Expr& simplify();
+
     private:
         std::unique_ptr<detail::ExprImpl> underlying_;
     };

locales/messages.json

@@ -1550,6 +1550,7 @@
   "UnexpectedStateCascade": "{feature_spec} was unexpectedly a cascading failure because the following dependencies are unavailable:",
   "_UnexpectedStateCascade.comment": "An example of {feature_spec} is zlib[featurea,featureb].",
   "UnexpectedStateCascadePortNote": "consider changing this to =cascade instead",
+  "UnexpectedStateCascadeSuggestLine": "consider adding the following line:",
   "UnexpectedStateFailedCascade": "{feature_spec} build failed but was expected to be a cascaded failure",
   "_UnexpectedStateFailedCascade.comment": "An example of {feature_spec} is zlib[featurea,featureb].",
   "UnexpectedStateFailedNoteConsiderSkippingPort": "consider adding `{package_name}=fail`, or `{spec}=fail`, or equivalent skips",

src/vcpkg-test/platform-expression.cpp

@@ -570,3 +570,21 @@ TEST_CASE ("invalid alternate expressions", "[platform-expression]")
     CHECK_FALSE(parse_expr("not! windows"));
     CHECK_FALSE(parse_expr("notx64 windows"));
 }
+
+TEST_CASE ("negate expression", "[platform-expression]")
+{
+    auto m_expr = parse_expr("uwp & !xbox, (windows | osx)");
+    REQUIRE(m_expr);
+    m_expr.get()->negate();
+    to_string(*m_expr.get());
+
+    CHECK(to_string(*m_expr.get()) == "(!uwp | xbox) & (!windows & !osx)");
+}
+
+TEST_CASE ("simplify expression", "[platform-expression]")
+{
+    auto m_expr = parse_expr("(uwp & (xbox & (uwp & xbox))) , !windows | (uwp, !windows)");
+    REQUIRE(m_expr);
+    m_expr.get()->simplify();
+    CHECK(to_string(*m_expr.get()) == "(uwp & xbox), !windows, uwp");
+}

src/vcpkg/commands.test-features.cpp

@@ -176,7 +176,8 @@ namespace
                                             const FullPackageSpec& spec,
                                             const std::string* ci_feature_baseline_file_name,
                                             const CiFeatureBaselineEntry* baseline,
-                                            std::string&& cascade_reason)
+                                            std::string&& cascade_reason,
+                                            LocalizedString&& fix_msg = {})
     {
         auto outcome = expected_outcome(baseline, spec.features);
         switch (outcome.value)
@@ -192,6 +193,10 @@ namespace
             case CiFeatureBaselineOutcome::ConfigurationFail:
                 add_build_cascade_diagnostic(
                     diagnostics, spec, ci_feature_baseline_file_name, outcome.loc, std::move(cascade_reason));
+                if (!fix_msg.empty())
+                {
+                    diagnostics.push_back(DiagnosticLine{DiagKind::Note, std::move(fix_msg)});
+                }
                 break;
             case CiFeatureBaselineOutcome::PortMarkedFail:
             case CiFeatureBaselineOutcome::FeatureFail:
@@ -201,6 +206,10 @@ namespace
                                                      *ci_feature_baseline_file_name,
                                                      TextRowCol{outcome.loc.row, outcome.loc.column},
                                                      msg::format(msgUnexpectedStateCascadePortNote)});
+                if (!fix_msg.empty())
+                {
+                    diagnostics.push_back(DiagnosticLine{DiagKind::Note, std::move(fix_msg)});
+                }
                 break;
             case CiFeatureBaselineOutcome::PortMarkedCascade:
             case CiFeatureBaselineOutcome::FeatureCascade:
@@ -727,21 +736,50 @@ namespace vcpkg
             if (!install_plan.unsupported_features.empty())
             {
                 std::vector<std::string> out;
+                std::vector<PlatformExpression::Expr> exprs;
                 for (const auto& entry : install_plan.unsupported_features)
                 {
                     out.push_back(msg::format(msgOnlySupports,
                                               msg::feature_spec = entry.first,
                                               msg::supports_expression = to_string(entry.second))
                                       .extract_data());
+                    if (spec.kind != SpecToTestKind::Combined && ci_feature_baseline_file_name)
+                    {
+                        exprs.push_back(entry.second);
+                    }
+                }
+                LocalizedString fix_msg;
+                if (!exprs.empty())
+                {
+                    auto all_or = PlatformExpression::Expr::And(std::move(exprs)).negate().simplify();
+                    fix_msg = msg::format(msgUnexpectedStateCascadeSuggestLine).append_raw('\n');
+                    if (spec.kind == SpecToTestKind::Core)
+                    {
+                        fix_msg.append_raw(
+                            fmt::format("{}({}) = cascade", spec.package_spec.name(), to_string(all_or)));
+                    }
+                    else if (spec.kind == SpecToTestKind::Separate)
+                    {
+                        fix_msg.append_raw(fmt::format("{}[{}]({}) = cascade",
+                                                       spec.package_spec.name(),
+                                                       spec.separate_feature,
+                                                       to_string(all_or)));
+                    }
                 }
+
                 msg::print(msg::format(msgSkipTestingOfPort,
                                        msg::feature_spec = install_plan.install_actions.back().display_name(),
                                        msg::triplet = target_triplet)
                                .append_raw('\n')
                                .append_raw(Strings::join("\n", out))
                                .append_raw('\n'));
-                handle_cascade_feature_test_result(
-                    diagnostics, all_ports, spec, ci_feature_baseline_file_name, baseline, Strings::join(", ", out));
+                handle_cascade_feature_test_result(diagnostics,
+                                                   all_ports,
+                                                   spec,
+                                                   ci_feature_baseline_file_name,
+                                                   baseline,
+                                                   Strings::join(", ", out),
+                                                   std::move(fix_msg));
                 continue;
             }
 

src/vcpkg/platform-expression.cpp

@@ -118,6 +118,117 @@ namespace vcpkg::PlatformExpression
                 return std::make_unique<ExprImpl>(
                     ExprImpl{kind, identifier, Util::fmap(exprs, [](auto&& p) { return p->clone(); })});
             }
+
+            bool operator==(const ExprImpl& other) const
+            {
+                struct Impl
+                {
+                    bool operator()(const std::unique_ptr<detail::ExprImpl>& lhs,
+                                    const std::unique_ptr<detail::ExprImpl>& rhs) const
+                    {
+                        return (*this)(*lhs, *rhs);
+                    }
+                    bool operator()(const detail::ExprImpl& lhs, const detail::ExprImpl& rhs) const
+                    {
+                        if (lhs.kind != rhs.kind) return false;
+
+                        if (lhs.kind == ExprKind::identifier)
+                        {
+                            return lhs.identifier == rhs.identifier;
+                        }
+                        else
+                        {
+                            const auto& exprs_l = lhs.exprs;
+                            const auto& exprs_r = rhs.exprs;
+                            return std::equal(exprs_l.begin(), exprs_l.end(), exprs_r.begin(), exprs_r.end(), *this);
+                        }
+                    }
+                };
+                return Impl{}(*this, other);
+            }
+            bool operator!=(const ExprImpl& other) const { return !(*this == other); }
+
+            void negate()
+            {
+                switch (kind)
+                {
+                    case ExprKind::identifier:
+                    {
+                        exprs.push_back(std::make_unique<ExprImpl>(std::move(*this)));
+                        kind = ExprKind::op_not;
+                        break;
+                    }
+                    case ExprKind::op_not:
+                    {
+                        auto sub_expr = std::move(*exprs.at(0));
+                        *this = std::move(sub_expr);
+                        break;
+                    }
+                    case ExprKind::op_and:
+                    case ExprKind::op_or:
+                    case ExprKind::op_list:
+                    {
+                        kind = (kind == ExprKind::op_and ? ExprKind::op_or : ExprKind::op_and);
+                        for (auto& expr : exprs)
+                        {
+                            expr->negate();
+                        }
+                        break;
+                    }
+                    case ExprKind::op_empty:
+                    case ExprKind::op_invalid: break;
+                }
+            }
+
+            void simplify()
+            {
+                switch (kind)
+                {
+                    case ExprKind::op_not:
+                    {
+                        if (exprs.at(0)->kind == ExprKind::op_not)
+                        {
+                            auto sub_sub_expr = std::move(*exprs.at(0)->exprs.at(0));
+                            *this = std::move(sub_sub_expr);
+                            simplify();
+                        }
+                        break;
+                    }
+                    case ExprKind::op_and:
+                    case ExprKind::op_or:
+                    case ExprKind::op_list:
+                    {
+                        auto add_if_different = [this](std::unique_ptr<ExprImpl>&& new_expr) {
+                            if (Util::all_of(exprs, [&](auto& expr) { return *expr != *new_expr; }))
+                            {
+                                exprs.push_back(std::move(new_expr));
+                            }
+                        };
+                        auto old_exprs = std::move(exprs);
+                        for (auto&& expr : old_exprs)
+                        {
+                            expr->simplify();
+                            if ((kind == ExprKind::op_and && expr->kind == ExprKind::op_and) ||
+                                ((kind == ExprKind::op_or || kind == ExprKind::op_list) &&
+                                 (expr->kind == ExprKind::op_or || expr->kind == ExprKind::op_list)))
+                            {
+                                for (auto&& sub_expr : expr->exprs)
+                                {
+                                    add_if_different(std::move(sub_expr));
+                                }
+                            }
+                            else
+                            {
+                                add_if_different(std::move(expr));
+                            }
+                        }
+                        break;
+                    }
+                    case ExprKind::identifier:
+                    case ExprKind::op_empty:
+                    case ExprKind::op_invalid: break;
+                }
+            }
         };
 
         struct ExpressionParser : ParserBase
@@ -655,6 +766,18 @@ namespace vcpkg::PlatformExpression
         return Impl{}(underlying_);
     }
 
+    Expr& Expr::negate()
+    {
+        underlying_->negate();
+        return *this;
+    }
+
+    Expr& Expr::simplify()
+    {
+        underlying_->simplify();
+        return *this;
+    }
+
     ExpectedL<Expr> parse_platform_expression(StringView expression, MultipleBinaryOperators multiple_binary_operators)
     {
         ExpressionParser parser(expression, multiple_binary_operators);
@@ -670,30 +793,6 @@ namespace vcpkg::PlatformExpression
 
     bool structurally_equal(const Expr& lhs, const Expr& rhs)
     {
-        struct Impl
-        {
-            bool operator()(const std::unique_ptr<detail::ExprImpl>& lhs,
-                            const std::unique_ptr<detail::ExprImpl>& rhs) const
-            {
-                return (*this)(*lhs, *rhs);
-            }
-            bool operator()(const detail::ExprImpl& lhs, const detail::ExprImpl& rhs) const
-            {
-                if (lhs.kind != rhs.kind) return false;
-
-                if (lhs.kind == ExprKind::identifier)
-                {
-                    return lhs.identifier == rhs.identifier;
-                }
-                else
-                {
-                    const auto& exprs_l = lhs.exprs;
-                    const auto& exprs_r = rhs.exprs;
-                    return std::equal(exprs_l.begin(), exprs_l.end(), exprs_r.begin(), exprs_r.end(), *this);
-                }
-            }
-        };
-
         if (lhs.is_empty())
         {
             return rhs.is_empty();
@@ -702,7 +801,7 @@ namespace vcpkg::PlatformExpression
         {
             return false;
         }
-        return Impl{}(lhs.underlying_, rhs.underlying_);
+        return *lhs.underlying_ == *rhs.underlying_;
     }
 
     int compare(const Expr& lhs, const Expr& rhs)