Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constant::eq skips spans #85208

Closed
wants to merge 2 commits into from
Closed

Constant::eq skips spans #85208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa9e3d8
WIP
spastorino May 13, 2021
c41bf86
WIP 2
spastorino May 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>

fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
let check = match terminator.kind {
mir::TerminatorKind::Call { func: mir::Operand::Constant(ref c), ref args, .. } => {
mir::TerminatorKind::Call { func: mir::Operand::Constant(box(_, ref c)), ref args, .. } => {
match *c.ty().kind() {
ty::FnDef(did, _) => Some((did, args)),
_ => None,
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// checked by const-qualification, which also
// promotes any complex rvalues to constants.
if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") {
if let mir::Operand::Constant(constant) = arg {
let c = self.eval_mir_constant(constant);
if let mir::Operand::Constant(box(span, constant)) = arg {
let c = self.eval_mir_constant(*span, constant);
let (llval, ty) =
self.simd_shuffle_indices(&bx, constant.span, constant.ty(), c);
self.simd_shuffle_indices(&bx, *span, constant.ty(), c);
return OperandRef {
val: Immediate(llval),
layout: bx.layout_of(ty),
Expand Down Expand Up @@ -821,9 +821,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
out_place.map(|out_place| self.codegen_place(&mut bx, out_place.as_ref()));
InlineAsmOperandRef::InOut { reg, late, in_value, out_place }
}
mir::InlineAsmOperand::Const { ref value } => {
mir::InlineAsmOperand::Const { span, ref value } => {
let const_value = self
.eval_mir_constant(value)
.eval_mir_constant(span, value)
.unwrap_or_else(|_| span_bug!(span, "asm const cannot be resolved"));
let ty = value.ty();
let size = bx.layout_of(ty).size;
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
pub fn eval_mir_constant_to_operand(
&self,
bx: &mut Bx,
span: Span,
constant: &mir::Constant<'tcx>,
) -> Result<OperandRef<'tcx, Bx::Value>, ErrorHandled> {
let val = self.eval_mir_constant(constant)?;
let val = self.eval_mir_constant(span, constant)?;
let ty = self.monomorphize(constant.ty());
Ok(OperandRef::from_const(bx, val, ty))
}

pub fn eval_mir_constant(
&self,
span: Span,
constant: &mir::Constant<'tcx>,
) -> Result<ConstValue<'tcx>, ErrorHandled> {
let ct = self.monomorphize(constant.literal);
Expand All @@ -35,12 +37,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
.tcx()
.const_eval_resolve(ty::ParamEnv::reveal_all(), ct, None)
.map_err(|err| {
self.cx.tcx().sess.span_err(constant.span, "erroneous constant encountered");
self.cx.tcx().sess.span_err(span, "erroneous constant encountered");
err
}),
ty::ConstKind::Value(value) => Ok(value),
err => span_bug!(
constant.span,
span,
"encountered bad ConstKind after monomorphizing: {:?}",
err
),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
None => continue,
};

if let Ok(operand) = self.eval_mir_constant_to_operand(bx, &c) {
if let Ok(operand) = self.eval_mir_constant_to_operand(bx, var.source_info.span, &c) {
let base = Self::spill_operand_to_stack(
&operand,
Some(var.name.to_string()),
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,14 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

// Evaluate all required consts; codegen later assumes that CTFE will never fail.
let mut all_consts_ok = true;
for const_ in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(const_) {
for (span, const_) in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(*span, const_) {
all_consts_ok = false;
match err {
// errored or at least linted
ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted => {}
ErrorHandled::TooGeneric => {
span_bug!(const_.span, "codgen encountered polymorphic constant: {:?}", err)
span_bug!(*span, "codgen encountered polymorphic constant: {:?}", err)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.codegen_consume(bx, place.as_ref())
}

mir::Operand::Constant(ref constant) => {
mir::Operand::Constant(box(span, ref constant)) => {
// This cannot fail because we checked all required_consts in advance.
self.eval_mir_constant_to_operand(bx, constant).unwrap_or_else(|_err| {
span_bug!(constant.span, "erroneous constant not captured by required_consts")
self.eval_mir_constant_to_operand(bx, span, constant).unwrap_or_else(|_err| {
span_bug!(span, "erroneous constant not captured by required_consts")
})
}
}
Expand Down
21 changes: 9 additions & 12 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub struct Body<'tcx> {

/// Constants that are required to evaluate successfully for this MIR to be well-formed.
/// We hold in this field all the constants we are not able to evaluate yet.
pub required_consts: Vec<Constant<'tcx>>,
pub required_consts: Vec<(Span, Constant<'tcx>)>,

/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
///
Expand Down Expand Up @@ -1222,6 +1222,7 @@ pub enum InlineAsmOperand<'tcx> {
out_place: Option<Place<'tcx>>,
},
Const {
span: Span,
value: Box<Constant<'tcx>>,
},
SymFn {
Expand Down Expand Up @@ -2029,7 +2030,7 @@ pub enum Operand<'tcx> {
Move(Place<'tcx>),

/// Synthesizes a constant value.
Constant(Box<Constant<'tcx>>),
Constant(Box<(Span, Constant<'tcx>)>),
}

#[cfg(target_arch = "x86_64")]
Expand Down Expand Up @@ -2057,11 +2058,10 @@ impl<'tcx> Operand<'tcx> {
span: Span,
) -> Self {
let ty = tcx.type_of(def_id).subst(tcx, substs);
Operand::Constant(box Constant {
span,
Operand::Constant(box (span, Constant {
user_ty: None,
literal: ConstantKind::Ty(ty::Const::zero_sized(tcx, ty)),
})
}))
}

pub fn is_move(&self) -> bool {
Expand All @@ -2088,11 +2088,10 @@ impl<'tcx> Operand<'tcx> {
};
scalar_size == type_size
});
Operand::Constant(box Constant {
span,
Operand::Constant(box (span, Constant {
user_ty: None,
literal: ConstantKind::Val(val.into(), ty),
})
}))
}

pub fn to_copy(&self) -> Self {
Expand All @@ -2113,9 +2112,9 @@ impl<'tcx> Operand<'tcx> {

/// Returns the `Constant` that is the target of this `Operand`, or `None` if this `Operand` is a
/// place.
pub fn constant(&self) -> Option<&Constant<'tcx>> {
pub fn constant(&self) -> Option<(&Span, &Constant<'tcx>)> {
match self {
Operand::Constant(x) => Some(&**x),
Operand::Constant(box(span, c)) => Some((span, c)),
Operand::Copy(_) | Operand::Move(_) => None,
}
}
Expand Down Expand Up @@ -2426,8 +2425,6 @@ impl<'tcx> Debug for Rvalue<'tcx> {

#[derive(Clone, Copy, PartialEq, PartialOrd, TyEncodable, TyDecodable, Hash, HashStable)]
pub struct Constant<'tcx> {
pub span: Span,

/// Optional user-given type: for something like
/// `collect::<Vec<_>>`, this would be present and would
/// indicate that `Vec<_>` was explicitly specified.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<'tcx> Operand<'tcx> {
{
match self {
&Operand::Copy(ref l) | &Operand::Move(ref l) => l.ty(local_decls, tcx).ty,
&Operand::Constant(ref c) => c.literal.ty(),
&Operand::Constant(box (_, ref c)) => c.literal.ty(),
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/terminator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,8 @@ impl<'tcx> TerminatorKind<'tcx> {
InlineAsmOperand::InOut { reg, late, in_value, out_place: None } => {
write!(fmt, "in{}out({}) {:?} => _", print_late(late), reg, in_value)?;
}
InlineAsmOperand::Const { value } => {
InlineAsmOperand::Const { value, .. } => {
// FIXME consider span?
spastorino marked this conversation as resolved.
Show resolved Hide resolved
write!(fmt, "const {:?}", value)?;
}
InlineAsmOperand::SymFn { value } => {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/type_foldable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ impl<'tcx, R: Idx, C: Idx> TypeFoldable<'tcx> for BitMatrix<R, C> {
impl<'tcx> TypeFoldable<'tcx> for Constant<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(self, folder: &mut F) -> Self {
Constant {
span: self.span,
user_ty: self.user_ty.fold_with(folder),
literal: self.literal.fold_with(folder),
}
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_middle/src/mir/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,9 @@ macro_rules! make_mir_visitor {

self.visit_span(&$($mutability)? body.span);

for const_ in &$($mutability)? body.required_consts {
for (span, const_) in &$($mutability)? body.required_consts {
let location = START_BLOCK.start_location();
self.visit_span(span);
self.visit_constant(const_, location);
}
}
Expand Down Expand Up @@ -606,8 +607,11 @@ macro_rules! make_mir_visitor {
);
}
}
InlineAsmOperand::Const { value }
| InlineAsmOperand::SymFn { value } => {
InlineAsmOperand::Const { value, span } => {
self.visit_span(span);
self.visit_constant(value, location);
}
InlineAsmOperand::SymFn { value } => {
self.visit_constant(value, location);
}
InlineAsmOperand::SymStatic { def_id: _ } => {}
Expand Down Expand Up @@ -775,7 +779,8 @@ macro_rules! make_mir_visitor {
location
);
}
Operand::Constant(constant) => {
Operand::Constant(box(span, constant)) => {
self.visit_span(span);
self.visit_constant(constant, location);
}
}
Expand Down Expand Up @@ -864,12 +869,10 @@ macro_rules! make_mir_visitor {
constant: & $($mutability)? Constant<'tcx>,
location: Location) {
let Constant {
span,
user_ty,
literal,
} = constant;

self.visit_span(span);
drop(user_ty); // no visit method for this
match literal {
ConstantKind::Ty(ct) => self.visit_const(ct, location),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
{
// Just point to the function, to reduce the chance of overlapping spans.
let function_span = match func {
Operand::Constant(c) => c.span,
Operand::Constant(box (span, _)) => *span,
Operand::Copy(place) | Operand::Move(place) => {
if let Some(l) = place.as_local() {
let local_decl = &self.body.local_decls[l];
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let terminator = self.body[location.block].terminator();
debug!("add_moved_or_invoked_closure_note: terminator={:?}", terminator);
if let TerminatorKind::Call {
func: Operand::Constant(box Constant { literal, .. }),
func: Operand::Constant(box (_, Constant { literal, .. })),
args,
..
} = &terminator.kind
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/invalidation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
self.mutate_place(location, out_place, Shallow(None), JustWrite);
}
}
InlineAsmOperand::Const { value: _ }
InlineAsmOperand::Const { span: _, value: _ }
| InlineAsmOperand::SymFn { value: _ }
| InlineAsmOperand::SymStatic { def_id: _ } => {}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc
);
}
}
InlineAsmOperand::Const { value: _ }
InlineAsmOperand::Const { span: _, value: _ }
| InlineAsmOperand::SymFn { value: _ }
| InlineAsmOperand::SymStatic { def_id: _ } => {}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/dataflow/move_paths/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
self.gather_init(out_place.as_ref(), InitKind::Deep);
}
}
InlineAsmOperand::Const { value: _ }
InlineAsmOperand::Const { span: _, value: _ }
| InlineAsmOperand::SymFn { value: _ }
| InlineAsmOperand::SymStatic { def_id: _ } => {}
}
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_mir/src/interpret/eval_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,14 +683,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.stack_mut().push(frame);

// Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
for const_ in &body.required_consts {
let span = const_.span;
for (span, const_) in &body.required_consts {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right! we have required_consts. Perfect. I think we may be able to remove the span from Operand::Const after all

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do this in a follow up PR? or do you want me to take some kind of action here?

let const_ =
self.subst_from_current_frame_and_normalize_erasing_regions(const_.literal);
self.mir_const_to_op(&const_, None).map_err(|err| {
// If there was an error, set the span of the current frame to this constant.
// Avoiding doing this when evaluation succeeds.
self.frame_mut().loc = Err(span);
self.frame_mut().loc = Err(*span);
err
})?;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/interpret/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// FIXME: do some more logic on `move` to invalidate the old location
Copy(place) | Move(place) => self.eval_place_to_op(place, layout)?,

Constant(ref constant) => {
Constant(box (_, ref constant)) => {
let val =
self.subst_from_current_frame_and_normalize_erasing_regions(constant.literal);
// This can still fail:
Expand Down
22 changes: 9 additions & 13 deletions compiler/rustc_mir/src/shim.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,11 +418,10 @@ impl CloneShimBuilder<'tcx> {

// `func == Clone::clone(&ty) -> ty`
let func_ty = tcx.mk_fn_def(self.def_id, substs);
let func = Operand::Constant(box Constant {
span: self.span,
let func = Operand::Constant(box (self.span, Constant {
user_ty: None,
literal: ty::Const::zero_sized(tcx, func_ty).into(),
});
}));

let ref_loc = self.make_place(
Mutability::Not,
Expand Down Expand Up @@ -474,12 +473,11 @@ impl CloneShimBuilder<'tcx> {
);
}

fn make_usize(&self, value: u64) -> Box<Constant<'tcx>> {
box Constant {
span: self.span,
fn make_usize(&self, value: u64) -> Box<(Span, Constant<'tcx>)> {
box (self.span, Constant {
user_ty: None,
literal: ty::Const::from_usize(self.tcx, value).into(),
}
})
}

fn array_shim(
Expand All @@ -506,11 +504,10 @@ impl CloneShimBuilder<'tcx> {
))),
self.make_statement(StatementKind::Assign(box (
end,
Rvalue::Use(Operand::Constant(box Constant {
span: self.span,
Rvalue::Use(Operand::Constant(box (self.span, Constant {
user_ty: None,
literal: len.into(),
})),
}))),
))),
];
self.block(inits, TerminatorKind::Goto { target: BasicBlock::new(1) }, false);
Expand Down Expand Up @@ -765,11 +762,10 @@ fn build_call_shim<'tcx>(
CallKind::Direct(def_id) => {
let ty = tcx.type_of(def_id);
(
Operand::Constant(box Constant {
span,
Operand::Constant(box (span, Constant {
user_ty: None,
literal: ty::Const::zero_sized(tcx, ty).into(),
}),
})),
rcvr.into_iter().collect::<Vec<_>>(),
)
}
Expand Down
Loading