Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 31 additions & 25 deletions crates/engine/src/parser/oracle_effect/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4959,10 +4959,20 @@ const MULTI_TARGET_VERBS: &[&str] = &[
"exile", "tap", "untap", "goad", "detain", "return", "destroy", "choose",
];

pub(super) const BOUNDED_TARGET_PHRASES: &[(&str, usize, usize)] = &[
("one or two targets", 1, 2),
("one, two, or three targets", 1, 3),
];
/// CR 115.1d + CR 601.2d: The bounded target-cardinality lists. The stem carries
/// the bare `" or "` that enumerates a target COUNT, never a disjunction of two
/// clauses; each list carries its `(min, max)` count bounds. This is the single
/// authority for the vocabulary — the complete set measured against the full
/// pool (`AtomicCards.json`, 34,632 cards). Every consumer composes the trailing
/// noun it needs off the stem (`" targets"`, `" target "`, `" target"`) rather
/// than re-spelling the list, so a future cardinality ("one, two, three, or
/// four …") is added here once. Consumers: `strip_bounded_targets_placeholder`,
/// `strip_bounded_target_prefix`, `subject::…each-of`, `oracle_target`'s
/// bare-count target arm, and the binary-choice splitter's divide/distribute
/// bail (which keys on this cardinality axis — shared by CR 601.2d's "damage or
/// counters" halves — rather than on a distribution verb).
pub(crate) const BOUNDED_TARGET_CARDINALITIES: &[(&str, usize, usize)] =
&[("one or two", 1, 2), ("one, two, or three", 1, 3)];

