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

Revert compute_const changes in WF #68977

Closed
varkor opened this issue Feb 9, 2020 · 5 comments · Fixed by #70107
Closed

Revert compute_const changes in WF #68977

varkor opened this issue Feb 9, 2020 · 5 comments · Fixed by #70107
Assignees
Labels
A-const-generics Area: const generics (parameters and arguments) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@varkor
Copy link
Member

varkor commented Feb 9, 2020

See https://github.com/rust-lang/rust/pull/56723/files/03b892860da5aff7a10cf4ca1364dbcacb95bfcd#r376641987. Opening this issue so I don't forget about it.

cc @eddyb

This issue has been assigned to @lcnr via this comment.

@varkor varkor added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-generics Area: const generics (parameters and arguments) labels Feb 9, 2020
@eddyb
Copy link
Member

eddyb commented Mar 4, 2020

I was trying something out to show @ruabmbua their idea can't work yet and accidentally came up with something that compiles even if it shouldn't. I'm pretty sure it's because of this bug.

@ruabmbua
Copy link
Contributor

ruabmbua commented Mar 4, 2020

Extended example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=aa9afdff6a2f23a8f0cc588fdafe4c17

It would be nice, if is possible to build something like that. Even, if it`s not exactly the same.

@varkor varkor self-assigned this Mar 14, 2020
@varkor
Copy link
Member Author

varkor commented Mar 14, 2020

Assigning to myself so I don't forget, but I'm not sure I'll be able to get to this very soon, so if someone else wants to take it, you're welcome to.

@eddyb
Copy link
Member

eddyb commented Mar 19, 2020

WF is also missing something like this for ty::ConstKind::Infer:

// Inference variables are the complicated case, since we don't
// know what type they are. We do two things:
//
// 1. Check if they have been resolved, and if so proceed with
// THAT type.
// 2. If not, check whether this is the type that we
// started with (ty0). In that case, we've made no
// progress at all, so return false. Otherwise,
// we've at least simplified things (i.e., we went
// from `Vec<$0>: WF` to `$0: WF`, so we can
// register a pending obligation and keep
// moving. (Goal is that an "inductive hypothesis"
// is satisfied to ensure termination.)
ty::Infer(_) => {
let ty = self.infcx.shallow_resolve(ty);
if let ty::Infer(_) = ty.kind {
// not yet resolved...
if ty == ty0 {
// ...this is the type we started from! no progress.
return false;
}
let cause = self.cause(traits::MiscObligation);
self.out.push(
// ...not the type we started from, so we made progress.
traits::Obligation::new(
cause,
self.param_env,
ty::Predicate::WellFormed(ty),
),
);
} else {
// Yes, resolved, proceed with the
// result. Should never return false because
// `ty` is not a Infer.
assert!(self.compute(ty));
}
}

Fixing this might require Predicate::WellFormedConst, which I thought would replace Predicate::ConstEvaluatable but now I'm less sure it would.
It shouldn't hurt to have both at once and we can deduplicate if possible later.

The other problem is this needs to add Predicate::WellFormedConsts:

/// Registers obligations that all types appearing in `substs` are well-formed.
pub fn add_wf_bounds(&self, substs: SubstsRef<'tcx>, expr: &hir::Expr<'_>) {
for ty in substs.types() {
if !ty.references_error() {
self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
}
}
}

@lcnr
Copy link
Contributor

lcnr commented Mar 19, 2020

@rustbot claim

@rustbot rustbot assigned rustbot and unassigned varkor Mar 19, 2020
@bors bors closed this as completed in ff4aff6 Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants