Skip to content

Commit 2a90757

Browse files
committed
yeast: Make translation failures panic
We catch any panics in `Runner::run_once`, and turn them into `Err` results. This prevents them from affecting the extraction of other files.
1 parent 33dfa65 commit 2a90757

5 files changed

Lines changed: 100 additions & 106 deletions

File tree

‎shared/yeast-macros/src/lower.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,18 +85,18 @@ impl Rule {
8585
#(#guard_bindings)*
8686
let ast = __ast;
8787
let #ctx = __user_ctx;
88-
Ok(#guard)
88+
let __guard: bool = #guard;
89+
__guard
8990
}),
9091
Box::new(|__ast: &yeast::Ast, __output: &mut yeast::OutputAst, __captures: yeast::captures::Captures, __source_range: Option<yeast::Range>, __user_ctx: &mut _, __translator: yeast::TranslatorHandle<'_, _>| {
9192
let __skip: &[&str] = &[#(#raw_capture_names),*];
9293
let __translated_captures =
93-
__translator.translate_captures(&__captures, __ast, __output, __user_ctx, __skip)?;
94+
__translator.translate_captures(&__captures, __ast, __output, __user_ctx, __skip);
9495
#(#raw_bindings)*
9596
#(#translated_bindings)*
9697
let mut #ctx = yeast::build::BuildCtx::with_translator(__ast, __output, __source_range, __user_ctx, __translator);
9798
let __result: Vec<yeast::OutputId> = { #replacement };
98-
let __result = #ctx.finish_rule(__result)?;
99-
Ok(__result)
99+
#ctx.finish_rule(__result)
100100
}),
101101
)
102102
}

