Skip to content

Commit bba9785

Browse files
committed
Auto merge of #100720 - camsteffen:representable, r=cjgillot
Rewrite representability * Improve placement of `Box` in the suggestion * Multiple items in a cycle emit 1 error instead of an error for each item in the cycle * Introduce `representability` query to avoid traversing an item every time it is used. * Also introduce `params_in_repr` query to avoid traversing generic items every time it is used.
2 parents a688a03 + ff940db commit bba9785

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+529
-736
lines changed

Cargo.lock

-1
Original file line numberDiff line numberDiff line change
@@ -3552,7 +3552,6 @@ dependencies = [
35523552
"rustc_span",
35533553
"rustc_target",
35543554
"rustc_trait_selection",
3555-
"rustc_ty_utils",
35563555
"rustc_type_ir",
35573556
"smallvec",
35583557
"tracing",

compiler/rustc_hir_analysis/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ rustc_span = { path = "../rustc_span" }
2626
rustc_index = { path = "../rustc_index" }
2727
rustc_infer = { path = "../rustc_infer" }
2828
rustc_trait_selection = { path = "../rustc_trait_selection" }
29-
rustc_ty_utils = { path = "../rustc_ty_utils" }
3029
rustc_lint = { path = "../rustc_lint" }
3130
rustc_serialize = { path = "../rustc_serialize" }
3231
rustc_type_ir = { path = "../rustc_type_ir" }

compiler/rustc_hir_analysis/src/check/check.rs

+3-25
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ use rustc_span::{self, Span};
3131
use rustc_target::spec::abi::Abi;
3232
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
3333
use rustc_trait_selection::traits::{self, ObligationCtxt};
34-
use rustc_ty_utils::representability::{self, Representability};
3534

3635
use std::ops::ControlFlow;
3736

@@ -381,7 +380,7 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) {
381380
let def = tcx.adt_def(def_id);
382381
let span = tcx.def_span(def_id);
383382
def.destructor(tcx); // force the destructor to be evaluated
384-
check_representable(tcx, span, def_id);
383+
let _ = tcx.representability(def_id);
385384

386385
if def.repr().simd() {
387386
check_simd(tcx, span, def_id);
@@ -395,7 +394,7 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
395394
let def = tcx.adt_def(def_id);
396395
let span = tcx.def_span(def_id);
397396
def.destructor(tcx); // force the destructor to be evaluated
398-
check_representable(tcx, span, def_id);
397+
let _ = tcx.representability(def_id);
399398
check_transparent(tcx, span, def);
400399
check_union_fields(tcx, span, def_id);
401400
check_packed(tcx, span, def);
@@ -1151,27 +1150,6 @@ fn check_impl_items_against_trait<'tcx>(
11511150
}
11521151
}
11531152

1154-
/// Checks whether a type can be represented in memory. In particular, it
1155-
/// identifies types that contain themselves without indirection through a
1156-
/// pointer, which would mean their size is unbounded.
1157-
pub(super) fn check_representable(tcx: TyCtxt<'_>, sp: Span, item_def_id: LocalDefId) -> bool {
1158-
let rty = tcx.type_of(item_def_id);
1159-
1160-
// Check that it is possible to represent this type. This call identifies
1161-
// (1) types that contain themselves and (2) types that contain a different
1162-
// recursive type. It is only necessary to throw an error on those that
1163-
// contain themselves. For case 2, there must be an inner type that will be
1164-
// caught by case 1.
1165-
match representability::ty_is_representable(tcx, rty, sp, None) {
1166-
Representability::SelfRecursive(spans) => {
1167-
recursive_type_with_infinite_size_error(tcx, item_def_id.to_def_id(), spans);
1168-
return false;
1169-
}
1170-
Representability::Representable | Representability::ContainsRecursive => (),
1171-
}
1172-
true
1173-
}
1174-
11751153
pub fn check_simd(tcx: TyCtxt<'_>, sp: Span, def_id: LocalDefId) {
11761154
let t = tcx.type_of(def_id);
11771155
if let ty::Adt(def, substs) = t.kind()
@@ -1509,7 +1487,7 @@ fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, vs: &'tcx [hir::Variant<'tcx>], def_id: L
15091487

