Skip to content

Commit d2a00da

Browse files
VeykrilVeykril
authored
Merge pull request #21569 from ChayimFriedman2/parens-multi-impl-trait
fix: Cover more cases where we need parentheses in `&(impl Trait1 + Trait2)`
2 parents ccd3924 + b6319a8 commit d2a00da

6 files changed

Lines changed: 118 additions & 108 deletions

File tree

‎crates/hir-ty/src/display.rs‎

Lines changed: 95 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use either::Either;
1212
use hir_def::{
1313
FindPathConfig, GenericDefId, GenericParamId, HasModule, LocalFieldId, Lookup, ModuleDefId,
1414
ModuleId, TraitId,
15-
db::DefDatabase,
1615
expr_store::{ExpressionStore, path::Path},
1716
find_path::{self, PrefixKind},
1817
hir::generics::{TypeOrConstParamData, TypeParamProvenance, WherePredicate},
@@ -100,6 +99,9 @@ pub struct HirFormatter<'a, 'db> {
10099
display_kind: DisplayKind,
101100
display_target: DisplayTarget,
102101
bounds_formatting_ctx: BoundsFormattingCtx<'db>,
102+
/// Whether formatting `impl Trait1 + Trait2` or `dyn Trait1 + Trait2` needs parentheses around it,
103+
/// for example when formatting `&(impl Trait1 + Trait2)`.
104+
trait_bounds_need_parens: bool,
103105
}
104106

105107
// FIXME: To consider, ref and dyn trait lifetimes can be omitted if they are `'_`, path args should
@@ -331,6 +333,7 @@ pub trait HirDisplay<'db> {
331333
show_container_bounds: false,
332334
display_lifetimes: DisplayLifetime::OnlyNamedOrStatic,
333335
bounds_formatting_ctx: Default::default(),
336+
trait_bounds_need_parens: false,
334337
}) {
335338
Ok(()) => {}
336339
Err(HirDisplayError::FmtError) => panic!("Writing to String can't fail!"),
@@ -566,6 +569,7 @@ impl<'db, T: HirDisplay<'db>> HirDisplayWrapper<'_, 'db, T> {
566569
show_container_bounds: self.show_container_bounds,
567570
display_lifetimes: self.display_lifetimes,
568571
bounds_formatting_ctx: Default::default(),
572+
trait_bounds_need_parens: false,
569573
})
570574
}
571575

@@ -612,7 +616,11 @@ impl<'db, T: HirDisplay<'db> + Internable> HirDisplay<'db> for Interned<T> {
612616
}
613617
}
614618

615-
fn write_projection<'db>(f: &mut HirFormatter<'_, 'db>, alias: &AliasTy<'db>) -> Result {
619+
fn write_projection<'db>(
620+
f: &mut HirFormatter<'_, 'db>,
621+
alias: &AliasTy<'db>,
622+
needs_parens_if_multi: bool,
623+
) -> Result {
616624
if f.should_truncate() {
617625
return write!(f, "{TYPE_HINT_TRUNCATION}");
618626
}
@@ -650,6 +658,7 @@ fn write_projection<'db>(f: &mut HirFormatter<'_, 'db>, alias: &AliasTy<'db>) ->
650658
Either::Left(Ty::new_alias(f.interner, AliasTyKind::Projection, *alias)),
651659
&bounds,
652660
SizedByDefault::NotSized,
661+
needs_parens_if_multi,
653662
)
654663
});
655664
}
@@ -1056,7 +1065,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
10561065
return write!(f, "{TYPE_HINT_TRUNCATION}");
10571066
}
10581067

