Skip to content

Commit f9b8d8e

Browse files
alexregalexreg
committed
Lint duplicate auto traits in trait objects.
1 parent 70fafb1 commit f9b8d8e

File tree

12 files changed

+261
-37
lines changed

12 files changed

+261
-37
lines changed

‎src/librustc/lint/builtin.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,12 @@ declare_lint! {
352352
"outlives requirements can be inferred"
353353
}
354354

355+
declare_lint! {
356+
pub DUPLICATE_AUTO_TRAITS_IN_TRAIT_OBJECTS,
357+
Warn,
358+
"duplicate auto traits in trait object bounds"
359+
}
360+
355361
/// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`.
356362
pub mod parser {
357363
declare_lint! {
@@ -430,6 +436,7 @@ impl LintPass for HardwiredLints {
430436
PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
431437
MACRO_USE_EXTERN_CRATE,
432438
MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS,
439+
DUPLICATE_AUTO_TRAITS_IN_TRAIT_OBJECTS,
433440
parser::QUESTION_MARK_MACRO_SEP,
434441
DEPRECATED_IN_FUTURE,
435442
)

‎src/librustc/traits/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub use self::engine::{TraitEngine, TraitEngineExt};
6161
pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs};
6262
pub use self::util::{supertraits, supertrait_def_ids, transitive_bounds,
6363
Supertraits, SupertraitDefIds};
64-
pub use self::util::{expand_trait_refs, TraitRefExpander};
64+
pub use self::util::{expand_trait_refs, TraitRefExpander, TraitRefExpansionInfoDignosticBuilder};
6565

6666
pub use self::chalk_fulfill::{
6767
CanonicalGoal as ChalkCanonicalGoal,

‎src/librustc/traits/util.rs

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use errors::DiagnosticBuilder;
12
use hir;
23
use hir::def_id::DefId;
34
use smallvec::SmallVec;
@@ -305,6 +306,26 @@ impl<'tcx> TraitRefExpansionInfo<'tcx> {
305306
}
306307
}
307308

309+
pub trait TraitRefExpansionInfoDignosticBuilder {
310+
fn label_with_exp_info<'tcx>(&mut self,
311+
info: &TraitRefExpansionInfo<'tcx>,
312+
top_label: &str) -> &mut Self;
313+
}
314+
315+
impl<'a> TraitRefExpansionInfoDignosticBuilder for DiagnosticBuilder<'a> {
316+
fn label_with_exp_info<'tcx>(&mut self,
317+
info: &TraitRefExpansionInfo<'tcx>,
318+
top_label: &str) -> &mut Self {
319+
self.span_label(info.top().1, top_label);
320+
if info.items.len() > 1 {
321+
for (_, sp) in info.items[1..(info.items.len() - 1)].iter().rev() {
322+
self.span_label(*sp, "referenced here");
323+
}
324+
}
325+
self
326+
}
327+
}
328+
308329
pub fn expand_trait_refs<'cx, 'gcx, 'tcx>(
309330
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
310331
trait_refs: impl IntoIterator<Item = (ty::PolyTraitRef<'tcx>, Span)>
@@ -314,7 +335,12 @@ pub fn expand_trait_refs<'cx, 'gcx, 'tcx>(
314335
.into_iter()
315336
.map(|(tr, sp)| TraitRefExpansionInfo::new(tr, sp))
316337
.collect();
317-
items.retain(|i| visited.insert(&i.trait_ref().to_predicate()));
338+
// Note: we also retain auto traits here for the purpose of linting duplicate auto traits
339+
// in the `AstConv::conv_object_ty_poly_trait_ref` function.
340+
items.retain(|i| {
341+
let trait_ref = i.trait_ref();
342+
visited.insert(&trait_ref.to_predicate()) || tcx.trait_is_auto(trait_ref.def_id())
343+
});
318344
TraitRefExpander { stack: items, visited: visited, }
319345
}
320346

@@ -324,6 +350,8 @@ impl<'cx, 'gcx, 'tcx> TraitRefExpander<'cx, 'gcx, 'tcx> {
324350
let tcx = self.visited.tcx;
325351
let trait_ref = item.trait_ref();
326352

353+
debug!("expand_trait_refs: trait_ref={:?}", trait_ref);
354+
327355
if !ty::is_trait_alias(tcx, trait_ref.def_id()) {
328356
return true;
329357
}
@@ -341,11 +369,15 @@ impl<'cx, 'gcx, 'tcx> TraitRefExpander<'cx, 'gcx, 'tcx> {
341369
})
342370
.collect();
343371

344-
debug!("expand_trait_refs: trait_ref={:?} items={:?}",
345-
trait_ref, items);
372+
debug!("expand_trait_refs: items={:?}", items);
346373

347374
// Only keep those items that we haven't already seen.
348-
items.retain(|i| self.visited.insert(&i.trait_ref().to_predicate()));
375+
// Note: we also retain auto traits here for the purpose of linting duplicate auto traits
376+
// in the `AstConv::conv_object_ty_poly_trait_ref` function.
377+
items.retain(|i| {
378+
let trait_ref = i.trait_ref();
379+
self.visited.insert(&trait_ref.to_predicate()) || tcx.trait_is_auto(trait_ref.def_id())
380+
});
349381

350382
self.stack.extend(items);
351383
false

‎src/librustc/ty/sty.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -519,11 +519,11 @@ impl<'tcx> UpvarSubsts<'tcx> {
519519

520520
#[derive(Debug, Copy, Clone, PartialEq, PartialOrd, Ord, Eq, Hash, RustcEncodable, RustcDecodable)]
521521
pub enum ExistentialPredicate<'tcx> {
522-
/// e.g., Iterator
522+
/// E.g., `Iterator`.
523523
Trait(ExistentialTraitRef<'tcx>),
524-
/// e.g., Iterator::Item = T
524+
/// E.g., `Iterator::Item = T`.
525525
Projection(ExistentialProjection<'tcx>),
526-
/// e.g., Send
526+
/// E.g., `Send`.
527527
AutoTrait(DefId),
528528
}
529529

@@ -1312,11 +1312,11 @@ impl<'a, 'tcx, 'gcx> PolyExistentialProjection<'tcx> {
13121312

13131313
impl DebruijnIndex {
13141314
/// Returns the resulting index when this value is moved into
1315-
/// `amount` number of new binders. So e.g., if you had
1315+
/// `amount` number of new binders. So, e.g., if you had
13161316
///
13171317
/// for<'a> fn(&'a x)
13181318
///
1319-
/// and you wanted to change to
1319+
/// and you wanted to change it to
13201320
///
13211321
/// for<'a> fn(for<'b> fn(&'a x))
13221322
///

‎src/librustc_lint/lib.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,12 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
336336
reference: "issue #52234 <https://github.com/rust-lang/rust/issues/52234>",
337337
edition: None,
338338
},
339-
]);
339+
FutureIncompatibleInfo {
340+
id: LintId::of(DUPLICATE_AUTO_TRAITS_IN_TRAIT_OBJECTS),
341+
reference: "issue #56522 <https://github.com/rust-lang/rust/issues/56522>",
342+
edition: None,
343+
},
344+
]);
340345

341346
// Register renamed and removed lints.
342347
store.register_renamed("single_use_lifetime", "single_use_lifetimes");

‎src/librustc_typeck/astconv.rs

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use util::nodemap::FxHashMap;
2929

3030
use std::collections::BTreeSet;
3131
use std::iter;
32+
use std::ops::Range;
3233
use std::slice;
3334

3435
use super::{check_type_alias_enum_variants_enabled};
@@ -954,6 +955,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
954955
}
955956

956957
fn conv_object_ty_poly_trait_ref(&self,
958+
node_id: ast::NodeId,
957959
span: Span,
958960
trait_bounds: &[hir::PolyTraitRef],
959961
lifetime: &hir::Lifetime)
@@ -978,7 +980,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
978980
debug!("principal: {:?}", principal);
979981

