From 7bb6de0ab90e0a1d4d8c7b93dfc1db870bcf1333 Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Thu, 10 May 2018 15:46:04 -0300 Subject: [PATCH 1/4] closures cannot be constants During type collection, error if a closures is found in constant position, catching that before they go causing ICEs. Fixes #50600. Fixes #48838. --- src/librustc_mir/interpret/const_eval.rs | 11 ++++++++++- src/librustc_typeck/collect.rs | 6 +++++- src/librustc_typeck/diagnostics.rs | 1 + src/test/ui/const-eval/issue-50600.rs | 20 ++++++++++++++++++++ src/test/ui/const-eval/issue-50600.stderr | 15 +++++++++++++++ 5 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/const-eval/issue-50600.rs create mode 100644 src/test/ui/const-eval/issue-50600.stderr diff --git a/src/librustc_mir/interpret/const_eval.rs b/src/librustc_mir/interpret/const_eval.rs index dff9fa271aba5..9659f1bc31409 100644 --- a/src/librustc_mir/interpret/const_eval.rs +++ b/src/librustc_mir/interpret/const_eval.rs @@ -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 { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 1b8f2e661c30d..1572c65d4d977 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -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) { + span_err!(tcx.sess, span, E0912, "closures cannot be constants"); + } + let substs = ty::ClosureSubsts { substs: Substs::for_item( tcx, diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index e3f7ff5cb3f74..4d7970c31b599 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -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, // closures cannot be constants } diff --git a/src/test/ui/const-eval/issue-50600.rs b/src/test/ui/const-eval/issue-50600.rs new file mode 100644 index 0000000000000..02da13cdf90f6 --- /dev/null +++ b/src/test/ui/const-eval/issue-50600.rs @@ -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 or the MIT license +// , 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 closures cannot be constants +enum Functions { + Square = |x:i32| { }, + //~^ ERROR closures cannot be constants +} + +fn main() {} diff --git a/src/test/ui/const-eval/issue-50600.stderr b/src/test/ui/const-eval/issue-50600.stderr new file mode 100644 index 0000000000000..2a3c4b73d2cd9 --- /dev/null +++ b/src/test/ui/const-eval/issue-50600.stderr @@ -0,0 +1,15 @@ +error[E0912]: closures cannot be constants + --> $DIR/issue-50600.rs:13:18 + | +LL | struct Foo ([u8; |x: u8| { }]); + | ^^^^^^^^^^^ + +error[E0912]: closures cannot be constants + --> $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`. From de038356c4e4d0b4367b3ee42915403049f0e6d5 Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Fri, 11 May 2018 11:34:15 -0300 Subject: [PATCH 2/4] Change error message for closures in constants --- src/librustc_typeck/collect.rs | 2 +- src/librustc_typeck/diagnostics.rs | 2 +- src/test/ui/const-eval/issue-50600.rs | 4 ++-- src/test/ui/const-eval/issue-50600.stderr | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 1572c65d4d977..9a9aa832daa4b 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1092,7 +1092,7 @@ fn type_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } if !tcx.has_typeck_tables(def_id) { - span_err!(tcx.sess, span, E0912, "closures cannot be constants"); + span_err!(tcx.sess, span, E0912, "expected numerical constant, found closure"); } let substs = ty::ClosureSubsts { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 4d7970c31b599..6405b5cdb6b68 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4770,5 +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, // closures cannot be constants + E0912, // expected numerical constant, found closure } diff --git a/src/test/ui/const-eval/issue-50600.rs b/src/test/ui/const-eval/issue-50600.rs index 02da13cdf90f6..e60734f22d8d5 100644 --- a/src/test/ui/const-eval/issue-50600.rs +++ b/src/test/ui/const-eval/issue-50600.rs @@ -11,10 +11,10 @@ // Testing that these do not ICE. struct Foo ([u8; |x: u8| { }]); -//~^ ERROR closures cannot be constants +//~^ ERROR expected numerical constant, found closure enum Functions { Square = |x:i32| { }, - //~^ ERROR closures cannot be constants + //~^ ERROR expected numerical constant, found closure } fn main() {} diff --git a/src/test/ui/const-eval/issue-50600.stderr b/src/test/ui/const-eval/issue-50600.stderr index 2a3c4b73d2cd9..6b9ef21ace02b 100644 --- a/src/test/ui/const-eval/issue-50600.stderr +++ b/src/test/ui/const-eval/issue-50600.stderr @@ -1,10 +1,10 @@ -error[E0912]: closures cannot be constants +error[E0912]: expected numerical constant, found closure --> $DIR/issue-50600.rs:13:18 | LL | struct Foo ([u8; |x: u8| { }]); | ^^^^^^^^^^^ -error[E0912]: closures cannot be constants +error[E0912]: expected numerical constant, found closure --> $DIR/issue-50600.rs:16:14 | LL | Square = |x:i32| { }, From 9c12e324747c6b8f5620489962ac5ac995531e1a Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Fri, 11 May 2018 17:44:30 -0300 Subject: [PATCH 3/4] In def collection attach closure and other sub-exprs to outermost expr of some const exprs --- src/librustc/hir/map/def_collector.rs | 48 +++++++++++-------- src/test/run-pass/ctfe/closure-in-constant.rs | 21 ++++++++ 2 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 src/test/run-pass/ctfe/closure-in-constant.rs diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index ebd8e6235825c..b671b11847204 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -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 { 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) { @@ -268,34 +269,41 @@ 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 { + self.parent_def = 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 + }.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) { diff --git a/src/test/run-pass/ctfe/closure-in-constant.rs b/src/test/run-pass/ctfe/closure-in-constant.rs new file mode 100644 index 0000000000000..6d6e3ad078312 --- /dev/null +++ b/src/test/run-pass/ctfe/closure-in-constant.rs @@ -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 or the MIT license +// , 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. +/* +enum Functions { + Square = (|x:i32| { }, 42).1, +} +*/ + +fn main() {} From 0356813694eb0893ae21523ad082007faebb3e54 Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Sat, 12 May 2018 14:20:59 -0300 Subject: [PATCH 4/4] Address review --- src/librustc/hir/map/def_collector.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index b671b11847204..27cfd702efa1f 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -149,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); } }); @@ -269,7 +271,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { fn visit_expr(&mut self, expr: &'a Expr) { let parent_def = self.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(..) => { @@ -279,8 +281,9 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { expr.span)) } _ => None - }.or(self.parent_def); + }; + self.parent_def = parent.or(self.parent_def); visit::walk_expr(self, expr); self.parent_def = parent_def; }