/// CR 115.1d + CR 601.2c: Strip exact target-count prefix before a targeted
/// phrase. "two target creatures" and "X target creatures" both set the exact
Expand Down Expand Up @@ -5005,8 +5015,10 @@ fn parse_exact_target_count_expr(input: &str) -> OracleResult<'_, QuantityExpr>
/// Returns the unconsumed remainder and a bounded `MultiTargetSpec` with min ≥ 1.
fn strip_bounded_targets_placeholder(text: &str) -> Option<(&str, MultiTargetSpec)> {
let lower = text.to_ascii_lowercase();
for &(phrase, min, max) in BOUNDED_TARGET_PHRASES {
if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>(phrase).parse(lower.as_str()) {
for &(stem, min, max) in BOUNDED_TARGET_CARDINALITIES {
if let Ok((rest, _)) =
(tag::<_, _, OracleError<'_>>(stem), tag(" targets")).parse(lower.as_str())
{
let consumed = lower.len() - rest.len();
return Some((
text[consumed..].trim_start(),
Expand All @@ -5022,11 +5034,10 @@ fn strip_bounded_targets_placeholder(text: &str) -> Option<(&str, MultiTargetSpe
/// players").
fn strip_bounded_target_prefix(text: &str) -> Option<(&str, MultiTargetSpec)> {
let lower = text.to_ascii_lowercase();
for (prefix, min, max) in [
("one or two target ", 1usize, 2usize),
("one, two, or three target ", 1, 3),
] {
if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>(prefix).parse(lower.as_str()) {
for &(stem, min, max) in BOUNDED_TARGET_CARDINALITIES {
if let Ok((rest, _)) =
(tag::<_, _, OracleError<'_>>(stem), tag(" target ")).parse(lower.as_str())
{
let consumed = lower.len() - rest.len();
return Some((
text[consumed..].trim_start(),
Expand Down Expand Up @@ -5765,20 +5776,15 @@ fn multi_target_for_distribute_among(distribution_amount: &QuantityExpr) -> Mult
/// Single authority for the "is this a distribution clause?" membership test —
/// extend here, never inline at a call site.
///
/// Two callers must agree on this set. `try_parse_distribute_damage` bounds its
/// Pattern-B quantity slice with it, and `try_parse_choose_one_of_inline` bails out
/// of the generic binary-choice splitter for any clause containing one, because the
/// target-count list embeds a bare " or " ("among one, two, or three targets") that
/// is a cardinality list, not a disjunction of clauses. If the two sets were allowed
/// to drift, a newly supported distribution phrasing would parse correctly here yet
/// still be split there — re-entering the clause cascade and overflowing the stack.
///
/// SCOPE: damage templating only. CR 601.2d also covers dividing *counters*
/// ("such as damage or counters"), and the counter templating ("Distribute three
/// +1/+1 counters among one, two, or three target creatures") carries the same bare
/// " or " and is NOT guarded here — it is a pre-existing hazard on `main`, tracked
/// separately. The durable fix keys the bail on the target-cardinality list rather
/// than on the distribution verb, which covers both halves of the rule at once.
/// One caller: `try_parse_distribute_damage` bounds its Pattern-B quantity slice
/// with this set. (The binary-choice splitter `try_parse_choose_one_of_inline`
/// formerly also consulted it to bail, but that coupling — and the set-drift
/// hazard it warned about — is gone: the splitter now keys its bail on the
/// target-cardinality list `BOUNDED_TARGET_CARDINALITIES`, the axis CR 601.2d
/// shares across both its "damage or counters" halves. That axis guards the
/// counter-distribution templating ("Distribute three +1/+1 counters among one,
/// two, or three target creatures") this damage-verb set never could, so the two
/// no longer need to agree.)
pub(super) const DISTRIBUTION_KEYWORDS: [&str; 2] =
["divided as you choose among", "divided evenly"];

Expand Down
52 changes: 24 additions & 28 deletions crates/engine/src/parser/oracle_effect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4973,34 +4973,29 @@ fn try_parse_choose_one_of_inline(
return None;
}

// CR 601.2d: The divided/distributed-damage target-count quantifier
// ("divided as you choose among one, two, or three targets") embeds a bare
// " or " inside the cardinality list "one, two, or three", NOT a disjunction
// of two imperative clauses. Splitting there hands the orphan tail
// ("three targets") back through the full effect→subject→static→imperative
// recognizer cascade as a trial parse; because that tail is not a standalone
// effect, the cascade re-enters this clause parser several levels deep, and
// the parser's large per-frame AST locals overflow small (~1-2 MB) thread
// stacks. The dedicated `try_parse_distribute_damage` handler
// (`oracle_effect::lower`) owns this whole clause and reads the count list
// via `strip_distribute_among_target_quantifier`, so skip the generic binary
// splitter for any distribution clause and let that handler claim it.
// CR 601.2d: A divided/distributed effect's target-count quantifier
// ("divided as you choose among one, two, or three targets"; "distribute
// three +1/+1 counters among one, two, or three target creatures") embeds a
// bare " or " inside the cardinality list "one, two, or three" (or "one or
// two"), NOT a disjunction of two imperative clauses. Splitting there hands
// the orphan tail ("three target creatures …") back through the full
// effect→subject→static→imperative recognizer cascade as a trial parse;
// because that tail is not a standalone effect, the cascade re-enters this
// clause parser several levels deep, and the parser's large per-frame AST
// locals overflow small (~1-2 MB) thread stacks.
//
// The keyword set is owned by `lower::DISTRIBUTION_KEYWORDS` — the same set the
// handler bounds its quantity slice with. Both must agree: a phrasing the handler
// learned but this guard did not would parse there and still split here.
//
// KNOWN GAP (pre-existing on `main`, not introduced here): this keys on the
// distribution *verb*, but the hazard is the target-cardinality list. CR 601.2d
// covers dividing "damage or counters", and the counter templating ("Distribute
// three +1/+1 counters among one, two, or three target creatures") carries the
// same bare " or " and is still split. The durable fix keys the bail on the
// cardinality list (`lower::BOUNDED_TARGET_PHRASES`), covering both halves at
// once; and the underlying cause is that a failed trial parse re-enters the
// clause cascade with no depth bound. Both are tracked as follow-ups.
if lower::DISTRIBUTION_KEYWORDS
// CR 601.2d divides an effect "(such as damage OR counters)", so the bail
// keys on the target-CARDINALITY list — the axis shared by both halves —
// rather than on a distribution verb (which names only the damage half and
// left the ~32 counter-distribution cards, e.g. Ajani, Mentor of Heroes;
// Biogenic Upgrade; Abzan Charm, still splitting). `BOUNDED_TARGET_CARDINALITIES`
// is the single authority for the vocabulary (the same lists the target-count
// layer strips via `strip_bounded_target_prefix`); compose the `" target"`
// stem-suffix it shares with both the "… targets" and "… target <noun>"
// forms, so the divide/distribute handlers claim these clauses whole.
if lower::BOUNDED_TARGET_CARDINALITIES
.iter()
.any(|needle| nom_primitives::scan_contains(tp.lower, needle))
.any(|&(stem, _, _)| nom_primitives::scan_contains(tp.lower, &format!("{stem} target")))
{
return None;
}
Expand Down Expand Up @@ -7748,9 +7743,10 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect
}

// CR 601.2d: "deal N damage divided as you choose among [targets]" /
// "distributed among" / "divided evenly" → DealDamage with distribute.
// "divided evenly" → DealDamage with distribute. (The "distributed among"
// needle was removed: it matches 0 of 34,632 cards — no Magic card uses that
// templating — so it only guarded phrasing that does not exist.)
if scan_contains_phrase(&lower, "divided as you choose among")
|| scan_contains_phrase(&lower, "distributed among")
|| scan_contains_phrase(&lower, "divided evenly")
{
if let Some(clause) = try_parse_distribute_damage(&lower, text) {
Expand Down
6 changes: 3 additions & 3 deletions crates/engine/src/parser/oracle_effect/subject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use super::animation::{
has_in_addition_to_other_types, parse_animation_spec,
};
use super::imperative;
use super::lower::BOUNDED_TARGET_PHRASES;
use super::lower::BOUNDED_TARGET_CARDINALITIES;
use super::{resolve_it_pronoun, ParseContext};
use crate::parser::oracle_ir::ast::*;
use crate::types::ability::{
Expand Down Expand Up @@ -2180,8 +2180,8 @@ pub(super) fn parse_subject_application(
}
// CR 115.1d: "each of one or two targets" — bounded multi-target selection
// where the effect applies to each chosen target (Prismari Charm).
for &(phrase, min, max) in BOUNDED_TARGET_PHRASES {
if tag::<_, _, OracleError<'_>>(phrase)
for &(stem, min, max) in BOUNDED_TARGET_CARDINALITIES {
if (tag::<_, _, OracleError<'_>>(stem), tag(" targets"))
.parse(remainder)
.is_ok()
{
Expand Down
45 changes: 45 additions & 0 deletions crates/engine/src/parser/oracle_effect/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6892,6 +6892,51 @@ fn blazing_salvo_unless_have_deal_damage() {
}
}

/// CR 601.2d (#5613): a divided/distributed cardinality-list clause's bare " or "
/// ("… among one, two, or three target creatures") is a target-COUNT enumerator.
/// Before this fix the binary-choice splitter split there and trial-parsed the
/// orphan tail back through the effect→subject→static→imperative cascade. That
/// trial parse FAILS and is discarded, so the final parse OUTPUT is identical on
/// `main` (the pool coverage-parse-diff shows zero card changes) — but it
/// re-enters the clause parser several frames deep. The regression is stack
/// DEPTH, not output, so this asserts it on a bounded-stack thread.
///
/// Peak stack for this parse (full `parse_oracle_text` pipeline, measured):
/// ~3.5 MiB on `main` (the wasted trial parse) vs ~1.4 MiB with the guard. A
/// 2.5 MiB stack thus survives here (~1.1 MiB headroom) and overflows on `main`
/// (~1 MiB margin): reintroducing the wasted trial parse overflows this thread's
/// guard page and aborts the test binary — so the test *fails when the fix is
/// reverted* (AI-CONTRIBUTOR.md §5(i)). The in-thread asserts additionally pin
/// the parse OUTPUT (`PutCounter` + a captured distribution).
#[test]
fn distribute_cardinality_clause_parses_within_a_bounded_stack() {
let handle = std::thread::Builder::new()
.stack_size(2_621_440) // 2.5 MiB — mid-gap between the ~1.4 / ~3.5 MiB floors
.spawn(|| {
let parsed = parse_oracle_text(
"Distribute three +1/+1 counters among one, two, or three target creatures.",
"Bounded Stack Test",
&[],
&["Sorcery".to_string()],
&[],
);
let def = parsed.abilities.first().expect("expected a spell ability");
assert!(
matches!(*def.effect, Effect::PutCounter { .. }),
"counter distribution must parse whole as PutCounter, got {:?}",
def.effect
);
assert!(
def.distribute.is_some(),
"distribution unit must be captured"
);
})
.expect("spawn bounded-stack parse thread");
handle
.join()
.expect("parsing the cardinality clause must not overflow a 2.5 MiB stack");
}

#[test]
fn lava_blister_its_controller_unless_have_deal_damage() {
let def = parse_effect_chain(
Expand Down
18 changes: 12 additions & 6 deletions crates/engine/src/parser/oracle_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,12 +628,18 @@ pub fn parse_target_with_syntax<'a>(
}
}

for phrase in [
"one, two, or three targets",
"one or two targets",
"any number of targets",
"targets",
] {
// CR 115.1d: bare bounded-count target arms ("one or two targets", "one,
// two, or three targets") share the single `BOUNDED_TARGET_CARDINALITIES`
// authority (composing the plural noun off the stem); the unbounded forms
// ("any number of targets", bare "targets") stay inline.
for &(stem, _, _) in crate::parser::oracle_effect::lower::BOUNDED_TARGET_CARDINALITIES {
if let Ok((rest, _)) =
(tag::<_, _, OracleError<'_>>(stem), tag(" targets")).parse(lower.as_str())
{
return (TargetFilter::Any, &text[lower.len() - rest.len()..], syntax);
}
}
for phrase in ["any number of targets", "targets"] {
if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>(phrase).parse(lower.as_str()) {
return (TargetFilter::Any, &text[lower.len() - rest.len()..], syntax);
}
Expand Down
Loading