15101488
detect_discriminant_duplicate(tcx, def.discriminants(tcx).collect(), vs, sp);
15111489

1512-
check_representable(tcx, sp, def_id);
1490+
let _ = tcx.representability(def_id);
15131491
check_transparent(tcx, sp, def);
15141492
}
15151493

compiler/rustc_hir_analysis/src/check/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ use rustc_span::{self, BytePos, Span, Symbol};
123123
use rustc_target::abi::VariantIdx;
124124
use rustc_target::spec::abi::Abi;
125125
use rustc_trait_selection::traits;
126-
use rustc_trait_selection::traits::error_reporting::recursive_type_with_infinite_size_error;
127126
use rustc_trait_selection::traits::error_reporting::suggestions::ReturnsVisitor;
128127
use std::cell::RefCell;
129128
use std::num::NonZeroU32;

compiler/rustc_lint_defs/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ macro_rules! pluralize {
2525
($x:expr) => {
2626
if $x != 1 { "s" } else { "" }
2727
};
28+
("has", $x:expr) => {
29+
if $x == 1 { "has" } else { "have" }
30+
};
2831
("is", $x:expr) => {
2932
if $x == 1 { "is" } else { "are" }
3033
};

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+1
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ provide! { tcx, def_id, other, cdata,
210210
lookup_const_stability => { table }
211211
lookup_default_body_stability => { table }
212212
lookup_deprecation_entry => { table }
213+
params_in_repr => { table }
213214
unused_generic_params => { table }
214215
opt_def_kind => { table_direct }
215216
impl_parent => { table }

compiler/rustc_metadata/src/rmeta/encoder.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
11561156
if let DefKind::Trait | DefKind::TraitAlias = def_kind {
11571157
record!(self.tables.super_predicates_of[def_id] <- self.tcx.super_predicates_of(def_id));
11581158
}
1159+
if let DefKind::Enum | DefKind::Struct | DefKind::Union = def_kind {
1160+
let params_in_repr = self.tcx.params_in_repr(def_id);
1161+
record!(self.tables.params_in_repr[def_id] <- params_in_repr);
1162+
}
11591163
if should_encode_trait_impl_trait_tys(tcx, def_id)
11601164
&& let Ok(table) = self.tcx.collect_trait_impl_trait_tys(def_id)
11611165
{

compiler/rustc_metadata/src/rmeta/mod.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ use rustc_hir::def::{CtorKind, DefKind};
1313
use rustc_hir::def_id::{CrateNum, DefId, DefIndex, DefPathHash, StableCrateId};
1414
use rustc_hir::definitions::DefKey;
1515
use rustc_hir::lang_items;
16-
use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec};
16+
use rustc_index::bit_set::{BitSet, FiniteBitSet};
17+
use rustc_index::vec::IndexVec;
1718
use rustc_middle::metadata::ModChild;
1819
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
1920
use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
@@ -383,6 +384,7 @@ define_tables! {
383384
inherent_impls: Table<DefIndex, LazyArray<DefIndex>>,
384385
expn_that_defined: Table<DefIndex, LazyValue<ExpnId>>,
385386
unused_generic_params: Table<DefIndex, LazyValue<FiniteBitSet<u32>>>,
387+
params_in_repr: Table<DefIndex, LazyValue<BitSet<u32>>>,
386388
repr_options: Table<DefIndex, LazyValue<ReprOptions>>,
387389
// `def_keys` and `def_path_hashes` represent a lazy version of a
388390
// `DefPathTable`. This allows us to avoid deserializing an entire

compiler/rustc_middle/src/arena.rs

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ macro_rules! arena_types {
103103
[] dep_kind: rustc_middle::dep_graph::DepKindStruct<'tcx>,
104104

105105
[decode] trait_impl_trait_tys: rustc_data_structures::fx::FxHashMap<rustc_hir::def_id::DefId, rustc_middle::ty::Ty<'tcx>>,
106+
[] bit_set_u32: rustc_index::bit_set::BitSet<u32>,
106107
]);
107108
)
108109
}