980982
for trait_bound in trait_bounds[1..].iter().rev() {
981-
// sanity check for non-principal trait bounds
983+
// Perform sanity check for non-principal trait bounds.
982984
let (tr, _) = self.instantiate_poly_trait_ref(trait_bound,
983985
dummy_self,
984986
&mut Vec::new());
@@ -987,18 +989,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
987989
bound_trait_refs.push((principal, trait_bounds[0].span));
988990

989991
let expanded_traits = traits::expand_trait_refs(tcx, bound_trait_refs);
990-
let (auto_traits, regular_traits): (Vec<_>, Vec<_>) =
992+
let (mut auto_traits, regular_traits): (Vec<_>, Vec<_>) =
991993
expanded_traits.partition(|i| tcx.trait_is_auto(i.trait_ref().def_id()));
992994
if regular_traits.len() > 1 {
993995
let extra_trait = &regular_traits[1];
994996
let mut err = struct_span_err!(tcx.sess, extra_trait.bottom().1, E0225,
995997
"only auto traits can be used as additional traits in a trait object");
996-
err.span_label(extra_trait.top().1, "non-auto additional trait");
997-
if extra_trait.items.len() > 2 {
998-
for (_, sp) in extra_trait.items[1..(extra_trait.items.len() - 1)].iter().rev() {
999-
err.span_label(*sp, "referenced by this alias");
1000-
}
1001-
}
998+
err.label_with_exp_info(extra_trait, "non-auto additional trait");
1002999
err.emit();
10031000
}
10041001

@@ -1142,26 +1139,48 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
11421139
})
11431140
});
11441141

1145-
// Dedup auto traits so that e.g. `dyn Trait + Send + Send` is the same as
1146-
// `dyn Trait + Send`. Skip the first auto trait if it is the principal trait.
1147-
let auto_traits_start_index =
1148-
if auto_traits.first().map_or(false, |i| i.trait_ref() == &principal) {
1149-
1
1150-
} else {
1151-
0
1152-
};
1153-
let mut auto_traits: Vec<_> =
1154-
auto_traits[auto_traits_start_index..].into_iter()
1155-
.map(|i| i.trait_ref().def_id())
1156-
.collect();
1157-
auto_traits.sort();
1158-
auto_traits.dedup();
1142+
// Lint duplicate auto traits, and then remove duplicates.
1143+
auto_traits.sort_by_key(|i| i.trait_ref().def_id());
1144+
let emit_dup_traits_err = |range: Range<usize>| {
1145+
let mut err = tcx.struct_span_lint_node(
1146+
lint::builtin::DUPLICATE_AUTO_TRAITS_IN_TRAIT_OBJECTS,
1147+
node_id,
1148+
auto_traits[range.clone()].iter().map(|i| i.bottom().1).collect::<Vec<_>>(),
1149+
&format!("duplicate auto trait `{}` found in trait object",
1150+
auto_traits[range.start].trait_ref()));
1151+
err.label_with_exp_info(&auto_traits[range.start], "first use of auto trait");
1152+
for i in (range.start + 1)..range.end {
1153+
err.label_with_exp_info(&auto_traits[i], "subsequent use of auto trait");
1154+
}
1155+
err.emit();
1156+
};
1157+
let mut seq_start = 0;
1158+
for i in 1..auto_traits.len() {
1159+
if auto_traits[i].trait_ref().def_id() != auto_traits[i - 1].trait_ref().def_id() {
1160+
if i - seq_start > 1 {
1161+
emit_dup_traits_err(seq_start..i);
1162+
}
1163+
seq_start = i;
1164+
}
1165+
}
1166+
if auto_traits.len() - seq_start > 1 {
1167+
emit_dup_traits_err(seq_start..auto_traits.len());
1168+
}
1169+
// Remove first auto trait if it is the principal trait.
1170+
// HACK(alexreg): we really want to do this *after* dedup, but we must maintain this
1171+
// behaviour for the sake of backwards compatibility, until the duplicate traits lint
1172+
// becomes a hard error.
1173+
if auto_traits.first().map_or(false, |tr| tr.trait_ref().def_id() == principal.def_id()) {
1174+
auto_traits.remove(0);
1175+
}
1176+
auto_traits.dedup_by_key(|i| i.trait_ref().def_id());
11591177
debug!("auto_traits: {:?}", auto_traits);
11601178

11611179
// Calling `skip_binder` is okay, since the predicates are re-bound.
11621180
let mut v =
11631181
iter::once(ty::ExistentialPredicate::Trait(*existential_principal.skip_binder()))
1164-
.chain(auto_traits.into_iter().map(ty::ExistentialPredicate::AutoTrait))
1182+
.chain(auto_traits.into_iter()
1183+
.map(|i| ty::ExistentialPredicate::AutoTrait(i.trait_ref().def_id())))
11651184
.chain(existential_projections
11661185
.map(|x| ty::ExistentialPredicate::Projection(*x.skip_binder())))
11671186
.collect::<SmallVec<[_; 8]>>();
@@ -1766,7 +1785,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
17661785
tcx.mk_fn_ptr(self.ty_of_fn(bf.unsafety, bf.abi, &bf.decl))
17671786
}
17681787
hir::TyKind::TraitObject(ref bounds, ref lifetime) => {
1769-
self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime)
1788+
self.conv_object_ty_poly_trait_ref(ast_ty.id, ast_ty.span, bounds, lifetime)
17701789
}
17711790
hir::TyKind::Path(hir::QPath::Resolved(ref maybe_qself, ref path)) => {
17721791
debug!("ast_ty_to_ty: maybe_qself={:?} path={:?}", maybe_qself, path);

‎src/test/run-pass/traits/trait-object-auto-dedup.stderr

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
warning: duplicate auto trait `std::marker::Send` found in trait object
2+
--> $DIR/trait-object-auto-dedup.rs:28:49
3+
|
4+
LL | fn takes_dyn_trait_send_send(_: Box<dyn Trait + Send + Send>) {}
5+
| ^^^^ ^^^^ subsequent use of auto trait
6+
| |
7+
| first use of auto trait
8+
|
9+
= note: #[warn(duplicate_auto_traits_in_trait_objects)] on by default
10+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
11+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
12+
13+
warning: duplicate auto trait `std::marker::Send` found in trait object
14+
--> $DIR/trait-object-auto-dedup.rs:29:54
15+
|
16+
LL | fn takes_dyn_trait_send_sendalias(_: Box<dyn Trait + Send + SendAlias>) {}
17+
| ^^^^ ^^^^^^^^^ subsequent use of auto trait
18+
| |
19+
| first use of auto trait
20+
|
21+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
22+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
23+
24+
warning: duplicate auto trait `std::marker::Send` found in trait object
25+
--> $DIR/trait-object-auto-dedup.rs:31:18
26+
|
27+
LL | impl dyn Trait + Send + Send {
28+
| ^^^^ ^^^^ subsequent use of auto trait
29+
| |
30+
| first use of auto trait
31+
|
32+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
33+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
34+
35+
warning: duplicate auto trait `std::marker::Send` found in trait object
36+
--> $DIR/trait-object-auto-dedup.rs:38:46
37+
|
38+
LL | let dyn_trait_send_send: Box<dyn Trait + Send + Send> = dyn_trait_send;
39+
| ^^^^ ^^^^ subsequent use of auto trait
40+
| |
41+
| first use of auto trait
42+
|
43+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
44+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
45+
46+
warning: duplicate auto trait `std::marker::Send` found in trait object
47+
--> $DIR/trait-object-auto-dedup.rs:45:67
48+
|
49+
LL | let dyn_trait_send_send = Box::new(Struct) as Box<dyn Trait + Send + Send>;
50+
| ^^^^ ^^^^ subsequent use of auto trait
51+
| |
52+
| first use of auto trait
53+
|
54+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
55+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
56+
57+
warning: duplicate auto trait `std::marker::Send` found in trait object
58+
--> $DIR/trait-object-auto-dedup.rs:28:49
59+
|
60+
LL | fn takes_dyn_trait_send_send(_: Box<dyn Trait + Send + Send>) {}
61+
| ^^^^ ^^^^ subsequent use of auto trait
62+
| |
63+
| first use of auto trait
64+
|
65+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
66+
= note: for more information, see issue #56522 <https://github.com/rust-lang/rust/issues/56522>
67+

‎src/test/ui/error-codes/E0225.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error[E0225]: only auto traits can be used as additional traits in a trait objec
1010
LL | trait Foo = std::io::Read + std::io::Write;
1111
| -------------- non-auto additional trait
1212
LL | trait Bar = Foo;
13-
| --- referenced by this alias
13+
| --- referenced here
1414
...
1515
LL | let _: Box<Bar>;
1616
| ^^^

‎src/test/ui/lint/lint-duplicate-traits.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// compile-pass
2+
3+
#![feature(trait_alias)]
4+
5+
trait Foo {}
6+
7+
trait SyncAlias = Sync;
8+
9+
impl Foo for dyn Send {}
10+
11+
impl Foo for dyn Send + Send {}
12+
13+
impl Foo for dyn Send + Sync + Send + SyncAlias {}
14+
15+
fn main() {}

0 commit comments

Comments
 (0)