Skip to content

A few cleanups and minor improvements to typeck #54591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 30, 2018
Merged

A few cleanups and minor improvements to typeck #54591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 35 additions & 39 deletions src/librustc_typeck/astconv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
bound_region: ty::BrNamed(id, name)
}))

// (*) -- not late-bound, won't change
// (*) -- not late-bound, won't change
}

None => {
Expand All @@ -167,8 +167,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
};

debug!("ast_region_to_region(lifetime={:?}) yields {:?}",
lifetime,
r);
lifetime,
r);

r
}
Expand Down Expand Up @@ -218,7 +218,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
span,
E0632,
"cannot provide explicit type parameters when `impl Trait` is \
used in argument position."
used in argument position."
};

err.emit();
Expand Down Expand Up @@ -538,7 +538,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
// region with the current anon region binding (in other words,
// whatever & would get replaced with).
debug!("create_substs_for_ast_path(def_id={:?}, self_ty={:?}, \
generic_args={:?})",
generic_args={:?})",
def_id, self_ty, generic_args);

let tcx = self.tcx();
Expand Down Expand Up @@ -609,7 +609,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
if default_needs_object_self(param) {
struct_span_err!(tcx.sess, span, E0393,
"the type parameter `{}` must be explicitly \
specified",
specified",
param.name)
.span_label(span,
format!("missing reference to `{}`", param.name))
Expand All @@ -623,7 +623,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
self.normalize_ty(
span,
tcx.at(span).type_of(param.def_id)
.subst_spanned(tcx, substs.unwrap(), Some(span))
.subst_spanned(tcx, substs.unwrap(), Some(span))
).into()
}
} else if infer_types {
Expand Down Expand Up @@ -851,7 +851,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
binding.span,
E0582,
"binding for associated type `{}` references lifetime `{}`, \
which does not appear in the trait input types",
which does not appear in the trait input types",
binding.item_name, br_name)
.emit();
}
Expand Down Expand Up @@ -891,7 +891,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
ref_id,
binding.span,
&format!("associated type binding `{}` specified more than once",
binding.item_name)
binding.item_name)
);
err.span_label(binding.span, "used more than once");
err.span_label(*prev_span, format!("first use of `{}`", binding.item_name));
Expand Down Expand Up @@ -994,7 +994,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
if !object_safety_violations.is_empty() {
tcx.report_object_safety_error(
span, principal.def_id(), object_safety_violations)
.emit();
.emit();
return tcx.types.err;
}

Expand All @@ -1014,13 +1014,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
for item_def_id in associated_types {
let assoc_item = tcx.associated_item(item_def_id);
let trait_def_id = assoc_item.container.id();
struct_span_err!(tcx.sess, span, E0191,
"the value of the associated type `{}` (from the trait `{}`) must be specified",
assoc_item.ident,
tcx.item_path_str(trait_def_id))
.span_label(span, format!(
"missing associated type `{}` value", assoc_item.ident))
.emit();
struct_span_err!(tcx.sess, span, E0191, "the value of the associated type `{}` \
(from the trait `{}`) must be specified",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someday we should probably get rustfmt working on this codebase (which might require some special tooling; compare #53896). I unambiguously agree with most of the spacing changes in this PR, but where my personal æsthetics happen to disagree (here, I would treat the format varargs the same as the other arguments to struct_span_err! rather than aligning them to the format string), I tend to doubt that that degree of bikeshedding is part of my duty as a reviewer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess at one point whitespace tidying will become automated; just I adjust some spots as I go to what I feel looks more readable and hope I'm not the one who feels that way 😜.

assoc_item.ident,
tcx.item_path_str(trait_def_id))
.span_label(span, format!("missing associated type `{}` value",
assoc_item.ident))
.emit();
}

// Dedup auto traits so that `dyn Trait + Send + Send` is the same as `dyn Trait + Send`.
Expand All @@ -1032,12 +1032,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
iter::once(ty::ExistentialPredicate::Trait(*existential_principal.skip_binder()))
.chain(auto_traits.into_iter().map(ty::ExistentialPredicate::AutoTrait))
.chain(existential_projections
.map(|x| ty::ExistentialPredicate::Projection(*x.skip_binder())))
.map(|x| ty::ExistentialPredicate::Projection(*x.skip_binder())))
.collect::<SmallVec<[_; 8]>>();
v.sort_by(|a, b| a.stable_cmp(tcx, b));
let existential_predicates = ty::Binder::bind(tcx.mk_existential_predicates(v.into_iter()));


