From 98728c2b35da6600e15d57e494041dcf6dc21c1a Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Mon, 10 May 2021 17:47:50 +0200 Subject: [PATCH] Implement changes suggested by tmiasko and davidtwco --- .../rustc_ty_utils/src/representability.rs | 85 +++++++++---------- .../struct-rec}/issue-74224.rs | 0 .../struct-rec}/issue-74224.stderr | 0 .../struct-rec}/issue-84611.rs | 0 .../struct-rec}/issue-84611.stderr | 0 .../struct-rec}/mutual-struct-recursion.rs | 0 .../mutual-struct-recursion.stderr | 0 7 files changed, 42 insertions(+), 43 deletions(-) rename src/test/ui/{issues => structs-enums/struct-rec}/issue-74224.rs (100%) rename src/test/ui/{issues => structs-enums/struct-rec}/issue-74224.stderr (100%) rename src/test/ui/{issues => structs-enums/struct-rec}/issue-84611.rs (100%) rename src/test/ui/{issues => structs-enums/struct-rec}/issue-84611.stderr (100%) rename src/test/ui/{ => structs-enums/struct-rec}/mutual-struct-recursion.rs (100%) rename src/test/ui/{ => structs-enums/struct-rec}/mutual-struct-recursion.stderr (100%) diff --git a/compiler/rustc_ty_utils/src/representability.rs b/compiler/rustc_ty_utils/src/representability.rs index bf8d6335dad..d3eb9fd9557 100644 --- a/compiler/rustc_ty_utils/src/representability.rs +++ b/compiler/rustc_ty_utils/src/representability.rs @@ -25,12 +25,17 @@ pub enum Representability { pub fn ty_is_representable<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, sp: Span) -> Representability { debug!("is_type_representable: {:?}", ty); // To avoid a stack overflow when checking an enum variant or struct that - // contains a different, structurally recursive type, maintain a stack - // of seen types and check recursion for each of them (issues #3008, #3779). + // contains a different, structurally recursive type, maintain a stack of + // seen types and check recursion for each of them (issues #3008, #3779, + // #74224, #84611). `shadow_seen` contains the full stack and `seen` only + // the one for the current type (e.g. if we have structs A and B, B contains + // a field of type A, and we're currently looking at B, then `seen` will be + // cleared when recursing to check A, but `shadow_seen` won't, so that we + // can catch cases of mutual recursion where A also contains B). let mut seen: Vec> = Vec::new(); - let mut shadow_seen: Vec> = Vec::new(); + let mut shadow_seen: Vec<&'tcx ty::AdtDef> = Vec::new(); let mut representable_cache = FxHashMap::default(); - let mut f_res = false; + let mut force_result = false; let r = is_type_structurally_recursive( tcx, sp, @@ -38,7 +43,7 @@ pub fn ty_is_representable<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, sp: Span) -> R &mut shadow_seen, &mut representable_cache, ty, - &mut f_res, + &mut force_result, ); debug!("is_type_representable: {:?} is {:?}", ty, r); r @@ -58,10 +63,10 @@ fn are_inner_types_recursive<'tcx>( tcx: TyCtxt<'tcx>, sp: Span, seen: &mut Vec>, - shadow_seen: &mut Vec>, + shadow_seen: &mut Vec<&'tcx ty::AdtDef>, representable_cache: &mut FxHashMap, Representability>, ty: Ty<'tcx>, - f_res: &mut bool, + force_result: &mut bool, ) -> Representability { debug!("are_inner_types_recursive({:?}, {:?}, {:?})", ty, seen, shadow_seen); match ty.kind() { @@ -75,7 +80,7 @@ fn are_inner_types_recursive<'tcx>( shadow_seen, representable_cache, ty, - f_res, + force_result, ) })) } @@ -88,7 +93,7 @@ fn are_inner_types_recursive<'tcx>( shadow_seen, representable_cache, ty, - f_res, + force_result, ), ty::Adt(def, substs) => { // Find non representable fields with their spans @@ -125,22 +130,12 @@ fn are_inner_types_recursive<'tcx>( // case (shadow_seen.first() is the type we are originally // interested in, and if we ever encounter the same AdtDef again, // we know that it must be SelfRecursive) and "forcibly" returning - // SelfRecursive (by setting f_res, which tells the calling + // SelfRecursive (by setting force_result, which tells the calling // invocations of are_inner_types_representable to forward the // result without adjusting). - if shadow_seen.len() > 1 && shadow_seen.len() > seen.len() { - match shadow_seen.first().map(|ty| ty.kind()) { - Some(ty::Adt(f_def, _)) => { - if f_def == def { - *f_res = true; - result = Some(Representability::SelfRecursive(vec![span])); - } - } - Some(_) => { - bug!("shadow_seen stack contains non-ADT type: {:?}", ty); - } - None => unreachable!(), - } + if shadow_seen.len() > seen.len() && shadow_seen.first() == Some(def) { + *force_result = true; + result = Some(Representability::SelfRecursive(vec![span])); } if result == None { @@ -154,15 +149,11 @@ fn are_inner_types_recursive<'tcx>( // If we have encountered an ADT definition that we have not seen // before (no need to check them twice), recurse to see whether that // definition is SelfRecursive. If so, we must be ContainsRecursive. - if shadow_seen.iter().len() > 1 - && !shadow_seen.iter().take(shadow_seen.iter().len() - 1).any(|seen_ty| { - match seen_ty.kind() { - ty::Adt(seen_def, _) => seen_def == def, - _ => { - bug!("seen stack contains non-ADT type: {:?}", seen_ty); - } - } - }) + if shadow_seen.len() > 1 + && !shadow_seen + .iter() + .take(shadow_seen.len() - 1) + .any(|seen_def| seen_def == def) { let adt_def_id = def.did; let raw_adt_ty = tcx.type_of(adt_def_id); @@ -180,10 +171,10 @@ fn are_inner_types_recursive<'tcx>( shadow_seen, representable_cache, raw_adt_ty, - f_res, + force_result, ) { Representability::SelfRecursive(_) => { - if *f_res { + if *force_result { Representability::SelfRecursive(vec![span]) } else { Representability::ContainsRecursive @@ -227,7 +218,7 @@ fn are_inner_types_recursive<'tcx>( shadow_seen, representable_cache, ty, - f_res, + force_result, ) { Representability::SelfRecursive(_) => { Representability::SelfRecursive(vec![span]) @@ -263,10 +254,10 @@ fn is_type_structurally_recursive<'tcx>( tcx: TyCtxt<'tcx>, sp: Span, seen: &mut Vec>, - shadow_seen: &mut Vec>, + shadow_seen: &mut Vec<&'tcx ty::AdtDef>, representable_cache: &mut FxHashMap, Representability>, ty: Ty<'tcx>, - f_res: &mut bool, + force_result: &mut bool, ) -> Representability { debug!("is_type_structurally_recursive: {:?} {:?}", ty, sp); if let Some(representability) = representable_cache.get(ty) { @@ -284,7 +275,7 @@ fn is_type_structurally_recursive<'tcx>( shadow_seen, representable_cache, ty, - f_res, + force_result, ); representable_cache.insert(ty, representability.clone()); @@ -295,10 +286,10 @@ fn is_type_structurally_recursive_inner<'tcx>( tcx: TyCtxt<'tcx>, sp: Span, seen: &mut Vec>, - shadow_seen: &mut Vec>, + shadow_seen: &mut Vec<&'tcx ty::AdtDef>, representable_cache: &mut FxHashMap, Representability>, ty: Ty<'tcx>, - f_res: &mut bool, + force_result: &mut bool, ) -> Representability { match ty.kind() { ty::Adt(def, _) => { @@ -346,7 +337,7 @@ fn is_type_structurally_recursive_inner<'tcx>( // For structs and enums, track all previously seen types by pushing them // onto the 'seen' stack. seen.push(ty); - shadow_seen.push(ty); + shadow_seen.push(def); let out = are_inner_types_recursive( tcx, sp, @@ -354,7 +345,7 @@ fn is_type_structurally_recursive_inner<'tcx>( shadow_seen, representable_cache, ty, - f_res, + force_result, ); shadow_seen.pop(); seen.pop(); @@ -362,7 +353,15 @@ fn is_type_structurally_recursive_inner<'tcx>( } _ => { // No need to push in other cases. - are_inner_types_recursive(tcx, sp, seen, shadow_seen, representable_cache, ty, f_res) + are_inner_types_recursive( + tcx, + sp, + seen, + shadow_seen, + representable_cache, + ty, + force_result, + ) } } } diff --git a/src/test/ui/issues/issue-74224.rs b/src/test/ui/structs-enums/struct-rec/issue-74224.rs similarity index 100% rename from src/test/ui/issues/issue-74224.rs rename to src/test/ui/structs-enums/struct-rec/issue-74224.rs diff --git a/src/test/ui/issues/issue-74224.stderr b/src/test/ui/structs-enums/struct-rec/issue-74224.stderr similarity index 100% rename from src/test/ui/issues/issue-74224.stderr rename to src/test/ui/structs-enums/struct-rec/issue-74224.stderr diff --git a/src/test/ui/issues/issue-84611.rs b/src/test/ui/structs-enums/struct-rec/issue-84611.rs similarity index 100% rename from src/test/ui/issues/issue-84611.rs rename to src/test/ui/structs-enums/struct-rec/issue-84611.rs diff --git a/src/test/ui/issues/issue-84611.stderr b/src/test/ui/structs-enums/struct-rec/issue-84611.stderr similarity index 100% rename from src/test/ui/issues/issue-84611.stderr rename to src/test/ui/structs-enums/struct-rec/issue-84611.stderr diff --git a/src/test/ui/mutual-struct-recursion.rs b/src/test/ui/structs-enums/struct-rec/mutual-struct-recursion.rs similarity index 100% rename from src/test/ui/mutual-struct-recursion.rs rename to src/test/ui/structs-enums/struct-rec/mutual-struct-recursion.rs diff --git a/src/test/ui/mutual-struct-recursion.stderr b/src/test/ui/structs-enums/struct-rec/mutual-struct-recursion.stderr similarity index 100% rename from src/test/ui/mutual-struct-recursion.stderr rename to src/test/ui/structs-enums/struct-rec/mutual-struct-recursion.stderr