compiler/rustc_middle/src/query/mod.rs

+26
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,32 @@ rustc_queries! {
301301
separate_provide_extern
302302
}
303303

304+
/// Checks whether a type is representable or infinitely sized
305+
query representability(_: LocalDefId) -> rustc_middle::ty::Representability {
306+
desc { "checking if {:?} is representable", tcx.def_path_str(key.to_def_id()) }
307+
// infinitely sized types will cause a cycle
308+
cycle_delay_bug
309+
// we don't want recursive representability calls to be forced with
310+
// incremental compilation because, if a cycle occurs, we need the
311+
// entire cycle to be in memory for diagnostics
312+
anon
313+
}
314+
315+
/// An implementation detail for the `representability` query
316+
query representability_adt_ty(_: Ty<'tcx>) -> rustc_middle::ty::Representability {
317+
desc { "checking if {:?} is representable", key }
318+
cycle_delay_bug
319+
anon
320+
}
321+
322+
/// Set of param indexes for type params that are in the type's representation
323+
query params_in_repr(key: DefId) -> rustc_index::bit_set::BitSet<u32> {
324+
desc { "finding type parameters in the representation" }
325+
arena_cache
326+
no_hash
327+
separate_provide_extern
328+
}
329+
304330
/// Fetch the THIR for a given body. If typeck for that body failed, returns an empty `Thir`.
305331
query thir_body(key: ty::WithOptConstParam<LocalDefId>)
306332
-> Result<(&'tcx Steal<thir::Thir<'tcx>>, thir::ExprId), ErrorGuaranteed>

compiler/rustc_middle/src/ty/adt.rs

+7
Original file line numberDiff line numberDiff line change
@@ -566,3 +566,10 @@ impl<'tcx> AdtDef<'tcx> {
566566
ty::EarlyBinder(tcx.adt_sized_constraint(self.did()).0)
567567
}
568568
}
569+
570+
#[derive(Clone, Copy, Debug)]
571+
#[derive(HashStable)]
572+
pub enum Representability {
573+
Representable,
574+
Infinite,
575+
}

compiler/rustc_middle/src/ty/parameterized.rs

+1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ trivially_parameterized_over_tcx! {
8282
rustc_hir::def::DefKind,
8383
rustc_hir::def_id::DefIndex,
8484
rustc_hir::definitions::DefKey,
85+
rustc_index::bit_set::BitSet<u32>,
8586
rustc_index::bit_set::FiniteBitSet<u32>,
8687
rustc_session::cstore::ForeignModule,
8788
rustc_session::cstore::LinkagePreference,

compiler/rustc_middle/src/values.rs

+165-5
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
1-
use rustc_middle::ty::{self, AdtSizedConstraint, Ty, TyCtxt};
1+
use rustc_data_structures::fx::FxHashSet;
2+
use rustc_errors::{pluralize, struct_span_err, Applicability, MultiSpan};
3+
use rustc_hir as hir;
4+
use rustc_hir::def::DefKind;
5+
use rustc_middle::ty::Representability;
6+
use rustc_middle::ty::{self, AdtSizedConstraint, DefIdTree, Ty, TyCtxt};
7+
use rustc_query_system::query::QueryInfo;
28
use rustc_query_system::Value;
9+
use rustc_span::def_id::LocalDefId;
10+
use rustc_span::Span;
11+
12+
use std::fmt::Write;
313

414
impl<'tcx> Value<TyCtxt<'tcx>> for Ty<'_> {
5-
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
15+
fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo]) -> Self {
616
// SAFETY: This is never called when `Self` is not `Ty<'tcx>`.
717
// FIXME: Represent the above fact in the trait system somehow.
818
unsafe { std::mem::transmute::<Ty<'tcx>, Ty<'_>>(tcx.ty_error()) }
919
}
1020
}
1121

1222
impl<'tcx> Value<TyCtxt<'tcx>> for ty::SymbolName<'_> {
13-
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
23+
fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo]) -> Self {
1424
// SAFETY: This is never called when `Self` is not `SymbolName<'tcx>`.
1525
// FIXME: Represent the above fact in the trait system somehow.
1626
unsafe {
@@ -22,7 +32,7 @@ impl<'tcx> Value<TyCtxt<'tcx>> for ty::SymbolName<'_> {
2232
}
2333

2434
impl<'tcx> Value<TyCtxt<'tcx>> for AdtSizedConstraint<'_> {
25-
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
35+
fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo]) -> Self {
2636
// SAFETY: This is never called when `Self` is not `AdtSizedConstraint<'tcx>`.
2737
// FIXME: Represent the above fact in the trait system somehow.
2838
unsafe {
@@ -34,7 +44,7 @@ impl<'tcx> Value<TyCtxt<'tcx>> for AdtSizedConstraint<'_> {
3444
}
3545

3646
impl<'tcx> Value<TyCtxt<'tcx>> for ty::Binder<'_, ty::FnSig<'_>> {
37-
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
47+
fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo]) -> Self {
3848
let err = tcx.ty_error();
3949
// FIXME(compiler-errors): It would be nice if we could get the
4050
// query key, so we could at least generate a fn signature that
@@ -52,3 +62,153 @@ impl<'tcx> Value<TyCtxt<'tcx>> for ty::Binder<'_, ty::FnSig<'_>> {
5262
unsafe { std::mem::transmute::<ty::PolyFnSig<'tcx>, ty::Binder<'_, ty::FnSig<'_>>>(fn_sig) }
5363
}
5464
}
65+
66+
impl<'tcx> Value<TyCtxt<'tcx>> for Representability {
67+
fn from_cycle_error(tcx: TyCtxt<'tcx>, cycle: &[QueryInfo]) -> Self {
68+
let mut item_and_field_ids = Vec::new();
69+
let mut representable_ids = FxHashSet::default();
70+
for info in cycle {
71+
if info.query.name == "representability"
72+
&& let Some(field_id) = info.query.def_id
73+
&& let Some(field_id) = field_id.as_local()
74+
&& let Some(DefKind::Field) = info.query.def_kind
75+
{
76+
let parent_id = tcx.parent(field_id.to_def_id());
77+
let item_id = match tcx.def_kind(parent_id) {
78+
DefKind::Variant => tcx.parent(parent_id),
79+
_ => parent_id,
80+
};
81+
item_and_field_ids.push((item_id.expect_local(), field_id));
82+
}
83+
}
84+
for info in cycle {
85+
if info.query.name == "representability_adt_ty"
86+
&& let Some(def_id) = info.query.ty_adt_id
87+
&& let Some(def_id) = def_id.as_local()
88+
&& !item_and_field_ids.iter().any(|&(id, _)| id == def_id)
89+
{
90+
representable_ids.insert(def_id);
91+
}
92+
}
93+
recursive_type_error(tcx, item_and_field_ids, &representable_ids);
94+
Representability::Infinite
95+
}
96+
}
97+
98+
// item_and_field_ids should form a cycle where each field contains the
99+
// type in the next element in the list
100+
pub fn recursive_type_error(
101+
tcx: TyCtxt<'_>,
102+
mut item_and_field_ids: Vec<(LocalDefId, LocalDefId)>,
103+
representable_ids: &FxHashSet<LocalDefId>,
104+
) {
105+
const ITEM_LIMIT: usize = 5;
106+
107+
// Rotate the cycle so that the item with the lowest span is first
108+
let start_index = item_and_field_ids
109+
.iter()
110+
.enumerate()
111+
.min_by_key(|&(_, &(id, _))| tcx.def_span(id))
112+
.unwrap()
113+
.0;
114+
item_and_field_ids.rotate_left(start_index);
115+
116+
let cycle_len = item_and_field_ids.len();
117+
let show_cycle_len = cycle_len.min(ITEM_LIMIT);
118+
119+
let mut err_span = MultiSpan::from_spans(
120+
item_and_field_ids[..show_cycle_len]
121+
.iter()
122+
.map(|(id, _)| tcx.def_span(id.to_def_id()))
123+
.collect(),
124+
);
125+
let mut suggestion = Vec::with_capacity(show_cycle_len * 2);
126+
for i in 0..show_cycle_len {
127+
let (_, field_id) = item_and_field_ids[i];
128+
let (next_item_id, _) = item_and_field_ids[(i + 1) % cycle_len];
129+
// Find the span(s) that contain the next item in the cycle
130+
let hir_id = tcx.hir().local_def_id_to_hir_id(field_id);
131+
let hir::Node::Field(field) = tcx.hir().get(hir_id) else { bug!("expected field") };
132+
let mut found = Vec::new();
133+
find_item_ty_spans(tcx, field.ty, next_item_id, &mut found, representable_ids);
134+
135+
// Couldn't find the type. Maybe it's behind a type alias?
136+
// In any case, we'll just suggest boxing the whole field.
137+
if found.is_empty() {
138+
found.push(field.ty.span);
139+
}
140+
141+
for span in found {
142+
err_span.push_span_label(span, "recursive without indirection");
143+
// FIXME(compiler-errors): This suggestion might be erroneous if Box is shadowed
144+
suggestion.push((span.shrink_to_lo(), "Box<".to_string()));
145+
suggestion.push((span.shrink_to_hi(), ">".to_string()));
146+
}
147+
}
148+
let items_list = {
149+
let mut s = String::new();
150+
for (i, (item_id, _)) in item_and_field_ids.iter().enumerate() {
151+
let path = tcx.def_path_str(item_id.to_def_id());
152+
write!(&mut s, "`{path}`").unwrap();
153+
if i == (ITEM_LIMIT - 1) && cycle_len > ITEM_LIMIT {
154+
write!(&mut s, " and {} more", cycle_len - 5).unwrap();
155+
break;
156+
}
157+
if cycle_len > 1 && i < cycle_len - 2 {
158+
s.push_str(", ");
159+
} else if cycle_len > 1 && i == cycle_len - 2 {
160+
s.push_str(" and ")
161+
}
162+
}
163+
s
164+
};
165+
let mut err = struct_span_err!(
166+
tcx.sess,
167+
err_span,
168+
E0072,
169+
"recursive type{} {} {} infinite size",
170+
pluralize!(cycle_len),
171+
items_list,
172+
pluralize!("has", cycle_len),
173+
);
174+
err.multipart_suggestion(
175+
"insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle",
176+
suggestion,
177+
Applicability::HasPlaceholders,
178+
);
179+
err.emit();
180+
}
181+
182+
fn find_item_ty_spans(
183+
tcx: TyCtxt<'_>,
184+
ty: &hir::Ty<'_>,
185+
needle: LocalDefId,
186+
spans: &mut Vec<Span>,
187+
seen_representable: &FxHashSet<LocalDefId>,
188+
) {
189+
match ty.kind {
190+
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => {
191+
if let Some(def_id) = path.res.opt_def_id() {
192+
let check_params = def_id.as_local().map_or(true, |def_id| {
193+
if def_id == needle {
194+
spans.push(ty.span);
195+
}
196+
seen_representable.contains(&def_id)
197+
});
198+
if check_params && let Some(args) = path.segments.last().unwrap().args {
199+
let params_in_repr = tcx.params_in_repr(def_id);
200+
for (i, arg) in args.args.iter().enumerate() {
201+
if let hir::GenericArg::Type(ty) = arg && params_in_repr.contains(i as u32) {
202+
find_item_ty_spans(tcx, ty, needle, spans, seen_representable);
203+
}
204+
}
205+
}
206+
}
207+
}
208+
hir::TyKind::Array(ty, _) => find_item_ty_spans(tcx, ty, needle, spans, seen_representable),
209+
hir::TyKind::Tup(tys) => {
210+
tys.iter().for_each(|ty| find_item_ty_spans(tcx, ty, needle, spans, seen_representable))
211+
}
212+
_ => {}
213+
}
214+
}

0 commit comments

Comments
 (0)