Skip to content

Commit 2050de9

Browse files
committed
fix: run after_pass in rev extension order
Signed-off-by: Yuchen Liang <yuchenl3@andrew.cmu.edu>
1 parent f01fd62 commit 2050de9

1 file changed

Lines changed: 67 additions & 1 deletion

File tree

optd/core/src/rules/pass.rs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ impl PassManager {
6969
let before = root.clone();
7070
let after = pass.run(root, ctx)?;
7171

72-
for extension in self.extensions.iter() {
72+
for extension in self.extensions.iter().rev() {
7373
extension.after_pass(pass.name(), &before, &after, ctx)?;
7474
}
7575

@@ -202,4 +202,70 @@ mod tests {
202202
assert_eq!(extension.recorded(), vec!["before:noop", "after:noop"]);
203203
Ok(())
204204
}
205+
206+
#[test]
207+
fn pass_manager_unwinds_after_hooks_in_reverse_registration_order() -> Result<()> {
208+
let ctx = test_ctx_with_tables(&[("t1", 1)])?;
209+
let plan = ctx.logical_get(TableRef::bare("t1"), None)?.build();
210+
let events = Arc::new(Mutex::new(Vec::new()));
211+
212+
struct SharedRecordingExtension {
213+
name: &'static str,
214+
events: Arc<Mutex<Vec<String>>>,
215+
}
216+
217+
impl PassExtension for SharedRecordingExtension {
218+
fn before_pass(
219+
&self,
220+
pass_name: &'static str,
221+
_root: &Arc<Operator>,
222+
_ctx: &IRContext,
223+
) -> Result<()> {
224+
self.events
225+
.lock()
226+
.unwrap()
227+
.push(format!("before:{}:{pass_name}", self.name));
228+
Ok(())
229+
}
230+
231+
fn after_pass(
232+
&self,
233+
pass_name: &'static str,
234+
_before: &Arc<Operator>,
235+
_after: &Arc<Operator>,
236+
_ctx: &IRContext,
237+
) -> Result<()> {
238+
self.events
239+
.lock()
240+
.unwrap()
241+
.push(format!("after:{}:{pass_name}", self.name));
242+
Ok(())
243+
}
244+
}
245+
246+
let manager = PassManager::builder()
247+
.add_extension_arc(Arc::new(SharedRecordingExtension {
248+
name: "first",
249+
events: events.clone(),
250+
}))
251+
.add_extension_arc(Arc::new(SharedRecordingExtension {
252+
name: "second",
253+
events: events.clone(),
254+
}))
255+
.build();
256+
257+
let rewritten = manager.run(&NoopPass, plan.clone(), &ctx)?;
258+
259+
assert_eq!(rewritten, plan);
260+
assert_eq!(
261+
events.lock().unwrap().as_slice(),
262+
[
263+
"before:first:noop",
264+
"before:second:noop",
265+
"after:second:noop",
266+
"after:first:noop",
267+
]
268+
);
269+
Ok(())
270+
}
205271
}

0 commit comments

Comments
 (0)