add comments explaining where post-mono const eval errors abort compilation

RalfJung · RalfJung · commit d765fb8fafba · 2024-03-10T14:39:26.000+01:00
diff --git a/compiler/rustc_codegen_cranelift/src/constant.rs b/compiler/rustc_codegen_cranelift/src/constant.rs
@@ -71,7 +71,7 @@ pub(crate) fn eval_mir_constant<'tcx>(
     // This cannot fail because we checked all required_consts in advance.
     let val = cv
         .eval(fx.tcx, ty::ParamEnv::reveal_all(), Some(constant.span))
-        .expect("erroneous constant not captured by required_consts");
+        .expect("erroneous constant missed by mono item collection");
     (val, cv.ty())
 }
 
diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs
@@ -21,11 +21,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     }
 
     pub fn eval_mir_constant(&self, constant: &mir::ConstOperand<'tcx>) -> mir::ConstValue<'tcx> {
-        // `MirUsedCollector` visited all constants before codegen began, so if we got here there
-        // can be no more constants that fail to evaluate.
+        // `MirUsedCollector` visited all required_consts before codegen began, so if we got here
+        // there can be no more constants that fail to evaluate.
         self.monomorphize(constant.const_)
             .eval(self.cx.tcx(), ty::ParamEnv::reveal_all(), Some(constant.span))
-            .expect("erroneous constant not captured by required_consts")
+            .expect("erroneous constant missed by mono item collection")
     }
 
     /// This is a convenience helper for `simd_shuffle_indices`. It has the precondition
diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs
@@ -211,7 +211,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
 
     // It may seem like we should iterate over `required_consts` to ensure they all successfully
     // evaluate; however, the `MirUsedCollector` already did that during the collection phase of
-    // monomorphization so we don't have to do it again.
+    // monomorphization, and if there is an error during collection then codegen never starts -- so
+    // we don't have to do it again.
 
     fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);
 
diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs
@@ -814,13 +814,16 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
         self.super_rvalue(rvalue, location);
     }
 
-    /// This does not walk the constant, as it has been handled entirely here and trying
-    /// to walk it would attempt to evaluate the `ty::Const` inside, which doesn't necessarily
-    /// work, as some constants cannot be represented in the type system.
+    /// This does not walk the MIR of the constant as that is not needed for codegen, all we need is
+    /// to ensure that the constant evaluates successfully and walk the result.
     #[instrument(skip(self), level = "debug")]
     fn visit_constant(&mut self, constant: &mir::ConstOperand<'tcx>, location: Location) {
         let const_ = self.monomorphize(constant.const_);
         let param_env = ty::ParamEnv::reveal_all();
+        // Evaluate the constant. This makes const eval failure a collection-time error (rather than
+        // a codegen-time error). rustc stops after collection if there was an error, so this
+        // ensures codegen never has to worry about failing consts.
+        // (codegen relies on this and ICEs will happen if this is violated.)
         let val = match const_.eval(self.tcx, param_env, None) {
             Ok(v) => v,
             Err(ErrorHandled::Reported(..)) => return,
diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs
@@ -1114,6 +1114,9 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co
 
     let (items, usage_map) = collector::collect_crate_mono_items(tcx, collection_mode);
 
+    // If there was an error during collection (e.g. from one of the constants we evaluated),
+    // then we stop here. This way codegen does not have to worry about failing constants.
+    // (codegen relies on this and ICEs will happen if this is violated.)
     tcx.dcx().abort_if_errors();
 
     let (codegen_units, _) = tcx.sess.time("partition_and_assert_distinct_symbols", || {

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ pub(crate) fn eval_mir_constant<'tcx>(`
`71`	`71`	`// This cannot fail because we checked all required_consts in advance.`
`72`	`72`	`let val = cv`
`73`	`73`	`.eval(fx.tcx, ty::ParamEnv::reveal_all(), Some(constant.span))`
`74`		`- .expect("erroneous constant not captured by required_consts");`
	`74`	`+ .expect("erroneous constant missed by mono item collection");`
`75`	`75`	`(val, cv.ty())`
`76`	`76`	`}`
`77`	`77`
Original file line number	Diff line number	Diff line change
`@@ -21,11 +21,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`pub fn eval_mir_constant(&self, constant: &mir::ConstOperand<'tcx>) -> mir::ConstValue<'tcx> {`
`24`		- // `MirUsedCollector` visited all constants before codegen began, so if we got here there
`25`		`- // can be no more constants that fail to evaluate.`
	`24`	+ // `MirUsedCollector` visited all required_consts before codegen began, so if we got here
	`25`	`+ // there can be no more constants that fail to evaluate.`
`26`	`26`	`self.monomorphize(constant.const_)`
`27`	`27`	`.eval(self.cx.tcx(), ty::ParamEnv::reveal_all(), Some(constant.span))`
`28`		`- .expect("erroneous constant not captured by required_consts")`
	`28`	`+ .expect("erroneous constant missed by mono item collection")`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	/// This is a convenience helper for `simd_shuffle_indices`. It has the precondition