Skip to content

Commit

Permalink
make InterpResult a dedicated type to avoid accidentally discarding t…
Browse files Browse the repository at this point in the history
…he error
  • Loading branch information
RalfJung committed Sep 30, 2024
1 parent 4b8a5bd commit dc0281f
Show file tree
Hide file tree
Showing 102 changed files with 1,596 additions and 1,469 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_const_eval/src/const_eval/dummy_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_middle::{bug, span_bug, ty};
use rustc_span::def_id::DefId;

use crate::interpret::{
self, HasStaticRootDefId, ImmTy, Immediate, InterpCx, PointerArithmetic, throw_machine_stop,
self, HasStaticRootDefId, ImmTy, Immediate, InterpCx, PointerArithmetic, interp_ok,
throw_machine_stop,
};

/// Macro for machine-specific `InterpError` without allocation.
Expand Down Expand Up @@ -79,7 +80,7 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine {
throw_machine_stop_str!("can't access mutable globals in ConstProp");
}

Ok(())
interp_ok(())
}

fn find_mir_or_eval_fn(
Expand Down Expand Up @@ -127,7 +128,7 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine {
right: &interpret::ImmTy<'tcx, Self::Provenance>,
) -> interpret::InterpResult<'tcx, ImmTy<'tcx, Self::Provenance>> {
use rustc_middle::mir::BinOp::*;
Ok(match bin_op {
interp_ok(match bin_op {
Eq | Ne | Lt | Le | Gt | Ge => {
// Types can differ, e.g. fn ptrs with different `for`.
assert_eq!(left.layout.abi, right.layout.abi);
Expand Down
53 changes: 31 additions & 22 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::const_eval::CheckAlignment;
use crate::interpret::{
CtfeValidationMode, GlobalId, Immediate, InternKind, InternResult, InterpCx, InterpError,
InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, StackPopCleanup, create_static_alloc,
eval_nullary_intrinsic, intern_const_alloc_recursive, throw_exhaust,
eval_nullary_intrinsic, intern_const_alloc_recursive, interp_ok, throw_exhaust,
};
use crate::{CTRL_C_RECEIVED, errors};

Expand Down Expand Up @@ -98,19 +98,19 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
return Err(ecx
.tcx
.dcx()
.emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
.into());
.emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }))
.into();
}
Err(InternResult::FoundBadMutablePointer) => {
return Err(ecx
.tcx
.dcx()
.emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind })
.into());
.emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind }))
.into();
}
}

Ok(R::make_result(ret, ecx))
interp_ok(R::make_result(ret, ecx))
}

/// The `InterpCx` is only meant to be used to do field and index projections into constants for
Expand Down Expand Up @@ -147,7 +147,8 @@ pub fn mk_eval_cx_for_const_val<'tcx>(
ty: Ty<'tcx>,
) -> Option<(CompileTimeInterpCx<'tcx>, OpTy<'tcx>)> {
let ecx = mk_eval_cx_to_read_const_val(tcx.tcx, tcx.span, param_env, CanAccessMutGlobal::No);
let op = ecx.const_val_to_op(val, ty, None).ok()?;
// FIXME: is it a problem to discard the error here?
let op = ecx.const_val_to_op(val, ty, None).discard_err()?;
Some((ecx, op))
}

Expand Down Expand Up @@ -185,12 +186,16 @@ pub(super) fn op_to_const<'tcx>(
_ => false,
};
let immediate = if force_as_immediate {
match ecx.read_immediate(op) {
match ecx.read_immediate(op).report_err() {
Ok(imm) => Right(imm),
Err(err) if !for_diagnostics => {
panic!("normalization works on validated constants: {err:?}")
Err(err) => {
if for_diagnostics {
// This discard the error, but for diagnostics that's okay.
op.as_mplace_or_imm()
} else {
panic!("normalization works on validated constants: {err:?}")
}
}
_ => op.as_mplace_or_imm(),
}
} else {
op.as_mplace_or_imm()
Expand Down Expand Up @@ -283,17 +288,19 @@ pub fn eval_to_const_value_raw_provider<'tcx>(
let ty::FnDef(_, args) = ty.kind() else {
bug!("intrinsic with type {:?}", ty);
};
return eval_nullary_intrinsic(tcx, key.param_env, def_id, args).map_err(|error| {
let span = tcx.def_span(def_id);

super::report(
tcx,
error.into_kind(),
span,
|| (span, vec![]),
|span, _| errors::NullaryIntrinsicError { span },
)
});
return eval_nullary_intrinsic(tcx, key.param_env, def_id, args).report_err().map_err(
|error| {
let span = tcx.def_span(def_id);

super::report(
tcx,
error.into_kind(),
span,
|| (span, vec![]),
|span, _| errors::NullaryIntrinsicError { span },
)
},
);
}

tcx.eval_to_allocation_raw(key).map(|val| turn_into_const_value(tcx, val, key))
Expand Down Expand Up @@ -376,6 +383,7 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>(
);
let res = ecx.load_mir(cid.instance.def, cid.promoted);
res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body))
.report_err()
.map_err(|error| report_eval_error(&ecx, cid, error))
}

