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.

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

rustc_mir::shim misuses ParamEnvs. #70520

Open
eddyb opened this issue Mar 29, 2020 · 0 comments
Open

rustc_mir::shim misuses ParamEnvs. #70520

eddyb opened this issue Mar 29, 2020 · 0 comments
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Mar 29, 2020

(This example is about CloneShim, but DropGlue has a similar issue)

In this snippet, param_env is obtained from tcx.param_env(def_id) and used twice with self_ty:

  1. for is_copy_modulo_regions
    • if self_ty doesn't involve type parameters, param_env doesn't matter, but if it does, they could be from anywhere (e.g. caller of clone, via MIR inliner), so the param_env being that of Clone::clone itself is not useful at all (all it contains is Self: Clone)
    • to get the right ParamEnv, the mir_shims query would need to take ParamEnvAnd<InstanceDef> instead of just InstanceDef (but we might not need this)
    • however it's a waste to generate one shim for every Copy type, we should instead do what DropShim does and have Option<Ty> in CloneShim, or split out of a CloneByCopyShim
  2. for evaluating an array length
    • the length (a ty::Const) is converted to u64 only to be converted back to ty::Const, which is unnecessary and we could skip it entirely

let param_env = tcx.param_env(def_id);
let mut builder = CloneShimBuilder::new(tcx, def_id, self_ty);
let is_copy = self_ty.is_copy_modulo_regions(tcx, param_env, builder.span);
let dest = Place::return_place();
let src = tcx.mk_place_deref(Place::from(Local::new(1 + 0)));
match self_ty.kind {
_ if is_copy => builder.copy_shim(),
ty::Array(ty, len) => {
let len = len.eval_usize(tcx, param_env);
builder.array_shim(dest, src, ty, len)
}

@jonas-schievink jonas-schievink added A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants