Skip to content

Commit

Permalink
Auto merge of #70452 - eddyb:repeat-expr-correct-generics-parent, r=n…
Browse files Browse the repository at this point in the history
…ikomatsakis

typeck: always expose repeat count `AnonConst`s' parent in `generics_of`.

This should reduce some of the confusion around #43408, although, if you look at the changed test outputs (for the last commit), they all hit #68436, so nothing new will start compiling.

We can let counts of "repeat expressions" (`N` in `[x; N]`) always have the correct generics parenting, because they're always in a body, so nothing in the `where` clauses or `impl` trait/type of the parent can use it, and therefore no query cycles can occur.

<hr/>

Other potential candidates we might want to apply the same approach to, are:
* ~~(easy) `enum` discriminants (see also #70453)~~ opened #70825
* (trickier) array *type* (not *expression*) lengths nested in:
  * bodies
  * types of (associated or not) `const`/`static`
  * RHS of `type` aliases and associated `type`s
  * `fn` signatures

We should've done so from the start, the only reason we haven't is because I was squeamish about blacklisting some of the cases, but if we whitelist instead we should be fine.
Also, lazy normalization is taking forever 😞.

<hr/>

There's also 5 other commits here:
* "typeck: track any errors injected during writeback and taint tables appropriately." - fixes #66706, as the next commit would otherwise trigger an ICE again
* "typeck: workaround WF hole in `to_const`." - its purpose is to emulate most of #70107's direct effect, at least in the case of repeat expressions, where the count always goes through `to_const`
  * this is the reason no new code can really compile, as the WF checks require #68436 to bypass
  * however, this has more test changes than I hoped, so it should be reviewed separately, and maybe even landed separately (as #70107 might take a while, as it's blocked on a few of my PRs)
* "ty: erase lifetimes early in `ty::Const::eval`." - first attempt at fixing #70773
  * still useful, I believe the new approach is less likely to cause issues long-term
  * I could take this out or move it into another PR if desired or someone else could take over (cc @Skinny121)
* "traits/query/normalize: add some `debug!` logging for the result." - debugging aid for #70773
* "borrow_check/type_check: normalize `Aggregate` and `Call` operands." - actually fixes #70773

r? @nikomatsakis cc @pnkfelix @varkor @yodaldevoid @oli-obk @estebank
  • Loading branch information
bors committed Apr 14, 2020
2 parents edc0258 + 8bb7b7b commit d12f030
Show file tree
Hide file tree
Showing 30 changed files with 276 additions and 118 deletions.
58 changes: 28 additions & 30 deletions src/librustc_middle/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2339,43 +2339,41 @@ impl<'tcx> Const<'tcx> {
/// Tries to evaluate the constant if it is `Unevaluated`. If that doesn't succeed, return the
/// unevaluated constant.
pub fn eval(&self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> &Const<'tcx> {
let try_const_eval = |did, param_env: ParamEnv<'tcx>, substs, promoted| {
if let ConstKind::Unevaluated(did, substs, promoted) = self.val {
let param_env_and_substs = param_env.with_reveal_all().and(substs);

// Avoid querying `tcx.const_eval(...)` with any inference vars.
if param_env_and_substs.needs_infer() {
return None;
}
// HACK(eddyb) this erases lifetimes even though `const_eval_resolve`
// also does later, but we want to do it before checking for
// inference variables.
let param_env_and_substs = tcx.erase_regions(&param_env_and_substs);

// HACK(eddyb) when the query key would contain inference variables,
// attempt using identity substs and `ParamEnv` instead, that will succeed
// when the expression doesn't depend on any parameters.
// FIXME(eddyb, skinny121) pass `InferCtxt` into here when it's available, so that
// we can call `infcx.const_eval_resolve` which handles inference variables.
let param_env_and_substs = if param_env_and_substs.needs_infer() {
tcx.param_env(did).and(InternalSubsts::identity_for_item(tcx, did))
} else {
param_env_and_substs
};

// FIXME(eddyb) maybe the `const_eval_*` methods should take
// `ty::ParamEnvAnd<SubstsRef>` instead of having them separate.
let (param_env, substs) = param_env_and_substs.into_parts();

// try to resolve e.g. associated constants to their definition on an impl, and then
// evaluate the const.
tcx.const_eval_resolve(param_env, did, substs, promoted, None)
.ok()
.map(|val| Const::from_value(tcx, val, self.ty))
};

match self.val {
ConstKind::Unevaluated(did, substs, promoted) => {
// HACK(eddyb) when substs contain inference variables,
// attempt using identity substs instead, that will succeed
// when the expression doesn't depend on any parameters.
// FIXME(eddyb, skinny121) pass `InferCtxt` into here when it's available, so that
// we can call `infcx.const_eval_resolve` which handles inference variables.
if substs.needs_infer() {
let identity_substs = InternalSubsts::identity_for_item(tcx, did);
// The `ParamEnv` needs to match the `identity_substs`.
let identity_param_env = tcx.param_env(did);
match try_const_eval(did, identity_param_env, identity_substs, promoted) {
Some(ct) => ct.subst(tcx, substs),
None => self,
}
} else {
try_const_eval(did, param_env, substs, promoted).unwrap_or(self)
}
match tcx.const_eval_resolve(param_env, did, substs, promoted, None) {
// NOTE(eddyb) `val` contains no lifetimes/types/consts,
// and we use the original type, so nothing from `substs`
// (which may be identity substs, see above),
// can leak through `val` into the const we return.
Ok(val) => Const::from_value(tcx, val, self.ty),

Err(_) => self,
}
_ => self,
} else {
self
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/borrow_check/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1760,6 +1760,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
for (n, (fn_arg, op_arg)) in sig.inputs().iter().zip(args).enumerate() {
let op_arg_ty = op_arg.ty(body, self.tcx());
let op_arg_ty = self.normalize(op_arg_ty, term_location);
let category = if from_hir_call {
ConstraintCategory::CallArgument
} else {
Expand Down Expand Up @@ -2402,6 +2403,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
};
let operand_ty = operand.ty(body, tcx);
let operand_ty = self.normalize(operand_ty, location);

if let Err(terr) = self.sub_types(
operand_ty,
Expand Down
15 changes: 13 additions & 2 deletions src/librustc_trait_selection/traits/query/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,22 @@ impl<'cx, 'tcx> AtExt<'tcx> for At<'cx, 'tcx> {
anon_depth: 0,
};

let value1 = value.fold_with(&mut normalizer);
let result = value.fold_with(&mut normalizer);
debug!(
"normalize::<{}>: result={:?} with {} obligations",
::std::any::type_name::<T>(),
result,
normalizer.obligations.len(),
);
debug!(
"normalize::<{}>: obligations={:?}",
::std::any::type_name::<T>(),
normalizer.obligations,
);
if normalizer.error {
Err(NoSolution)
} else {
Ok(Normalized { value: value1, obligations: normalizer.obligations })
Ok(Normalized { value: result, obligations: normalizer.obligations })
}
}
}
Expand Down
27 changes: 25 additions & 2 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3311,8 +3311,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

pub fn to_const(&self, ast_c: &hir::AnonConst) -> &'tcx ty::Const<'tcx> {
let c = self.tcx.hir().local_def_id(ast_c.hir_id).expect_local();
ty::Const::from_anon_const(self.tcx, c)
let const_def_id = self.tcx.hir().local_def_id(ast_c.hir_id).expect_local();
let c = ty::Const::from_anon_const(self.tcx, const_def_id);

// HACK(eddyb) emulate what a `WellFormedConst` obligation would do.
// This code should be replaced with the proper WF handling ASAP.
if let ty::ConstKind::Unevaluated(def_id, substs, promoted) = c.val {
assert!(promoted.is_none());

// HACK(eddyb) let's hope these are always empty.
// let obligations = self.nominal_obligations(def_id, substs);
// self.out.extend(obligations);

let cause = traits::ObligationCause::new(
self.tcx.def_span(const_def_id.to_def_id()),
self.body_id,
traits::MiscObligation,
);
self.register_predicate(traits::Obligation::new(
cause,
self.param_env,
ty::Predicate::ConstEvaluatable(def_id, substs),
));
}

c
}

// If the type given by the user has free regions, save it for later, since
Expand Down
20 changes: 16 additions & 4 deletions src/librustc_typeck/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
wbcx.tables.upvar_list =
mem::replace(&mut self.tables.borrow_mut().upvar_list, Default::default());

wbcx.tables.tainted_by_errors = self.is_tainted_by_errors();
wbcx.tables.tainted_by_errors |= self.is_tainted_by_errors();

debug!("writeback: tables for {:?} are {:#?}", item_def_id, wbcx.tables);

Expand Down Expand Up @@ -578,14 +578,21 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
}
}

fn resolve<T>(&self, x: &T, span: &dyn Locatable) -> T
fn resolve<T>(&mut self, x: &T, span: &dyn Locatable) -> T
where
T: TypeFoldable<'tcx>,
{
let x = x.fold_with(&mut Resolver::new(self.fcx, span, self.body));
let mut resolver = Resolver::new(self.fcx, span, self.body);
let x = x.fold_with(&mut resolver);
if cfg!(debug_assertions) && x.needs_infer() {
span_bug!(span.to_span(self.fcx.tcx), "writeback: `{:?}` has inference variables", x);
}

// We may have introduced e.g. `ty::Error`, if inference failed, make sure
// to mark the `TypeckTables` as tainted in that case, so that downstream
// users of the tables don't produce extra errors, or worse, ICEs.
self.tables.tainted_by_errors |= resolver.replaced_with_error;

x
}
}
Expand Down Expand Up @@ -613,6 +620,9 @@ struct Resolver<'cx, 'tcx> {
infcx: &'cx InferCtxt<'cx, 'tcx>,
span: &'cx dyn Locatable,
body: &'tcx hir::Body<'tcx>,

/// Set to `true` if any `Ty` or `ty::Const` had to be replaced with an `Error`.
replaced_with_error: bool,
}

impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
Expand All @@ -621,7 +631,7 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
span: &'cx dyn Locatable,
body: &'tcx hir::Body<'tcx>,
) -> Resolver<'cx, 'tcx> {
Resolver { tcx: fcx.tcx, infcx: fcx, span, body }
Resolver { tcx: fcx.tcx, infcx: fcx, span, body, replaced_with_error: false }
}

fn report_error(&self, t: Ty<'tcx>) {
Expand All @@ -644,6 +654,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Resolver<'cx, 'tcx> {
Err(_) => {
debug!("Resolver::fold_ty: input type `{:?}` not fully resolvable", t);
self.report_error(t);
self.replaced_with_error = true;
self.tcx().types.err
}
}
Expand All @@ -661,6 +672,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Resolver<'cx, 'tcx> {
debug!("Resolver::fold_const: input const `{:?}` not fully resolvable", ct);
// FIXME: we'd like to use `self.report_error`, but it doesn't yet
// accept a &'tcx ty::Const.
self.replaced_with_error = true;
self.tcx().consts.err
}
}
Expand Down
20 changes: 17 additions & 3 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,14 +1170,28 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics {
}
// FIXME(#43408) enable this always when we get lazy normalization.
Node::AnonConst(_) => {
let parent_id = tcx.hir().get_parent_item(hir_id);
let parent_def_id = tcx.hir().local_def_id(parent_id);

// HACK(eddyb) this provides the correct generics when
// `feature(const_generics)` is enabled, so that const expressions
// used with const generics, e.g. `Foo<{N+1}>`, can work at all.
if tcx.features().const_generics {
let parent_id = tcx.hir().get_parent_item(hir_id);
Some(tcx.hir().local_def_id(parent_id))
Some(parent_def_id)
} else {
None
let parent_node = tcx.hir().get(tcx.hir().get_parent_node(hir_id));
match parent_node {
// HACK(eddyb) this provides the correct generics for repeat
// expressions' count (i.e. `N` in `[x; N]`), as they shouldn't
// be able to cause query cycle errors.
Node::Expr(&Expr { kind: ExprKind::Repeat(_, ref constant), .. })
if constant.hir_id == hir_id =>
{
Some(parent_def_id)
}

_ => None,
}
}
}
Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(..), .. }) => {
Expand Down
5 changes: 5 additions & 0 deletions src/test/compile-fail/issue-52443.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ fn main() {
//~| WARN denote infinite loops with
[(); { for _ in 0usize.. {}; 0}];
//~^ ERROR `for` is not allowed in a `const`
//~| ERROR calls in constants are limited to constant functions
//~| ERROR references in constants may only refer to immutable values
//~| ERROR calls in constants are limited to constant functions
//~| ERROR constant contains unimplemented expression type
//~| ERROR evaluation of constant value failed
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl Foo for Def {

pub fn test<A: Foo, B: Foo>() {
let _array = [4; <A as Foo>::Y];
//~^ ERROR the trait bound `A: Foo` is not satisfied [E0277]
//~^ ERROR constant expression depends on a generic parameter
}

fn main() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
error[E0277]: the trait bound `A: Foo` is not satisfied
error: constant expression depends on a generic parameter
--> $DIR/associated-const-type-parameter-arrays-2.rs:16:22
|
LL | const Y: usize;
| --------------- required by `Foo::Y`
...
LL | let _array = [4; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
| ^^^^^^^^^^^^^
|
help: consider further restricting this bound
|
LL | pub fn test<A: Foo + Foo, B: Foo>() {
| ^^^^^
= note: this may fail depending on what value the parameter takes

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
3 changes: 1 addition & 2 deletions src/test/ui/const-generics/issues/issue-62456.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

// build-pass

fn foo<const N: usize>() {
let _ = [0u64; N + 1];
//~^ ERROR constant expression depends on a generic parameter
}

fn main() {}
10 changes: 9 additions & 1 deletion src/test/ui/const-generics/issues/issue-62456.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,13 @@ LL | #![feature(const_generics)]
|
= note: `#[warn(incomplete_features)]` on by default

warning: 1 warning emitted
error: constant expression depends on a generic parameter
--> $DIR/issue-62456.rs:5:20
|
LL | let _ = [0u64; N + 1];
| ^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to previous error; 1 warning emitted

1 change: 1 addition & 0 deletions src/test/ui/const-generics/issues/issue-62504.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ impl<const X: usize> ArrayHolder<X> {
pub const fn new() -> Self {
ArrayHolder([0; Self::SIZE])
//~^ ERROR: mismatched types
//~| ERROR constant expression depends on a generic parameter
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/test/ui/const-generics/issues/issue-62504.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ LL | ArrayHolder([0; Self::SIZE])
= note: expected array `[u32; _]`
found array `[u32; _]`

error: aborting due to previous error
error: constant expression depends on a generic parameter
--> $DIR/issue-62504.rs:18:25
|
LL | ArrayHolder([0; Self::SIZE])
| ^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
3 changes: 1 addition & 2 deletions src/test/ui/const-generics/issues/issue-66205.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// check-pass

#![allow(incomplete_features, dead_code, unconditional_recursion)]
#![feature(const_generics)]

fn fact<const N: usize>() {
fact::<{ N - 1 }>();
//~^ ERROR constant expression depends on a generic parameter
}

fn main() {}
10 changes: 10 additions & 0 deletions src/test/ui/const-generics/issues/issue-66205.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: constant expression depends on a generic parameter
--> $DIR/issue-66205.rs:5:12
|
LL | fact::<{ N - 1 }>();
| ^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to previous error

3 changes: 1 addition & 2 deletions src/test/ui/const-generics/issues/issue-67739.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// Regression test for #67739

// check-pass

#![allow(incomplete_features)]
#![feature(const_generics)]

Expand All @@ -12,6 +10,7 @@ pub trait Trait {

fn associated_size(&self) -> usize {
[0u8; mem::size_of::<Self::Associated>()];
//~^ ERROR constant expression depends on a generic parameter
0
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/const-generics/issues/issue-67739.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: constant expression depends on a generic parameter
--> $DIR/issue-67739.rs:12:15
|
LL | [0u8; mem::size_of::<Self::Associated>()];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to previous error

2 changes: 2 additions & 0 deletions src/test/ui/consts/const-eval/issue-52442.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
fn main() {
[(); { &loop { break } as *const _ as usize } ];
//~^ ERROR `loop` is not allowed in a `const`
//~| ERROR casting pointers to integers in constants is unstable
//~| ERROR evaluation of constant value failed
}
Loading

0 comments on commit d12f030

Please sign in to comment.