Expand All @@ -400,6 +408,7 @@ fn const_validate_mplace<'tcx>(
}
};
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)
.report_err()
// Instead of just reporting the `InterpError` via the usual machinery, we give a more targeted
// error about the validation failure.
.map_err(|error| report_validation_error(&ecx, cid, error, alloc_id))?;
Expand Down
68 changes: 34 additions & 34 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use crate::fluent_generated as fluent;
use crate::interpret::{
self, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame, GlobalAlloc, ImmTy,
InterpCx, InterpResult, MPlaceTy, OpTy, Pointer, PointerArithmetic, RangeSet, Scalar,
StackPopCleanup, compile_time_machine, err_ub, throw_exhaust, throw_inval, throw_ub_custom,
throw_unsup, throw_unsup_format,
StackPopCleanup, compile_time_machine, interp_ok, throw_exhaust, throw_inval, throw_ub,
throw_ub_custom, throw_unsup, throw_unsup_format,
};

/// When hitting this many interpreted terminators we emit a deny by default lint
Expand Down Expand Up @@ -247,7 +247,7 @@ impl<'tcx> CompileTimeInterpCx<'tcx> {
let msg = Symbol::intern(self.read_str(&msg_place)?);
let span = self.find_closest_untracked_caller_location();
let (file, line, col) = self.location_triple_for_span(span);
return Err(ConstEvalErrKind::Panic { msg, file, line, col }.into());
return Err(ConstEvalErrKind::Panic { msg, file, line, col }).into();
} else if self.tcx.is_lang_item(def_id, LangItem::PanicFmt) {
// For panic_fmt, call const_panic_fmt instead.
let const_def_id = self.tcx.require_lang_item(LangItem::ConstPanicFmt, None);
Expand All @@ -259,16 +259,16 @@ impl<'tcx> CompileTimeInterpCx<'tcx> {
self.cur_span(),
);

return Ok(Some(new_instance));
return interp_ok(Some(new_instance));
} else if self.tcx.is_lang_item(def_id, LangItem::AlignOffset) {
let args = self.copy_fn_args(args);
// For align_offset, we replace the function call if the pointer has no address.
match self.align_offset(instance, &args, dest, ret)? {
ControlFlow::Continue(()) => return Ok(Some(instance)),
ControlFlow::Break(()) => return Ok(None),
ControlFlow::Continue(()) => return interp_ok(Some(instance)),
ControlFlow::Break(()) => return interp_ok(None),
}
}
Ok(Some(instance))
interp_ok(Some(instance))
}

/// `align_offset(ptr, target_align)` needs special handling in const eval, because the pointer
Expand Down Expand Up @@ -323,25 +323,25 @@ impl<'tcx> CompileTimeInterpCx<'tcx> {
dest,
StackPopCleanup::Goto { ret, unwind: mir::UnwindAction::Unreachable },
)?;
Ok(ControlFlow::Break(()))
interp_ok(ControlFlow::Break(()))
} else {
// Not alignable in const, return `usize::MAX`.
let usize_max = Scalar::from_target_usize(self.target_usize_max(), self);
self.write_scalar(usize_max, dest)?;
self.return_to_block(ret)?;
Ok(ControlFlow::Break(()))
interp_ok(ControlFlow::Break(()))
}
}
Err(_addr) => {
// The pointer has an address, continue with function call.
Ok(ControlFlow::Continue(()))
interp_ok(ControlFlow::Continue(()))
}
}
}

/// See documentation on the `ptr_guaranteed_cmp` intrinsic.
fn guaranteed_cmp(&mut self, a: Scalar, b: Scalar) -> InterpResult<'tcx, u8> {
Ok(match (a, b) {
interp_ok(match (a, b) {
// Comparisons between integers are always known.
(Scalar::Int { .. }, Scalar::Int { .. }) => {
if a == b {
Expand Down Expand Up @@ -403,8 +403,8 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
instance: ty::InstanceKind<'tcx>,
) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> {
match instance {
ty::InstanceKind::Item(def) => Ok(ecx.tcx.mir_for_ctfe(def)),
_ => Ok(ecx.tcx.instance_mir(instance)),
ty::InstanceKind::Item(def) => interp_ok(ecx.tcx.mir_for_ctfe(def)),
_ => interp_ok(ecx.tcx.instance_mir(instance)),
}
}

Expand All @@ -422,7 +422,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
// Replace some functions.
let Some(instance) = ecx.hook_special_const_fn(orig_instance, args, dest, ret)? else {
// Call has already been handled.
return Ok(None);
return interp_ok(None);
};

// Only check non-glue functions
Expand All @@ -444,14 +444,14 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
// This is a const fn. Call it.
// In case of replacement, we return the *original* instance to make backtraces work out
// (and we hope this does not confuse the FnAbi checks too much).
Ok(Some((ecx.load_mir(instance.def, None)?, orig_instance)))
interp_ok(Some((ecx.load_mir(instance.def, None)?, orig_instance)))
}

fn panic_nounwind(ecx: &mut InterpCx<'tcx, Self>, msg: &str) -> InterpResult<'tcx> {
let msg = Symbol::intern(msg);
let span = ecx.find_closest_untracked_caller_location();
let (file, line, col) = ecx.location_triple_for_span(span);
Err(ConstEvalErrKind::Panic { msg, file, line, col }.into())
Err(ConstEvalErrKind::Panic { msg, file, line, col }).into()
}

fn call_intrinsic(
Expand All @@ -464,7 +464,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
// Shared intrinsics.
if ecx.eval_intrinsic(instance, args, dest, target)? {
return Ok(None);
return interp_ok(None);
}
let intrinsic_name = ecx.tcx.item_name(instance.def_id());

Expand Down Expand Up @@ -541,7 +541,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
"intrinsic `{intrinsic_name}` is not supported at compile-time"
);
}
return Ok(Some(ty::Instance {
return interp_ok(Some(ty::Instance {
def: ty::InstanceKind::Item(instance.def_id()),
args: instance.args,
}));
Expand All @@ -550,7 +550,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {

// Intrinsic is done, jump to next block.
ecx.return_to_block(target)?;
Ok(None)
interp_ok(None)
}

fn assert_panic(
Expand Down Expand Up @@ -581,7 +581,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
}
}
};
Err(ConstEvalErrKind::AssertFailure(err).into())
Err(ConstEvalErrKind::AssertFailure(err)).into()
}

fn binary_ptr_op(
Expand Down Expand Up @@ -652,7 +652,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
}
}

Ok(())
interp_ok(())
}

#[inline(always)]
Expand All @@ -670,7 +670,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
if !ecx.recursion_limit.value_within_limit(ecx.stack().len() + 1) {
throw_exhaust!(StackFrameLimitReached)
} else {
Ok(frame)
interp_ok(frame)
}
}

Expand Down Expand Up @@ -700,22 +700,22 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
if is_write {
// Write access. These are never allowed, but we give a targeted error message.
match alloc.mutability {
Mutability::Not => Err(err_ub!(WriteToReadOnly(alloc_id)).into()),
Mutability::Mut => Err(ConstEvalErrKind::ModifiedGlobal.into()),
Mutability::Not => throw_ub!(WriteToReadOnly(alloc_id)),
Mutability::Mut => Err(ConstEvalErrKind::ModifiedGlobal).into(),
}
} else {
// Read access. These are usually allowed, with some exceptions.
if machine.can_access_mut_global == CanAccessMutGlobal::Yes {
// Machine configuration allows us read from anything (e.g., `static` initializer).
Ok(())
interp_ok(())
} else if alloc.mutability == Mutability::Mut {
// Machine configuration does not allow us to read statics (e.g., `const`
// initializer).
Err(ConstEvalErrKind::ConstAccessesMutGlobal.into())
Err(ConstEvalErrKind::ConstAccessesMutGlobal).into()
} else {
// Immutable global, this read is fine.
assert_eq!(alloc.mutability, Mutability::Not);
Ok(())
interp_ok(())
}
}
}
Expand Down Expand Up @@ -748,9 +748,9 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
// even when there is interior mutability.)
place.map_provenance(CtfeProvenance::as_shared_ref)
};
Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
interp_ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
} else {
Ok(val.clone())
interp_ok(val.clone())
}
}

Expand All @@ -763,20 +763,20 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
) -> InterpResult<'tcx> {
if range.size == Size::ZERO {
// Nothing to check.
return Ok(());
return interp_ok(());
}
// Reject writes through immutable pointers.
if immutable {
return Err(ConstEvalErrKind::WriteThroughImmutablePointer.into());
return Err(ConstEvalErrKind::WriteThroughImmutablePointer).into();
}
// Everything else is fine.
Ok(())
interp_ok(())
}

fn before_alloc_read(ecx: &InterpCx<'tcx, Self>, alloc_id: AllocId) -> InterpResult<'tcx> {
// Check if this is the currently evaluated static.
if Some(alloc_id) == ecx.machine.static_root_ids.map(|(id, _)| id) {
return Err(ConstEvalErrKind::RecursiveStatic.into());
return Err(ConstEvalErrKind::RecursiveStatic).into();
}
// If this is another static, make sure we fire off the query to detect cycles.
// But only do that when checks for static recursion are enabled.
Expand All @@ -788,7 +788,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
ecx.ctfe_query(|tcx| tcx.eval_static_initializer(def_id))?;
}
}
Ok(())
interp_ok(())
}

fn cached_union_data_range<'e>(
Expand Down
Loading

0 comments on commit dc0281f

Please sign in to comment.