From 3045ffa512db4fd9745278e43c8e775db2d0d19d Mon Sep 17 00:00:00 2001 From: Tristan Burgess Date: Wed, 22 Aug 2018 00:02:07 -0400 Subject: [PATCH 1/2] 52985: better cycle error for existential types - Original cycle error diagnostics PR'd against this issue caught panic-causing error while resolving std::mem::transmute calls - Now, catch invalid use case of not providing a concrete sized type behind existential type in definining use case. - Update relevant test to reflect this new error 52985: revert normalize query changes - PR 53588 invalidates 53316, causing a correct cycle error to occur with a good span. - Don't need to revert the whole merge as the test files are still fine, just need to revert the normalize query changes. - It should now be correct that infinite recursion detected during normalize query type folding is a bug, should have been caught earlier (when resolving the existential type's defining use cases). 52985: code review impl - Only cause cycle error if anonymous type resolves to anonymous type that has the same def id (is the same type) as the original (parent) type. - Add test case to cover this case for existential types. 52985: remove Ty prefix from TyAnon - To align with changes per commit 6f637da50c56a22f745fd056691da8c86824cd9b --- src/librustc/traits/query/normalize.rs | 45 ++++--------------- src/librustc_typeck/check/writeback.rs | 8 ++++ .../nested_existential_types.rs | 31 +++++++++++++ .../no_inferrable_concrete_type.stderr | 9 +++- 4 files changed, 55 insertions(+), 38 deletions(-) create mode 100644 src/test/ui/existential_types/nested_existential_types.rs diff --git a/src/librustc/traits/query/normalize.rs b/src/librustc/traits/query/normalize.rs index 7b81989c6415b..45f026c9b68bb 100644 --- a/src/librustc/traits/query/normalize.rs +++ b/src/librustc/traits/query/normalize.rs @@ -12,15 +12,15 @@ //! which folds deeply, invoking the underlying //! `normalize_projection_ty` query when it encounters projections. -use infer::at::At; use infer::{InferCtxt, InferOk}; -use mir::interpret::{ConstValue, GlobalId}; +use infer::at::At; +use mir::interpret::{GlobalId, ConstValue}; use rustc_data_structures::small_vec::SmallVec; -use traits::project::Normalized; use traits::{Obligation, ObligationCause, PredicateObligation, Reveal}; +use traits::project::Normalized; +use ty::{self, Ty, TyCtxt}; use ty::fold::{TypeFoldable, TypeFolder}; use ty::subst::{Subst, Substs}; -use ty::{self, Ty, TyCtxt}; use super::NoSolution; @@ -121,36 +121,9 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for QueryNormalizer<'cx, 'gcx, 'tcx let concrete_ty = generic_ty.subst(self.tcx(), substs); self.anon_depth += 1; if concrete_ty == ty { - // The type in question can only be inferred in terms of itself. This - // is likely a user code issue, not a compiler issue. Thus, we will - // induce a cycle error by calling the parent query again on the type. - // - // FIXME: Perhaps a better solution would be to have fold_ty() - // itself be a query. Then, a type fold cycle would be detected - // and reported more naturally as part of the query system, rather - // than forcing it here. - // - // FIXME: Need a better span than just one pointing to the type def. - // Should point to a defining use of the type that results in this - // un-normalizable state. - if let Some(param_env_lifted) = - self.tcx().lift_to_global(&self.param_env) - { - if let Some(ty_lifted) = self.tcx().lift_to_global(&concrete_ty) { - let span = self.tcx().def_span(def_id); - self.tcx() - .global_tcx() - .at(span) - .normalize_ty_after_erasing_regions( - param_env_lifted.and(ty_lifted), - ); - self.tcx().sess.abort_if_errors(); - } - } - // If a cycle error can't be emitted, indicate a NoSolution error - // and let the caller handle it. - self.error = true; - return concrete_ty; + bug!("infinite recursion generic_ty: {:#?}, substs: {:#?}, \ + concrete_ty: {:#?}, ty: {:#?}", generic_ty, substs, concrete_ty, + ty); } let folded_ty = self.fold_ty(concrete_ty); self.anon_depth -= 1; @@ -176,8 +149,8 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for QueryNormalizer<'cx, 'gcx, 'tcx let gcx = self.infcx.tcx.global_tcx(); let mut orig_values = SmallVec::new(); - let c_data = self.infcx - .canonicalize_query(&self.param_env.and(*data), &mut orig_values); + let c_data = + self.infcx.canonicalize_query(&self.param_env.and(*data), &mut orig_values); debug!("QueryNormalizer: c_data = {:#?}", c_data); debug!("QueryNormalizer: orig_values = {:#?}", orig_values); match gcx.normalize_projection_ty(c_data) { diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 1e147b6c2638d..72103caadf623 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -492,6 +492,14 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { }) }; + if let ty::Anon(defin_ty_def_id, _substs) = definition_ty.sty { + if def_id == defin_ty_def_id { + // Concrete type resolved to the existential type itself + // Force a cycle error + self.tcx().at(span).type_of(defin_ty_def_id); + } + } + let old = self.tables.concrete_existential_types.insert(def_id, definition_ty); if let Some(old) = old { if old != definition_ty { diff --git a/src/test/ui/existential_types/nested_existential_types.rs b/src/test/ui/existential_types/nested_existential_types.rs new file mode 100644 index 0000000000000..aac72c71d0308 --- /dev/null +++ b/src/test/ui/existential_types/nested_existential_types.rs @@ -0,0 +1,31 @@ +// 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. + +#![feature(existential_type)] +// compile-pass +mod my_mod { + use std::fmt::Debug; + + pub existential type Foo: Debug; + pub existential type Foot: Debug; + + pub fn get_foo() -> Foo { + 5i32 + } + + pub fn get_foot() -> Foot { + get_foo() + } +} + +fn main() { + let _: my_mod::Foot = my_mod::get_foot(); +} + diff --git a/src/test/ui/existential_types/no_inferrable_concrete_type.stderr b/src/test/ui/existential_types/no_inferrable_concrete_type.stderr index ffb4f4cc14400..06e40fd6ab554 100644 --- a/src/test/ui/existential_types/no_inferrable_concrete_type.stderr +++ b/src/test/ui/existential_types/no_inferrable_concrete_type.stderr @@ -1,10 +1,15 @@ -error[E0391]: cycle detected when normalizing `ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: All }, value: Foo }` +error[E0391]: cycle detected when processing `Foo` --> $DIR/no_inferrable_concrete_type.rs:16:1 | LL | existential type Foo: Copy; //~ cycle detected | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: ...which again requires normalizing `ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: All }, value: Foo }`, completing the cycle +note: ...which requires processing `bar`... + --> $DIR/no_inferrable_concrete_type.rs:19:23 + | +LL | fn bar(x: Foo) -> Foo { x } + | ^^^^^ + = note: ...which again requires processing `Foo`, completing the cycle error: aborting due to previous error From 7440125d622356291909efd1027162b4983c6f9f Mon Sep 17 00:00:00 2001 From: Tristan Burgess Date: Wed, 22 Aug 2018 00:24:03 -0400 Subject: [PATCH 2/2] 52985: formatting PR files --- src/librustc/traits/query/normalize.rs | 23 +++++--- src/librustc_typeck/check/writeback.rs | 81 ++++++++++++++------------ 2 files changed, 58 insertions(+), 46 deletions(-) diff --git a/src/librustc/traits/query/normalize.rs b/src/librustc/traits/query/normalize.rs index 45f026c9b68bb..821d3229fdc66 100644 --- a/src/librustc/traits/query/normalize.rs +++ b/src/librustc/traits/query/normalize.rs @@ -12,15 +12,15 @@ //! which folds deeply, invoking the underlying //! `normalize_projection_ty` query when it encounters projections. -use infer::{InferCtxt, InferOk}; use infer::at::At; -use mir::interpret::{GlobalId, ConstValue}; +use infer::{InferCtxt, InferOk}; +use mir::interpret::{ConstValue, GlobalId}; use rustc_data_structures::small_vec::SmallVec; -use traits::{Obligation, ObligationCause, PredicateObligation, Reveal}; use traits::project::Normalized; -use ty::{self, Ty, TyCtxt}; +use traits::{Obligation, ObligationCause, PredicateObligation, Reveal}; use ty::fold::{TypeFoldable, TypeFolder}; use ty::subst::{Subst, Substs}; +use ty::{self, Ty, TyCtxt}; use super::NoSolution; @@ -121,9 +121,14 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for QueryNormalizer<'cx, 'gcx, 'tcx let concrete_ty = generic_ty.subst(self.tcx(), substs); self.anon_depth += 1; if concrete_ty == ty { - bug!("infinite recursion generic_ty: {:#?}, substs: {:#?}, \ - concrete_ty: {:#?}, ty: {:#?}", generic_ty, substs, concrete_ty, - ty); + bug!( + "infinite recursion generic_ty: {:#?}, substs: {:#?}, \ + concrete_ty: {:#?}, ty: {:#?}", + generic_ty, + substs, + concrete_ty, + ty + ); } let folded_ty = self.fold_ty(concrete_ty); self.anon_depth -= 1; @@ -149,8 +154,8 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for QueryNormalizer<'cx, 'gcx, 'tcx let gcx = self.infcx.tcx.global_tcx(); let mut orig_values = SmallVec::new(); - let c_data = - self.infcx.canonicalize_query(&self.param_env.and(*data), &mut orig_values); + let c_data = self.infcx + .canonicalize_query(&self.param_env.and(*data), &mut orig_values); debug!("QueryNormalizer: c_data = {:#?}", c_data); debug!("QueryNormalizer: orig_values = {:#?}", orig_values); match gcx.normalize_projection_ty(c_data) { diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 72103caadf623..319de51713b93 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -17,15 +17,15 @@ use rustc::hir; use rustc::hir::def_id::{DefId, DefIndex}; use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc::infer::InferCtxt; +use rustc::ty::adjustment::{Adjust, Adjustment}; +use rustc::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder}; use rustc::ty::subst::UnpackedKind; use rustc::ty::{self, Ty, TyCtxt}; -use rustc::ty::adjustment::{Adjust, Adjustment}; -use rustc::ty::fold::{TypeFoldable, TypeFolder, BottomUpFolder}; use rustc::util::nodemap::DefIdSet; +use rustc_data_structures::sync::Lrc; +use std::mem; use syntax::ast; use syntax_pos::Span; -use std::mem; -use rustc_data_structures::sync::Lrc; /////////////////////////////////////////////////////////////////////////// // Entry point @@ -55,8 +55,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ); debug!( "used_trait_imports({:?}) = {:?}", - item_def_id, - used_trait_imports + item_def_id, used_trait_imports ); wbcx.tables.used_trait_imports = used_trait_imports; @@ -64,8 +63,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { debug!( "writeback: tables for {:?} are {:#?}", - item_def_id, - wbcx.tables + item_def_id, wbcx.tables ); self.tcx.alloc_tables(wbcx.tables) @@ -118,8 +116,8 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { // operating on scalars, we clear the overload. fn fix_scalar_builtin_expr(&mut self, e: &hir::Expr) { match e.node { - hir::ExprKind::Unary(hir::UnNeg, ref inner) | - hir::ExprKind::Unary(hir::UnNot, ref inner) => { + hir::ExprKind::Unary(hir::UnNeg, ref inner) + | hir::ExprKind::Unary(hir::UnNot, ref inner) => { let inner_ty = self.fcx.node_ty(inner.hir_id); let inner_ty = self.fcx.resolve_type_vars_if_possible(&inner_ty); @@ -178,8 +176,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { let index_ty = tables.expr_ty_adjusted(&index); let index_ty = self.fcx.resolve_type_vars_if_possible(&index_ty); - if base_ty.builtin_index().is_some() - && index_ty == self.fcx.tcx.types.usize { + if base_ty.builtin_index().is_some() && index_ty == self.fcx.tcx.types.usize { // Remove the method call record tables.type_dependent_defs_mut().remove(e.hir_id); tables.node_substs_mut().remove(e.hir_id); @@ -191,24 +188,26 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { // of size information - we need to get rid of it // Since this is "after" the other adjustment to be // discarded, we do an extra `pop()` - Some(Adjustment { kind: Adjust::Unsize, .. }) => { + Some(Adjustment { + kind: Adjust::Unsize, + .. + }) => { // So the borrow discard actually happens here a.pop(); - }, + } _ => {} } }); } - }, + } // Might encounter non-valid indexes at this point, so there // has to be a fall-through - _ => {}, + _ => {} } } } } - /////////////////////////////////////////////////////////////////////////// // Impl of Visitor for Resolver // @@ -262,7 +261,9 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> { if let Some(&bm) = self.fcx.tables.borrow().pat_binding_modes().get(p.hir_id) { self.tables.pat_binding_modes_mut().insert(p.hir_id, bm); } else { - self.tcx().sess.delay_span_bug(p.span, "missing binding mode"); + self.tcx() + .sess + .delay_span_bug(p.span, "missing binding mode"); } } hir::PatKind::Struct(_, ref fields, _) => { @@ -310,8 +311,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { }; debug!( "Upvar capture for {:?} resolved to {:?}", - upvar_id, - new_upvar_capture + upvar_id, new_upvar_capture ); self.tables .upvar_capture_map @@ -425,8 +425,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { if subst == ty { // found it in the substitution list, replace with the // parameter from the existential type - return self - .tcx() + return self.tcx() .global_tcx() .mk_ty_param(param.index, param.name); } @@ -464,14 +463,16 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { name: p.name, }; trace!("replace {:?} with {:?}", region, reg); - return self.tcx().global_tcx() + return self.tcx() + .global_tcx() .mk_region(ty::ReEarlyBound(reg)); } } } trace!("anon_defn: {:#?}", anon_defn); trace!("generics: {:#?}", generics); - self.tcx().sess + self.tcx() + .sess .struct_span_err( span, "non-defining existential type use in defining scope", @@ -480,7 +481,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { span, format!( "lifetime `{}` is part of concrete type but not used \ - in parameter list of existential type", + in parameter list of existential type", region, ), ) @@ -488,25 +489,27 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { self.tcx().global_tcx().mk_region(ty::ReStatic) } } - } + }, }) }; if let ty::Anon(defin_ty_def_id, _substs) = definition_ty.sty { if def_id == defin_ty_def_id { - // Concrete type resolved to the existential type itself - // Force a cycle error - self.tcx().at(span).type_of(defin_ty_def_id); + // Concrete type resolved to the existential type itself + // Force a cycle error + self.tcx().at(span).type_of(defin_ty_def_id); } } - let old = self.tables.concrete_existential_types.insert(def_id, definition_ty); + let old = self.tables + .concrete_existential_types + .insert(def_id, definition_ty); if let Some(old) = old { if old != definition_ty { span_bug!( span, "visit_anon_types tried to write \ - different types for the same existential type: {:?}, {:?}, {:?}", + different types for the same existential type: {:?}, {:?}, {:?}", def_id, definition_ty, old, @@ -518,7 +521,12 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { fn visit_field_id(&mut self, node_id: ast::NodeId) { let hir_id = self.tcx().hir.node_to_hir_id(node_id); - if let Some(index) = self.fcx.tables.borrow_mut().field_indices_mut().remove(hir_id) { + if let Some(index) = self.fcx + .tables + .borrow_mut() + .field_indices_mut() + .remove(hir_id) + { self.tables.field_indices_mut().insert(hir_id, index); } } @@ -567,8 +575,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { let resolved_adjustment = self.resolve(&adjustment, &span); debug!( "Adjustments for node {:?}: {:?}", - hir_id, - resolved_adjustment + hir_id, resolved_adjustment ); self.tables .adjustments_mut() @@ -592,8 +599,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { let resolved_adjustment = self.resolve(&adjustment, &span); debug!( "pat_adjustments for node {:?}: {:?}", - hir_id, - resolved_adjustment + hir_id, resolved_adjustment ); self.tables .pat_adjustments_mut() @@ -709,7 +715,8 @@ impl<'cx, 'gcx, 'tcx> Resolver<'cx, 'gcx, 'tcx> { fn report_error(&self, t: Ty<'tcx>) { if !self.tcx.sess.has_errors() { self.infcx - .need_type_info_err(Some(self.body.id()), self.span.to_span(&self.tcx), t).emit(); + .need_type_info_err(Some(self.body.id()), self.span.to_span(&self.tcx), t) + .emit(); } } }