‎shared/yeast/src/build.rs‎

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::{
2828
/// When constructed by the framework (via the rule! macro), `BuildCtx` also
2929
/// carries a [`TranslatorHandle`] that the [`translate`] method delegates
3030
/// to. When constructed by hand (e.g. in tests), the translator is `None`
31-
/// and [`translate`] returns an error.
31+
/// and [`translate`] panics.
3232
pub struct BuildCtx<'a, C: 'a = ()> {
3333
ast: &'a Ast,
3434
pub output: &'a mut OutputAst,
@@ -44,10 +44,6 @@ pub struct BuildCtx<'a, C: 'a = ()> {
4444
/// Optional translator handle, populated when the context is built by
4545
/// the framework's rule driver. None when the context is built by hand.
4646
pub(crate) translator: Option<TranslatorHandle<'a, C>>,
47-
/// First failure encountered while auto-translating an input `Id` during
48-
/// template construction. Templates remain infallible expressions, so
49-
/// [`finish_rule`](Self::finish_rule) returns this error at the rule boundary.
50-
deferred_translation_error: Option<String>,
5147
/// Nodes built directly through this context without an explicit source
5248
/// range in either their content or constructor argument. Recursive
5349
/// translations use their own context and therefore do not contribute to
@@ -64,7 +60,6 @@ impl<'a, C> BuildCtx<'a, C> {
6460
source_range: None,
6561
user_ctx,
6662
translator: None,
67-
deferred_translation_error: None,
6863
created_nodes_without_source_range: BTreeSet::new(),
6964
}
7065
}
@@ -82,7 +77,6 @@ impl<'a, C> BuildCtx<'a, C> {
8277
source_range,
8378
user_ctx,
8479
translator: None,
85-
deferred_translation_error: None,
8680
created_nodes_without_source_range: BTreeSet::new(),
8781
}
8882
}
@@ -102,7 +96,6 @@ impl<'a, C> BuildCtx<'a, C> {
10296
source_range,
10397
user_ctx,
10498
translator: Some(translator),
105-
deferred_translation_error: None,
10699
created_nodes_without_source_range: BTreeSet::new(),
107100
}
108101
}
@@ -159,18 +152,15 @@ impl<'a, C> BuildCtx<'a, C> {
159152
/// Finish the current rule invocation by applying the matched source range
160153
/// to locally-created result roots.
161154
#[doc(hidden)]
162-
pub fn finish_rule(self, results: Vec<OutputId>) -> Result<Vec<OutputId>, String> {
163-
if let Some(error) = self.deferred_translation_error {
164-
return Err(error);
165-
}
155+
pub fn finish_rule(self, results: Vec<OutputId>) -> Vec<OutputId> {
166156
if let Some(source_range) = self.source_range {
167157
for &id in &results {
168158
if self.created_nodes_without_source_range.contains(&id) {
169159
self.output.extend_source_range(id, source_range);
170160
}
171161
}
172162
}
173-
Ok(results)
163+
results
174164
}
175165

176166
/// Assign an explicit source range to a newly-built result root.
@@ -300,40 +290,31 @@ impl<C: Clone> BuildCtx<'_, C> {
300290

301291
#[doc(hidden)]
302292
pub fn translate_into(&mut self, id: Id, out: &mut Vec<OutputId>) {
303-
if self.deferred_translation_error.is_some() {
304-
return;
305-
}
306-
match self.translate(id) {
307-
Ok(ids) => out.extend(ids),
308-
Err(error) => self.deferred_translation_error = Some(error),
309-
}
293+
out.extend(self.translate(id));
310294
}
311295

312296
/// Recursively translate every id in the given iterable via the
313297
/// framework's rule machinery and return the accumulated resulting node
314298
/// ids in order.
315299
///
316300
/// The single-`Id` case works too, because `Id: IntoIterator<Item
317-
/// = Id>` is a singleton iterator — so `ctx.translate(some_id)?`
301+
/// = Id>` is a singleton iterator — so `ctx.translate(some_id)`
318302
/// returns a `Vec<OutputId>` containing whatever `some_id` translated to.
319303
///
320-
/// Errors if this `BuildCtx` was constructed by hand (without a
304+
/// Panics if this `BuildCtx` was constructed by hand (without a
321305
/// translator handle) — for example, in unit tests that don't go
322306
/// through the rule driver.
323-
pub fn translate(
324-
&mut self,
325-
ids: impl IntoIterator<Item = Id>,
326-
) -> Result<Vec<OutputId>, String> {
307+
pub fn translate(&mut self, ids: impl IntoIterator<Item = Id>) -> Vec<OutputId> {
327308
let translator = self
328309
.translator
329310
.as_ref()
330-
.ok_or("translate() called on a BuildCtx without a translator handle")?;
311+
.expect("translate() called on a BuildCtx without a translator handle");
331312
let mut out = Vec::new();
332313
for id in ids {
333-
let translated = translator.translate(self.ast, self.output, self.user_ctx, id)?;
314+
let translated = translator.translate(self.ast, self.output, self.user_ctx, id);
334315
out.extend(translated);
335316
}
336-
Ok(out)
317+
out
337318
}
338319

339320
/// Run `f` with a temporary child [`BuildCtx`] whose `user_ctx` is
@@ -358,9 +339,9 @@ impl<C: Clone> BuildCtx<'_, C> {
358339
/// let val = ctx.scoped(|ctx| {
359340
/// ctx.reset();
360341
/// ctx.translate(val)
361-
/// })?;
342+
/// });
362343
/// // `ctx` here is untouched by the reset inside the closure.
363-
/// let other = ctx.translate(other_id)?;
344+
/// let other = ctx.translate(other_id);
364345
/// ```
365346
pub fn scoped<F, R>(&mut self, f: F) -> R
366347
where
@@ -373,15 +354,11 @@ impl<C: Clone> BuildCtx<'_, C> {
373354
source_range: self.source_range,
374355
user_ctx: &mut child_user_ctx,
375356
translator: self.translator,
376-
deferred_translation_error: None,
377357
created_nodes_without_source_range: BTreeSet::new(),
378358
};
379359
let result = f(&mut child);
380360
let created_nodes_without_source_range =
381361
std::mem::take(&mut child.created_nodes_without_source_range);
382-
if self.deferred_translation_error.is_none() {
383-
self.deferred_translation_error = child.deferred_translation_error.take();
384-
}
385362
drop(child);
386363
self.created_nodes_without_source_range
387364
.extend(created_nodes_without_source_range);

‎shared/yeast/src/captures.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,23 @@ impl Captures {
5454
self.captures.entry(key).or_default().push(id);
5555
}
5656

57-
pub(crate) fn try_translate_except<E>(
57+
pub(crate) fn translate_except(
5858
&self,
5959
skip: &[&str],
60-
mut f: impl FnMut(Id) -> Result<Vec<OutputId>, E>,
61-
) -> Result<TranslatedCaptures, E> {
60+
mut f: impl FnMut(Id) -> Vec<OutputId>,
61+
) -> TranslatedCaptures {
6262
let mut translated = TranslatedCaptures::new();
6363
for (name, ids) in &self.captures {
6464
if skip.contains(name) {
6565
continue;
6666
}
6767
for &id in ids {
68-
for translated_id in f(id)? {
68+
for translated_id in f(id) {
6969
translated.insert(name, translated_id);
7070
}
7171
}
7272
}
73-
Ok(translated)
73+
translated
7474
}
7575

7676
pub fn merge(&mut self, other: &Captures) {

‎shared/yeast/src/lib.rs‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,7 +1145,7 @@ impl<'a, C: Clone> TranslatorHandle<'a, C> {
11451145
output: &mut OutputAst,
11461146
user_ctx: &mut C,
11471147
id: Id,
1148-
) -> Result<Vec<OutputId>, String> {
1148+
) -> Vec<OutputId> {
11491149
apply_rules_inner(
11501150
self.index,
11511151
ast,
@@ -1163,10 +1163,10 @@ impl<'a, C: Clone> TranslatorHandle<'a, C> {
11631163
output: &mut OutputAst,
11641164
user_ctx: &mut C,
11651165
skip: &[&str],
1166-
) -> Result<TranslatedCaptures, String> {
1167-
captures.try_translate_except(skip, |id| {
1166+
) -> TranslatedCaptures {
1167+
captures.translate_except(skip, |id| {
11681168
if id == self.matched_root {
1169-
Err("cannot eagerly translate a capture of the matched rule root".to_string())
1169+
panic!("cannot eagerly translate a capture of the matched rule root")
11701170
} else {
11711171
self.translate(ast, output, user_ctx, id)
11721172
}
@@ -1178,7 +1178,7 @@ impl<'a, C: Clone> TranslatorHandle<'a, C> {
11781178
///
11791179
/// Takes the immutable input AST, mutable output AST, raw captured variables,
11801180
/// source range of the matched node, mutable user context, and a
1181-
/// [`TranslatorHandle`]. Returns output IDs or an error.
1181+
/// [`TranslatorHandle`]. Returns output IDs.
11821182
///
11831183
/// Transforms receive raw input captures. Macro-generated transforms eagerly
11841184
/// translate `@` captures and translate raw ids when they are returned or
@@ -1191,7 +1191,7 @@ pub type Transform<C = ()> = Box<
11911191
Option<Range>,
11921192
&mut C,
11931193
TranslatorHandle<'_, C>,
1194-
) -> Result<Vec<OutputId>, String>
1194+
) -> Vec<OutputId>
11951195
+ Send
11961196
+ Sync,
11971197
>;
@@ -1200,7 +1200,7 @@ pub type Transform<C = ()> = Box<
12001200
/// runs. Returning `false` makes the rule behave as if its query did not
12011201
/// match, so the driver continues to the next rule. Guards have read-only
12021202
/// access to the shared user context.
1203-
pub type Guard<C = ()> = Box<dyn Fn(&Ast, &Captures, &C) -> Result<bool, String> + Send + Sync>;
1203+
pub type Guard<C = ()> = Box<dyn Fn(&Ast, &Captures, &C) -> bool + Send + Sync>;
12041204

12051205
pub struct Rule<C = ()> {
12061206
query: QueryNode,
@@ -1238,21 +1238,21 @@ impl<C> Rule<C> {
12381238
/// Attempt to match this rule's query against `node`, returning the raw
12391239
/// captures on success. Does not evaluate the guard or invoke the
12401240
/// transform.
1241-
fn match_query(&self, ast: &Ast, node: Id) -> Result<Option<Captures>, String> {
1241+
fn match_query(&self, ast: &Ast, node: Id) -> Option<Captures> {
12421242
let mut captures = Captures::new();
1243-
if !self.query.do_match(ast, node, &mut captures)? {
1244-
return Ok(None);
1243+
if !self.query.do_match(ast, node, &mut captures) {
1244+
return None;
12451245
}
1246-
Ok(Some(captures))
1246+
Some(captures)
12471247
}
12481248

12491249
/// Evaluate this rule's guard against a successful query match. An
12501250
/// unguarded rule always succeeds.
1251-
fn guard_matches(&self, ast: &Ast, captures: &Captures, user_ctx: &C) -> Result<bool, String> {
1251+
fn guard_matches(&self, ast: &Ast, captures: &Captures, user_ctx: &C) -> bool {
12521252
if let Some(guard) = &self.guard {
12531253
guard(ast, captures, user_ctx)
12541254
} else {
1255-
Ok(true)
1255+
true
12561256
}
12571257
}
12581258

@@ -1266,7 +1266,7 @@ impl<C> Rule<C> {
12661266
node: Id,
12671267
user_ctx: &mut C,
12681268
translator: TranslatorHandle<'_, C>,
1269-
) -> Result<Vec<OutputId>, String> {
1269+
) -> Vec<OutputId> {
12701270
let source_range = ast.source_range_ignoring_fields(node, &self.ignored_location_fields);
12711271
(self.transform)(ast, output, captures, source_range, user_ctx, translator)
12721272
}
@@ -1306,14 +1306,14 @@ impl<'a, C> RuleIndex<'a, C> {
13061306

13071307
/// Apply the first matching rule to each visited node. Recursion proceeds
13081308
/// only through captured nodes (not through the input node's children
1309-
/// directly), and an error is returned if no rule matches a visited node.
1309+
/// directly), and panics if no rule matches a visited node.
13101310
fn apply_rules<C: Clone>(
13111311
rules: &[Rule<C>],
13121312
ast: &Ast,
13131313
output: &mut OutputAst,
13141314
user_ctx: &mut C,
13151315
id: Id,
1316-
) -> Result<Vec<OutputId>, String> {
1316+
) -> Vec<OutputId> {
13171317
let index = RuleIndex::new(rules);
13181318
apply_rules_inner(&index, ast, output, user_ctx, id, 0)
13191319
}
@@ -1325,22 +1325,22 @@ fn apply_rules_inner<C: Clone>(
13251325
user_ctx: &mut C,
13261326
id: Id,
13271327
rewrite_depth: usize,
1328-
) -> Result<Vec<OutputId>, String> {
1328+
) -> Vec<OutputId> {
13291329
if rewrite_depth > MAX_REWRITE_DEPTH {
1330-
return Err(format!(
1330+
panic!(
13311331
"Desugaring exceeded maximum rewrite depth ({MAX_REWRITE_DEPTH}). \
13321332
This likely indicates a non-terminating rule cycle."
1333-
));
1333+
);
13341334
}
13351335

13361336
let node_kind = ast.get_node(id).map(|n| n.kind_name()).unwrap_or("");
13371337

13381338
for rule in index.rules_for_kind(node_kind) {
1339-
let Some(captures) = rule.match_query(ast, id)? else {
1339+
let Some(captures) = rule.match_query(ast, id) else {
13401340
continue;
13411341
};
13421342

1343-
if !rule.guard_matches(ast, &captures, user_ctx)? {
1343+
if !rule.guard_matches(ast, &captures, user_ctx) {
13441344
continue;
13451345
}
13461346

@@ -1355,11 +1355,11 @@ fn apply_rules_inner<C: Clone>(
13551355
rewrite_depth,
13561356
matched_root: id,
13571357
};
1358-
let result = rule.run_transform(ast, output, captures, id, &mut local, translator)?;
1359-
return Ok(result);
1358+
let result = rule.run_transform(ast, output, captures, id, &mut local, translator);
1359+
return result;
13601360
}
13611361

1362-
Err(format!("no rule matched node of kind '{node_kind}'"))
1362+
panic!("no rule matched node of kind '{node_kind}'")
13631363
}
13641364

13651365
/// A named bundle of exhaustive rules for the one-shot translation pass.
@@ -1617,23 +1617,40 @@ impl<'a, C: Clone> Runner<'a, C> {
16171617
}
16181618

16191619
fn run_once(&self, ast: Ast, user_ctx: &mut C) -> Result<OutputAst, String> {
1620-
if self.phases.len() != 1 {
1621-
return Err("YEAST requires exactly one translation phase".to_string());
1622-
}
1620+
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
1621+
self.run_once_inner(ast, user_ctx)
1622+
}))
1623+
.map(Ok)
1624+
.unwrap_or_else(|payload| {
1625+
let message = if let Some(message) = payload.downcast_ref::<&str>() {
1626+
(*message).to_owned()
1627+
} else if let Some(message) = payload.downcast_ref::<String>() {
1628+
message.clone()
1629+
} else {
1630+
"unknown panic payload".to_owned()
1631+
};
1632+
Err(format!("YEAST translation panicked: {message}"))
1633+
})
1634+
}
1635+
1636+
fn run_once_inner(&self, ast: Ast, user_ctx: &mut C) -> OutputAst {
1637+
assert_eq!(
1638+
self.phases.len(),
1639+
1,
1640+
"YEAST requires exactly one translation phase"
1641+
);
16231642
let phase = &self.phases[0];
16241643
let mut output = OutputAst::with_schema(self.schema.clone());
1625-
let res = apply_rules(&phase.rules, &ast, &mut output, user_ctx, ast.get_root())
1626-
.map_err(|e| format!("Phase `{}`: {e}", phase.name))?;
1627-
if res.len() != 1 {
1628-
return Err(format!(
1629-
"Phase `{}`: expected exactly one result node, got {}",
1630-
phase.name,
1631-
res.len()
1632-
));
1633-
}
1644+
let res = apply_rules(&phase.rules, &ast, &mut output, user_ctx, ast.get_root());
1645+
assert_eq!(
1646+
res.len(),
1647+
1,
1648+
"expected exactly one result node, got {}",
1649+
res.len()
1650+
);
16341651
output.set_root(res[0]);
16351652
output.set_source(ast.into_source());
1636-
Ok(output)
1653+
output
16371654
}
16381655
}
16391656

0 commit comments

Comments
 (0)