// Explicitly specified region bound. Use that.
let region_bound = if !lifetime.is_elided() {
self.ast_region_to_region(lifetime, None)
Expand Down Expand Up @@ -1072,7 +1071,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
struct_span_err!(self.tcx().sess, span, E0223, "ambiguous associated type")
.span_label(span, "ambiguous associated type")
.note(&format!("specify the type using the syntax `<{} as {}>::{}`",
type_str, trait_str, name))
type_str, trait_str, name))
.emit();

}
Expand All @@ -1094,8 +1093,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {

// Check that there is exactly one way to find an associated type with the
// correct name.
let suitable_bounds =
traits::transitive_bounds(tcx, &bounds)
let suitable_bounds = traits::transitive_bounds(tcx, &bounds)
.filter(|b| self.trait_defines_associated_type_named(b.def_id(), assoc_name));

let param_node_id = tcx.hir.as_local_node_id(ty_param_def_id).unwrap();
Expand All @@ -1110,20 +1108,20 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
// Checks that bounds contains exactly one element and reports appropriate
// errors otherwise.
fn one_bound_for_assoc_type<I>(&self,
mut bounds: I,
ty_param_name: &str,
assoc_name: ast::Ident,
span: Span)
mut bounds: I,
ty_param_name: &str,
assoc_name: ast::Ident,
span: Span)
-> Result<ty::PolyTraitRef<'tcx>, ErrorReported>
where I: Iterator<Item=ty::PolyTraitRef<'tcx>>
{
let bound = match bounds.next() {
Some(bound) => bound,
None => {
struct_span_err!(self.tcx().sess, span, E0220,
"associated type `{}` not found for `{}`",
assoc_name,
ty_param_name)
"associated type `{}` not found for `{}`",
assoc_name,
ty_param_name)
.span_label(span, format!("associated type `{}` not found", assoc_name))
.emit();
return Err(ErrorReported);
Expand All @@ -1142,14 +1140,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
for bound in bounds {
let bound_span = self.tcx().associated_items(bound.def_id()).find(|item| {
item.kind == ty::AssociatedKind::Type &&
self.tcx().hygienic_eq(assoc_name, item.ident, bound.def_id())
self.tcx().hygienic_eq(assoc_name, item.ident, bound.def_id())
})
.and_then(|item| self.tcx().hir.span_if_local(item.def_id));

if let Some(span) = bound_span {
err.span_label(span, format!("ambiguous `{}` from `{}`",
assoc_name,
bound));
assoc_name,
bound));
} else {
span_note!(&mut err, span,
"associated type `{}` could derive from `{}`",
Expand Down Expand Up @@ -1198,8 +1196,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
}
};

let candidates =
traits::supertraits(tcx, ty::Binder::bind(trait_ref))
let candidates = traits::supertraits(tcx, ty::Binder::bind(trait_ref))
.filter(|r| self.trait_defines_associated_type_named(r.def_id(), assoc_name));

match self.one_bound_for_assoc_type(candidates, "Self", assoc_name, span) {
Expand Down Expand Up @@ -1230,7 +1227,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
let (assoc_ident, def_scope) = tcx.adjust_ident(assoc_name, trait_did, ref_id);
let item = tcx.associated_items(trait_did).find(|i| {
Namespace::from(i.kind) == Namespace::Type &&
i.ident.modern() == assoc_ident
i.ident.modern() == assoc_ident
})
.expect("missing associated type");

Expand Down Expand Up @@ -1293,16 +1290,15 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
if err_for_lt { continue }
err_for_lt = true;
(struct_span_err!(self.tcx().sess, lt.span, E0110,
"lifetime parameters are not allowed on \
this type"),
"lifetime parameters are not allowed on this type"),
lt.span,
"lifetime")
}
hir::GenericArg::Type(ty) => {
if err_for_ty { continue }
err_for_ty = true;
(struct_span_err!(self.tcx().sess, ty.span, E0109,
"type parameters are not allowed on this type"),
"type parameters are not allowed on this type"),
ty.span,
"type")
}
Expand Down Expand Up @@ -1590,7 +1586,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
));

// Find any late-bound regions declared in return type that do
// not appear in the arguments. These are not wellformed.
// not appear in the arguments. These are not well-formed.
//
// Example:
// for<'a> fn() -> &'a str <-- 'a is bad
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check_unused.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a, 'tcx> CheckVisitor<'a, 'tcx> {
let msg = if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
format!("unused import: `{}`", snippet)
} else {
"unused import".to_string()
"unused import".to_owned()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also prefer .to_owned over .to_string for literals (the former indicates that we're getting ownership of something which is already a string in some sense of the word, whereas the latter could be anything Display-able), but it's possible that we're in the minority insofar as The Book recommends .to_string or String::from.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think The Book just recommends the solution that is simplest to grasp and remember; to_string calls String::from which, in turn, calls to_owned(), so everything boils down to to_owned anyways.

};
self.tcx.lint_node(lint::builtin::UNUSED_IMPORTS, id, span, &msg);
}
Expand Down
74 changes: 33 additions & 41 deletions src/librustc_typeck/coherence/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,33 +55,29 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
}

fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) {
match tcx.type_of(impl_did).sty {
ty::Adt(..) => {}
_ => {
// Destructors only work on nominal types.
if let Some(impl_node_id) = tcx.hir.as_local_node_id(impl_did) {
match tcx.hir.find(impl_node_id) {
Some(Node::Item(item)) => {
let span = match item.node {
ItemKind::Impl(.., ref ty, _) => ty.span,
_ => item.span,
};
struct_span_err!(tcx.sess,
span,
E0120,
"the Drop trait may only be implemented on \
structures")
.span_label(span, "implementing Drop requires a struct")
.emit();
}
_ => {
bug!("didn't find impl in ast map");
}
}
if let ty::Adt(..) = tcx.type_of(impl_did).sty {
/* do nothing */
} else {
// Destructors only work on nominal types.
if let Some(impl_node_id) = tcx.hir.as_local_node_id(impl_did) {
if let Some(Node::Item(item)) = tcx.hir.find(impl_node_id) {
let span = match item.node {
ItemKind::Impl(.., ref ty, _) => ty.span,
_ => item.span,
};
struct_span_err!(tcx.sess,
span,
E0120,
"the Drop trait may only be implemented on \
structures")
.span_label(span, "implementing Drop requires a struct")
.emit();
} else {
bug!("found external impl of Drop trait on \
something other than a struct");
bug!("didn't find impl in ast map");
}
} else {
bug!("found external impl of Drop trait on \
something other than a struct");
}
}
}
Expand All @@ -92,8 +88,7 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did:
let impl_node_id = if let Some(n) = tcx.hir.as_local_node_id(impl_did) {
n
} else {
debug!("visit_implementation_of_copy(): impl not in this \
crate");
debug!("visit_implementation_of_copy(): impl not in this crate");
return;
};

Expand All @@ -119,11 +114,11 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did:
};

let mut err = struct_span_err!(tcx.sess,
span,
E0204,
"the trait `Copy` may not be implemented for this type");
span,
E0204,
"the trait `Copy` may not be implemented for this type");
for span in fields.iter().map(|f| tcx.def_span(f.did)) {
err.span_label(span, "this field does not implement `Copy`");
err.span_label(span, "this field does not implement `Copy`");
}
err.emit()
}
Expand Down Expand Up @@ -173,12 +168,9 @@ pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>,
debug!("compute_coerce_unsized_info(impl_did={:?})", impl_did);
let coerce_unsized_trait = gcx.lang_items().coerce_unsized_trait().unwrap();

let unsize_trait = match gcx.lang_items().require(UnsizeTraitLangItem) {
Ok(id) => id,
Err(err) => {
gcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err));
}
};
let unsize_trait = gcx.lang_items().require(UnsizeTraitLangItem).unwrap_or_else(|err| {
gcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err));
});

// this provider should only get invoked for local def-ids
let impl_node_id = gcx.hir.as_local_node_id(impl_did).unwrap_or_else(|| {
Expand Down Expand Up @@ -210,9 +202,9 @@ pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>,
mk_ptr: &dyn Fn(Ty<'gcx>) -> Ty<'gcx>| {
if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) {
infcx.report_mismatched_types(&cause,
mk_ptr(mt_b.ty),
target,
ty::error::TypeError::Mutability)
mk_ptr(mt_b.ty),
target,
ty::error::TypeError::Mutability)
.emit();
}
(mt_a.ty, mt_b.ty, unsize_trait, None)
Expand All @@ -235,7 +227,7 @@ pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>,
}

(&ty::Adt(def_a, substs_a), &ty::Adt(def_b, substs_b)) if def_a.is_struct() &&
def_b.is_struct() => {
def_b.is_struct() => {
if def_a != def_b {
let source_path = gcx.item_path_str(def_a.did);
let target_path = gcx.item_path_str(def_b.did);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/coherence/inherent_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,7 @@ impl<'a, 'tcx> InherentCollect<'a, 'tcx> {
E0116,
"cannot define inherent `impl` for a type outside of the crate \
where the type is defined")
.span_label(item.span,
"impl for type defined outside of crate.")
.span_label(item.span, "impl for type defined outside of crate.")
.note("define and implement a trait or new type instead")
.emit();
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/coherence/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) {

if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id) {
debug!("(checking implementation) adding impl for trait '{:?}', item '{}'",
trait_ref,
tcx.item_path_str(impl_def_id));
trait_ref,
tcx.item_path_str(impl_def_id));

// Skip impls where one of the self type is an error type.
// This occurs with e.g. resolve failures (#30589).
Expand Down
Loading