revert: multiple epilogues (#68)

Author: julio4

src/pipelines/exec/navi.rs

@@ -25,7 +25,7 @@ use {
 pub(crate) struct StepPath(SmallVec<[usize; 8]>);
 
 const PROLOGUE_INDEX: usize = usize::MIN;
-const EPILOGUE_START_INDEX: usize = usize::MAX - 1024; // Reserve space for epilogue steps
+const EPILOGUE_INDEX: usize = usize::MAX;
 const STEP0_INDEX: usize = PROLOGUE_INDEX + 1;
 
 /// Public API
@@ -86,7 +86,7 @@ impl StepPath {
 
 	/// Returns `true` if the path is pointing to an epilogue of a pipeline.
 	pub(crate) fn is_epilogue(&self) -> bool {
-		self.leaf() >= EPILOGUE_START_INDEX
+		self.leaf() == EPILOGUE_INDEX
 	}
 }
 
@@ -232,15 +232,9 @@ impl StepPath {
 		Self(smallvec![PROLOGUE_INDEX])
 	}
 
-	/// Returns a leaf step path pointing at the first epilogue step.
+	/// Returns a leaf step path pointing at the epilogue step.
 	pub(in crate::pipelines) fn epilogue() -> Self {
-		Self::epilogue_step(0)
-	}
-
-	/// Returns a leaf step path pointing at a specific epilogue step with the
-	/// given index.
-	pub(in crate::pipelines) fn epilogue_step(epilogue_index: usize) -> Self {
-		Self(smallvec![epilogue_index + EPILOGUE_START_INDEX])
+		Self(smallvec![EPILOGUE_INDEX])
 	}
 
 	/// Returns a new step path that points to the first non-prologue step.
@@ -281,18 +275,14 @@ impl core::fmt::Display for StepPath {
 		if let Some(&first) = iter.next() {
 			match first {
 				PROLOGUE_INDEX => write!(f, "p"),
-				idx if idx >= EPILOGUE_START_INDEX => {
-					write!(f, "e{}", idx - EPILOGUE_START_INDEX)
-				}
+				EPILOGUE_INDEX => write!(f, "e"),
 				index => write!(f, "{index}"),
 			}?;
 
 			for &index in iter {
 				match index {
 					PROLOGUE_INDEX => write!(f, "_p"),
-					idx if idx >= EPILOGUE_START_INDEX => {
-						write!(f, "_e{}", idx - EPILOGUE_START_INDEX)
-					}
+					EPILOGUE_INDEX => write!(f, "_e"),
 					index => write!(f, "_{index}"),
 				}?;
 			}
@@ -328,7 +318,7 @@ impl<'a, P: Platform> StepNavigator<'a, P> {
 	/// In pipelines with a prologue, this will point to the prologue step.
 	/// In pipelines without a prologue, this will point to the first step.
 	/// In pipelines with no steps, but with an epilogue, this will point to the
-	/// first epilogue step.
+	/// epilogue step.
 	///
 	/// If the first item in the pipeline is a nested pipeline, this will dig
 	/// deeper into the nested pipeline to find the first executable item.
@@ -346,10 +336,9 @@ impl<'a, P: Platform> StepNavigator<'a, P> {
 
 		// pipeline has no prologue
 		if pipeline.steps().is_empty() {
-			// If there are no steps, but there is an epilogue, return the first
-			// epilogue step.
-			if !pipeline.epilogue().is_empty() {
-				return Self(StepPath::epilogue(), vec![pipeline]).enter();
+			// If there are no steps, but there is an epilogue, return it.
+			if pipeline.epilogue().is_some() {
+				return Some(Self(StepPath::epilogue(), vec![pipeline]));
 			}
 
 			// this is an empty pipeline, there is nothing executable.
@@ -373,20 +362,11 @@ impl<'a, P: Platform> StepNavigator<'a, P> {
 		} else if self.is_epilogue() {
 			enclosing_pipeline
 				.epilogue()
-				.get(
-					step_index
-						.checked_sub(EPILOGUE_START_INDEX)
-						.expect("step index should be >= epilogue start index"),
-				)
 				.expect("Step path points to a non-existing epilogue")
 		} else {
 			let StepOrPipeline::Step(step) = enclosing_pipeline
 				.steps()
-				.get(
-					step_index
-						.checked_sub(STEP0_INDEX)
-						.expect("step index should be >= step0 index"),
-				)
+				.get(step_index - STEP0_INDEX)
 				.expect("Step path points to a non-existing step")
 			else {
 				unreachable!(
@@ -420,19 +400,7 @@ impl<'a, P: Platform> StepNavigator<'a, P> {
 	/// Returns `None` if there are no more steps to execute in the pipeline.
 	pub(crate) fn next_ok(self) -> Option<Self> {
 		if self.is_epilogue() {
-			// we are in an epilogue step, check if there are more epilogue steps
-			let enclosing_pipeline = self.pipeline();
-			let epilogue_index = self
-				.0
-				.leaf()
-				.checked_sub(EPILOGUE_START_INDEX)
-				.expect("invalid epilogue step index in the step path");
-
-			if epilogue_index + 1 < enclosing_pipeline.epilogue().len() {
-				// there are more epilogue steps, go to the next one
-				return Self(self.0.increment_leaf(), self.1.clone()).enter();
-			}
-			// this is the last epilogue step, we are done with this pipeline
+			// the loop is over.
 			return self.next_in_parent();
 		}
 
@@ -551,56 +519,45 @@ impl<P: Platform> StepNavigator<'_, P> {
 			"StepNavigator should always have at least one enclosing pipeline",
 		);
 
-		if path.is_prologue() {
+		if path.is_prologue() || path.is_epilogue() {
 			assert!(
-				enclosing_pipeline.prologue().is_some(),
-				"path is prologue, but the enclosing pipeline has none",
+				enclosing_pipeline.prologue().is_some()
+					|| enclosing_pipeline.epilogue().is_some(),
+				"path is prologue or epilogue, but enclosing pipeline has none",
 			);
-			// if we are in a prologue, we can just return ourselves.
+			// if we are in a prologue or epilogue, we can just return ourselves.
 			return Some(Self(path, ancestors));
 		}
 
-		if path.is_epilogue() {
-			assert!(
-				!enclosing_pipeline.epilogue().is_empty(),
-				"path is epilogue, but the enclosing pipeline has none",
-			);
-			Some(Self(path, ancestors))
-		} else {
-			let step_index = path
-				.leaf()
-				.checked_sub(STEP0_INDEX)
-				.expect("path is not prologue or epilogue");
-
-			match enclosing_pipeline.steps().get(step_index)? {
-				StepOrPipeline::Step(_) => {
-					// if we are pointing at a step, we can just return ourselves.
-					Some(Self(path, ancestors))
-				}
-				StepOrPipeline::Pipeline(_, nested) => {
-					// if we are pointing at a pipeline, we need to dig into its
-					// entrypoint.
-					Some(StepNavigator(path, ancestors).join(Self::entrypoint(nested)?))
-				}
+		let step_index = path
+			.leaf()
+			.checked_sub(STEP0_INDEX)
+			.expect("path is not prologue or epilogue");
+
+		match enclosing_pipeline.steps().get(step_index)? {
+			StepOrPipeline::Step(_) => {
+				// if we are pointing at a step, we can just return ourselves.
+				Some(Self(path, ancestors))
+			}
+			StepOrPipeline::Pipeline(_, nested) => {
+				// if we are pointing at a pipeline, we need to dig into its
+				// entrypoint.
+				Some(StepNavigator(path, ancestors).join(Self::entrypoint(nested)?))
 			}
 		}
 	}
 
 	/// Finds the next step to run when a loop is finished.
 	///
-	/// The next step could be either the first epilogue step of the current
+	/// The next step could be either the epilogue step of the current
 	/// pipeline or the next step in the parent pipeline.
 	fn after_loop(self) -> Option<Self> {
-		if self.pipeline().epilogue().is_empty() {
-			self.next_in_parent()
+		if self.pipeline().epilogue().is_some() {
+			// we've reached the epilogue of this pipeline, regardless of the
+			// looping behavior, we should go to the next step in the parent pipeline.
+			Some(Self(self.0.replace_leaf(EPILOGUE_INDEX), self.1.clone()))
 		} else {
-			// we've reached the epilogue of this pipeline, go to the first epilogue
-			// step
-			Some(Self(
-				self.0.replace_leaf(EPILOGUE_START_INDEX),
-				self.1.clone(),
-			))
-			.and_then(|nav| nav.enter())
+			self.next_in_parent()
 		}
 	}
 
@@ -667,9 +624,7 @@ mod test {
 
 	fake_step!(Epilogue1);
 	fake_step!(Epilogue2);
-
-	fake_step!(EpilogueStep1);
-	fake_step!(EpilogueStep2);
+	fake_step!(Epilogue3);
 
 	fake_step!(Prologue1);
 	fake_step!(Prologue2);
@@ -711,8 +666,7 @@ mod test {
 						Once,
 						(StepI, StepII, StepIII)
 							.with_name("nested1.2")
-							.with_epilogue(EpilogueStep1)
-							.with_epilogue(EpilogueStep2),
+							.with_epilogue(Epilogue3),
 					)
 			})
 			.with_step(Step4)
@@ -728,10 +682,6 @@ mod test {
 			self.concat(StepPath::epilogue())
 		}
 
-		fn append_epilogue_step(self, step_index: usize) -> Self {
-			self.concat(StepPath::epilogue_step(step_index))
-		}
-
 		fn append_step(self, step_index: usize) -> Self {
 			self.concat(StepPath::step(step_index))
 		}
@@ -755,7 +705,7 @@ mod test {
 			}};
 		}
 
-		// one step
+		// one step with no prologue
 		assert_entrypoint!(
 			Pipeline::<Ethereum>::default().with_step(Step1),
 			StepPath::step0(),
@@ -772,7 +722,7 @@ mod test {
 			vec!["one"]
 		);
 
-		// no steps, but with a one-step epilogue
+		// no steps, but with epilogue
 		assert_entrypoint!(
 			Pipeline::<Ethereum>::named("one").with_epilogue(Epilogue1),
 			StepPath::epilogue(),
@@ -797,7 +747,7 @@ mod test {
 			vec!["one", "two"]
 		);
 
-		// one nested pipeline with no steps, but with a one-step epilogue
+		// one nested pipeline with no steps, but with an epilogue
 		assert_entrypoint!(
 			Pipeline::<Ethereum>::named("one")
 				.with_pipeline(Loop, Pipeline::named("two").with_epilogue(Epilogue1)),
@@ -935,13 +885,7 @@ mod test {
 		assert_eq!(cursor.0, StepPath::step(2).append_step(3).append_step(1));
 
 		let cursor = cursor.next_break().unwrap();
-		assert_eq!(
-			cursor.0,
-			StepPath::step(2).append_step(3).append_epilogue_step(0)
-		);
-
-		let cursor = cursor.next_ok().unwrap();
-		StepPath::step(2).append_step(3).append_epilogue_step(1);
+		assert_eq!(cursor.0, StepPath::step(2).append_step(3).append_epilogue());
 
 		let cursor = cursor.next_ok().unwrap();
 		assert_eq!(cursor.0, StepPath::step(2).append_step(0));

src/pipelines/iter.rs

@@ -11,7 +11,7 @@ struct Frame<'a, P: Platform> {
 	path: StepPath,
 	next_ix: usize,
 	yielded_prologue: bool,
-	epilogue_ix: usize,
+	yielded_epilogue: bool,
 }
 
 impl<'a, P: Platform> StepPathIter<'a, P> {
@@ -22,7 +22,7 @@ impl<'a, P: Platform> StepPathIter<'a, P> {
 				next_ix: 0,
 				path: StepPath::empty(),
 				yielded_prologue: false,
-				epilogue_ix: 0,
+				yielded_epilogue: false,
 			}],
 		}
 	}
@@ -56,18 +56,17 @@ impl<P: Platform> Iterator for StepPathIter<'_, P> {
 							path: next_path,
 							next_ix: 0,
 							yielded_prologue: false,
-							epilogue_ix: 0,
+							yielded_epilogue: false,
 						});
 						continue;
 					}
 				}
 			}
 
-			// Walk epilogue steps/pipelines; descend into nested pipelines.
-			if frame.epilogue_ix < frame.pipeline.epilogue.len() {
-				let ix = frame.epilogue_ix;
-				frame.epilogue_ix += 1;
-				return Some(frame.path.clone().concat(StepPath::epilogue_step(ix)));
+			// Yield epilogue once, if present.
+			if !frame.yielded_epilogue && frame.pipeline.epilogue.is_some() {
+				frame.yielded_epilogue = true;
+				return Some(frame.path.clone().concat(StepPath::epilogue()));
 			}
 
 			// Done with this frame; pop and continue with parent.
@@ -103,7 +102,7 @@ mod tests {
 			.with_step(Step3)
 			.with_epilogue(EpilogueOne);
 
-		let expected = vec!["p", "1", "2", "3", "e0"];
+		let expected = vec!["p", "1", "2", "3", "e"];
 		let actual = pipeline
 			.iter_steps()
 			.map(|step| step.to_string())
@@ -126,7 +125,7 @@ mod tests {
 			.with_epilogue(EpilogueOne);
 
 		let expected = vec![
-			"p", "1_p", "1_1", "1_2_1", "1_2_2", "1_2_3", "1_2_e0", "1_3", "e0",
+			"p", "1_p", "1_1", "1_2_1", "1_2_2", "1_2_3", "1_2_e", "1_3", "e",
 		];
 
 		let actual = pipeline