Skip to content

Closures cannot be constants #50623

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 31 additions & 20 deletions src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,19 @@ impl<'a> DefCollector<'a> {
self.parent_def = parent;
}

pub fn visit_const_expr(&mut self, expr: &Expr) {
pub fn visit_const_expr(&mut self, expr: &Expr) -> Option<DefIndex> {
match expr.node {
// Find the node which will be used after lowering.
ExprKind::Paren(ref inner) => return self.visit_const_expr(inner),
ExprKind::Mac(..) => return self.visit_macro_invoc(expr.id, true),
ExprKind::Paren(ref inner) => self.visit_const_expr(inner),
ExprKind::Mac(..) => {
self.visit_macro_invoc(expr.id, true);
None
}
// FIXME(eddyb) Closures should have separate
// function definition IDs and expression IDs.
ExprKind::Closure(..) => return,
_ => {}
ExprKind::Closure(..) => None,
_ => Some(self.create_def(expr.id, DefPathData::Initializer, REGULAR_SPACE, expr.span))
}

self.create_def(expr.id, DefPathData::Initializer, REGULAR_SPACE, expr.span);
}

fn visit_macro_invoc(&mut self, id: NodeId, const_expr: bool) {
Expand Down Expand Up @@ -148,6 +149,8 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
}

if let Some(ref expr) = v.node.disr_expr {
// FIXME(#50689) The way this is visited makes it harder
// to fix that bug, see test closure-in-constant.rs
this.visit_const_expr(expr);
}
});
Expand Down Expand Up @@ -268,34 +271,42 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
let parent_def = self.parent_def;

match expr.node {
let parent = match expr.node {
ExprKind::Mac(..) => return self.visit_macro_invoc(expr.id, false),
ExprKind::Repeat(_, ref count) => self.visit_const_expr(count),
ExprKind::Closure(..) => {
let def = self.create_def(expr.id,
DefPathData::ClosureExpr,
REGULAR_SPACE,
expr.span);
self.parent_def = Some(def);
Some(self.create_def(expr.id,
DefPathData::ClosureExpr,
REGULAR_SPACE,
expr.span))
}
_ => {}
}
_ => None
};

self.parent_def = parent.or(self.parent_def);
visit::walk_expr(self, expr);
self.parent_def = parent_def;
}

fn visit_ty(&mut self, ty: &'a Ty) {
match ty.node {
let parent = match ty.node {
TyKind::Mac(..) => return self.visit_macro_invoc(ty.id, false),
TyKind::Array(_, ref length) => self.visit_const_expr(length),
TyKind::Array(_, ref length) => {
self.visit_const_expr(length)
}
TyKind::ImplTrait(..) => {
self.create_def(ty.id, DefPathData::ImplTrait, REGULAR_SPACE, ty.span);
None
}
TyKind::Typeof(ref expr) => self.visit_const_expr(expr),
_ => {}
}
TyKind::Typeof(ref expr) => {
self.visit_const_expr(expr)
}
_ => None
};
let prev_parent = self.parent_def;
self.parent_def = parent.or(self.parent_def);
visit::walk_ty(self, ty);
self.parent_def = prev_parent;
}

fn visit_stmt(&mut self, stmt: &'a Stmt) {
Expand Down
11 changes: 10 additions & 1 deletion src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,18 @@ pub fn const_eval_provider<'a, 'tcx>(
}

if let Some(id) = tcx.hir.as_local_node_id(def_id) {
let tables = tcx.typeck_tables_of(def_id);
let span = tcx.def_span(def_id);

// Closures in constant position can hit this in type collection.
if !tcx.has_typeck_tables(def_id) {
return Err(ConstEvalErr {
kind: Lrc::new(TypeckError),
span,
});
}

let tables = tcx.typeck_tables_of(def_id);

// Do match-check before building MIR
if tcx.check_match(def_id).is_err() {
return Err(ConstEvalErr {
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,12 +1085,16 @@ fn type_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

NodeField(field) => icx.to_ty(&field.ty),

NodeExpr(&hir::Expr { node: hir::ExprClosure(.., gen), .. }) => {
NodeExpr(&hir::Expr { node: hir::ExprClosure(.., gen), span, .. }) => {
if gen.is_some() {
let hir_id = tcx.hir.node_to_hir_id(node_id);
return tcx.typeck_tables_of(def_id).node_id_to_type(hir_id);
}

if !tcx.has_typeck_tables(def_id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this check? I thought the DefIds were setup correctly now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The def ids in discriminants will still have the enum as parent (so will fail here), for structs a case like struct Foo([u8; |x: u8| { }]) will also fail because the closure itself is the outer expression so it will have no parent to attach. So yhea without the check this would still ICE in some cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh... I thought the changes that made (|x: u8| {}, 42).1 work also got rid of the ICE.

span_err!(tcx.sess, span, E0912, "expected numerical constant, found closure");
}

let substs = ty::ClosureSubsts {
substs: Substs::for_item(
tcx,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4770,4 +4770,5 @@ register_diagnostics! {
E0641, // cannot cast to/from a pointer with an unknown kind
E0645, // trait aliases not finished
E0907, // type inside generator must be known in this context
E0912, // expected numerical constant, found closure
}
21 changes: 21 additions & 0 deletions src/test/run-pass/ctfe/closure-in-constant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

struct Foo ([u8; (|x: u8| { }, 9).1]);

// FIXME(#50689) We'd like the below to also work,
// but due to how DefCollector handles discr_expr differently it doesn't right now.
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also add a this fixme the the location where discr_expr is handled?

/*
enum Functions {
Square = (|x:i32| { }, 42).1,
}
*/

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/const-eval/issue-50600.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Testing that these do not ICE.

struct Foo ([u8; |x: u8| { }]);
//~^ ERROR expected numerical constant, found closure
enum Functions {
Square = |x:i32| { },
//~^ ERROR expected numerical constant, found closure
}

fn main() {}
15 changes: 15 additions & 0 deletions src/test/ui/const-eval/issue-50600.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0912]: expected numerical constant, found closure
--> $DIR/issue-50600.rs:13:18
|
LL | struct Foo ([u8; |x: u8| { }]);
| ^^^^^^^^^^^

error[E0912]: expected numerical constant, found closure
--> $DIR/issue-50600.rs:16:14
|
LL | Square = |x:i32| { },
| ^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0912`.