1059-
use TyKind;
1068+
let trait_bounds_need_parens = mem::replace(&mut f.trait_bounds_need_parens, false);
10601069
match self.kind() {
10611070
TyKind::Never => write!(f, "!")?,
10621071
TyKind::Str => write!(f, "str")?,
@@ -1077,103 +1086,34 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
10771086
c.hir_fmt(f)?;
10781087
write!(f, "]")?;
10791088
}
1080-
kind @ (TyKind::RawPtr(t, m) | TyKind::Ref(_, t, m)) => {
1081-
if let TyKind::Ref(l, _, _) = kind {
1082-
f.write_char('&')?;
1083-
if f.render_region(l) {
1084-
l.hir_fmt(f)?;
1085-
f.write_char(' ')?;
1086-
}
1087-
match m {
1088-
rustc_ast_ir::Mutability::Not => (),
1089-
rustc_ast_ir::Mutability::Mut => f.write_str("mut ")?,
1090-
}
1091-
} else {
1092-
write!(
1093-
f,
1094-
"*{}",
1095-
match m {
1096-
rustc_ast_ir::Mutability::Not => "const ",
1097-
rustc_ast_ir::Mutability::Mut => "mut ",
1098-
}
1099-
)?;
1089+
TyKind::Ref(l, t, m) => {
1090+
f.write_char('&')?;
1091+
if f.render_region(l) {
1092+
l.hir_fmt(f)?;
1093+
f.write_char(' ')?;
1094+
}
1095+
match m {
1096+
rustc_ast_ir::Mutability::Not => (),
1097+
rustc_ast_ir::Mutability::Mut => f.write_str("mut ")?,
11001098
}
11011099

1102-
// FIXME: all this just to decide whether to use parentheses...
1103-
let (preds_to_print, has_impl_fn_pred) = match t.kind() {
1104-
TyKind::Dynamic(bounds, region) => {
1105-
let contains_impl_fn =
1106-
bounds.iter().any(|bound| match bound.skip_binder() {
1107-
ExistentialPredicate::Trait(trait_ref) => {
1108-
let trait_ = trait_ref.def_id.0;
1109-
fn_traits(f.lang_items()).any(|it| it == trait_)
1110-
}
1111-
_ => false,
1112-
});
1113-
let render_lifetime = f.render_region(region);
1114-
(bounds.len() + render_lifetime as usize, contains_impl_fn)
1115-
}
1116-
TyKind::Alias(AliasTyKind::Opaque, ty) => {
1117-
let opaque_ty_id = match ty.def_id {
1118-
SolverDefId::InternedOpaqueTyId(id) => id,
1119-
_ => unreachable!(),
1120-
};
1121-
let impl_trait_id = db.lookup_intern_impl_trait_id(opaque_ty_id);
1122-
if let ImplTraitId::ReturnTypeImplTrait(func, _) = impl_trait_id {
1123-
let data = impl_trait_id.predicates(db);
1124-
let bounds =
1125-
|| data.iter_instantiated_copied(f.interner, ty.args.as_slice());
1126-
let mut len = bounds().count();
1127-
1128-
// Don't count Sized but count when it absent
1129-
// (i.e. when explicit ?Sized bound is set).
1130-
let default_sized = SizedByDefault::Sized { anchor: func.krate(db) };
1131-
let sized_bounds = bounds()
1132-
.filter(|b| {
1133-
matches!(
1134-
b.kind().skip_binder(),
1135-
ClauseKind::Trait(trait_ref)
1136-
if default_sized.is_sized_trait(
1137-
trait_ref.def_id().0,
1138-
db,
1139-
),
1140-
)
1141-
})
1142-
.count();
1143-
match sized_bounds {
1144-
0 => len += 1,
1145-
_ => {
1146-
len = len.saturating_sub(sized_bounds);
1147-
}
1148-
}
1149-
1150-
let contains_impl_fn = bounds().any(|bound| {
1151-
if let ClauseKind::Trait(trait_ref) = bound.kind().skip_binder() {
1152-
let trait_ = trait_ref.def_id().0;
1153-
fn_traits(f.lang_items()).any(|it| it == trait_)
1154-
} else {
1155-
false
1156-
}
1157-
});
1158-
(len, contains_impl_fn)
1159-
} else {
1160-
(0, false)
1161-
}
1100+
f.trait_bounds_need_parens = true;
1101+
t.hir_fmt(f)?;
1102+
f.trait_bounds_need_parens = false;
1103+
}
1104+
TyKind::RawPtr(t, m) => {
1105+
write!(
1106+
f,
1107+
"*{}",
1108+
match m {
1109+
rustc_ast_ir::Mutability::Not => "const ",
1110+
rustc_ast_ir::Mutability::Mut => "mut ",
11621111
}
1163-
_ => (0, false),
1164-
};
1165-
1166-
if has_impl_fn_pred && preds_to_print <= 2 {
1167-
return t.hir_fmt(f);
1168-
}
1112+
)?;
11691113

1170-
if preds_to_print > 1 {
1171-
write!(f, "(")?;
1172-
t.hir_fmt(f)?;
1173-
write!(f, ")")?;
1174-
} else {
1175-
t.hir_fmt(f)?;
1176-
}
1114+
f.trait_bounds_need_parens = true;
1115+
t.hir_fmt(f)?;
1116+
f.trait_bounds_need_parens = false;
11771117
}
11781118
TyKind::Tuple(tys) => {
11791119
if tys.len() == 1 {
@@ -1328,7 +1268,9 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
13281268

13291269
hir_fmt_generics(f, parameters.as_slice(), Some(def.def_id().0.into()), None)?;
13301270
}
1331-
TyKind::Alias(AliasTyKind::Projection, alias_ty) => write_projection(f, &alias_ty)?,
1271+
TyKind::Alias(AliasTyKind::Projection, alias_ty) => {
1272+
write_projection(f, &alias_ty, trait_bounds_need_parens)?
1273+
}
13321274
TyKind::Foreign(alias) => {
13331275
let type_alias = db.type_alias_signature(alias.0);
13341276
f.start_location_link(alias.0.into());
@@ -1363,6 +1305,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
13631305
Either::Left(*self),
13641306
&bounds,
13651307
SizedByDefault::Sized { anchor: krate },
1308+
trait_bounds_need_parens,
13661309
)?;
13671310
}
13681311
TyKind::Closure(id, substs) => {
@@ -1525,6 +1468,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
15251468
Either::Left(*self),
15261469
&bounds,
15271470
SizedByDefault::Sized { anchor: krate },
1471+
trait_bounds_need_parens,
15281472
)?;
15291473
}
15301474
},
@@ -1567,6 +1511,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
15671511
Either::Left(*self),
15681512
&bounds_to_display,
15691513
SizedByDefault::NotSized,
1514+
trait_bounds_need_parens,
15701515
)?;
15711516
}
15721517
TyKind::Error(_) => {
@@ -1806,11 +1751,11 @@ pub enum SizedByDefault {
18061751
}
18071752

18081753
impl SizedByDefault {
1809-
fn is_sized_trait(self, trait_: TraitId, db: &dyn DefDatabase) -> bool {
1754+
fn is_sized_trait(self, trait_: TraitId, interner: DbInterner<'_>) -> bool {
18101755
match self {
18111756
Self::NotSized => false,
1812-
Self::Sized { anchor } => {
1813-
let sized_trait = hir_def::lang_item::lang_items(db, anchor).Sized;
1757+
Self::Sized { .. } => {
1758+
let sized_trait = interner.lang_items().Sized;
18141759
Some(trait_) == sized_trait
18151760
}
18161761
}
@@ -1823,16 +1768,62 @@ pub fn write_bounds_like_dyn_trait_with_prefix<'db>(
18231768
this: Either<Ty<'db>, Region<'db>>,
18241769
predicates: &[Clause<'db>],
18251770
default_sized: SizedByDefault,
1771+
needs_parens_if_multi: bool,
18261772
) -> Result {
1773+
let needs_parens =
1774+
needs_parens_if_multi && trait_bounds_need_parens(f, this, predicates, default_sized);
1775+
if needs_parens {
1776+
write!(f, "(")?;
1777+
}
18271778
write!(f, "{prefix}")?;
18281779
if !predicates.is_empty()
18291780
|| predicates.is_empty() && matches!(default_sized, SizedByDefault::Sized { .. })
18301781
{
18311782
write!(f, " ")?;
1832-
write_bounds_like_dyn_trait(f, this, predicates, default_sized)
1833-
} else {
1834-
Ok(())
1783+
write_bounds_like_dyn_trait(f, this, predicates, default_sized)?;
1784+
}
1785+
if needs_parens {
1786+
write!(f, ")")?;
1787+
}
1788+
Ok(())
1789+
}
1790+
1791+
fn trait_bounds_need_parens<'db>(
1792+
f: &mut HirFormatter<'_, 'db>,
1793+
this: Either<Ty<'db>, Region<'db>>,
1794+
predicates: &[Clause<'db>],
1795+
default_sized: SizedByDefault,
1796+
) -> bool {
1797+
// This needs to be kept in sync with `write_bounds_like_dyn_trait()`.
1798+
let mut distinct_bounds = 0usize;
1799+
let mut is_sized = false;
1800+
for p in predicates {
1801+
match p.kind().skip_binder() {
1802+
ClauseKind::Trait(trait_ref) => {
1803+
let trait_ = trait_ref.def_id().0;
1804+
if default_sized.is_sized_trait(trait_, f.interner) {
1805+
is_sized = true;
1806+
if matches!(default_sized, SizedByDefault::Sized { .. }) {
1807+
// Don't print +Sized, but rather +?Sized if absent.
1808+
continue;
1809+
}
1810+
}
1811+
1812+
distinct_bounds += 1;
1813+
}
1814+
ClauseKind::TypeOutlives(to) if Either::Left(to.0) == this => distinct_bounds += 1,
1815+
ClauseKind::RegionOutlives(lo) if Either::Right(lo.0) == this => distinct_bounds += 1,
1816+
_ => {}
1817+
}
18351818
}
1819+
1820+
if let SizedByDefault::Sized { .. } = default_sized
1821+
&& !is_sized
1822+
{
1823+
distinct_bounds += 1;
1824+
}
1825+
1826+
distinct_bounds > 1
18361827
}
18371828

18381829
fn write_bounds_like_dyn_trait<'db>(
@@ -1855,7 +1846,7 @@ fn write_bounds_like_dyn_trait<'db>(
18551846
match p.kind().skip_binder() {
18561847
ClauseKind::Trait(trait_ref) => {
18571848
let trait_ = trait_ref.def_id().0;
1858-
if default_sized.is_sized_trait(trait_, f.db) {
1849+
if default_sized.is_sized_trait(trait_, f.interner) {
18591850
is_sized = true;
18601851
if matches!(default_sized, SizedByDefault::Sized { .. }) {
18611852
// Don't print +Sized, but rather +?Sized if absent.

‎crates/hir-ty/src/tests/coercion.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ trait Foo {}
608608
fn test(f: impl Foo, g: &(impl Foo + ?Sized)) {
609609
let _: &dyn Foo = &f;
610610
let _: &dyn Foo = g;
611-
//^ expected &'? (dyn Foo + 'static), got &'? impl Foo + ?Sized
611+
//^ expected &'? (dyn Foo + 'static), got &'? (impl Foo + ?Sized)
612612
}
613613
"#,
614614
);

‎crates/hir-ty/src/tests/display_source_code.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ fn test(
111111
b;
112112
//^ impl Foo
113113
c;
114-
//^ &impl Foo + ?Sized
114+
//^ &(impl Foo + ?Sized)
115115
d;
116116
//^ S<impl Foo>
117117
ref_any;
@@ -192,7 +192,7 @@ fn test(
192192
b;
193193
//^ fn(impl Foo) -> impl Foo
194194
c;
195-
} //^ fn(&impl Foo + ?Sized) -> &impl Foo + ?Sized
195+
} //^ fn(&(impl Foo + ?Sized)) -> &(impl Foo + ?Sized)
196196
"#,
197197
);
198198
}

‎crates/hir-ty/src/tests/traits.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4821,7 +4821,7 @@ fn allowed3(baz: impl Baz<Assoc = Qux<impl Foo>>) {}
48214821
431..433 '{}': ()
48224822
447..450 'baz': impl Baz<Assoc = impl Foo>
48234823
480..482 '{}': ()
4824-
500..503 'baz': impl Baz<Assoc = &'a impl Foo + 'a>
4824+
500..503 'baz': impl Baz<Assoc = &'a (impl Foo + 'a)>
48254825
544..546 '{}': ()
48264826
560..563 'baz': impl Baz<Assoc = Qux<impl Foo>>
48274827
598..600 '{}': ()

‎crates/hir/src/display.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,7 @@ impl<'db> HirDisplay<'db> for TypeParam {
587587
Either::Left(ty),
588588
&predicates,
589589
SizedByDefault::Sized { anchor: krate },
590+
false,
590591
);
591592
}
592593
},
@@ -614,6 +615,7 @@ impl<'db> HirDisplay<'db> for TypeParam {
614615
Either::Left(ty),
615616
&predicates,
616617
default_sized,
618+
false,
617619
)?;
618620
}
619621
Ok(())

‎crates/ide/src/inlay_hints/bind_pat.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,4 +1382,21 @@ fn f<'a>() {
13821382
"#]],
13831383
);
13841384
}
1385+
1386+
#[test]
1387+
fn ref_multi_trait_impl_trait() {
1388+
check_with_config(
1389+
InlayHintsConfig { type_hints: true, ..DISABLED_CONFIG },
1390+
r#"
1391+
//- minicore: sized
1392+
trait Eq {}
1393+
trait Ord {}
1394+
1395+
fn foo(argument: &(impl Eq + Ord)) {
1396+
let x = argument;
1397+
// ^ &(impl Eq + Ord)
1398+
}
1399+
"#,
1400+
);
1401+
}
13851402
}

0 commit comments

Comments
 (0)