Skip to content

Commit 442d92b

Browse files
jroeschjroesch
committed
Refactor compare_impl_method to use all bounds
Refactor compare_impl_method into its own file, and change the code to stop comparing and checking individual parameter bounds. Instead we now use the predicates list attached to the trait and implementation generics. This ensures consistency even when bounds are declared in different places (i.e on a parameter vs. in a where clause).
1 parent c7dd3c4 commit 442d92b

File tree

3 files changed

+271
-2
lines changed

3 files changed

+271
-2
lines changed

‎src/librustc_typeck/check/compare_method.rs

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use middle::infer;
12+
use middle::traits;
13+
use middle::ty::{mod};
14+
use middle::subst::{mod, Subst};
15+
use util::ppaux::{mod, Repr};
16+
17+
use syntax::ast;
18+
use syntax::codemap::{Span};
19+
use syntax::parse::token;
20+
21+
/// Checks that a method from an impl conforms to the signature of
22+
/// the same method as declared in the trait.
23+
///
24+
/// # Parameters
25+
///
26+
/// - impl_m: type of the method we are checking
27+
/// - impl_m_span: span to use for reporting errors
28+
/// - impl_m_body_id: id of the method body
29+
/// - trait_m: the method in the trait
30+
/// - impl_trait_ref: the TraitRef corresponding to the trait implementation
31+
32+
pub fn compare_impl_method<'tcx>(tcx: &ty::ctxt<'tcx>,
33+
impl_m: &ty::Method<'tcx>,
34+
impl_m_span: Span,
35+
impl_m_body_id: ast::NodeId,
36+
trait_m: &ty::Method<'tcx>,
37+
impl_trait_ref: &ty::TraitRef<'tcx>) {
38+
debug!("compare_impl_method(impl_trait_ref={})",
39+
impl_trait_ref.repr(tcx));
40+
41+
debug!("impl_trait_ref (liberated) = {}",
42+
impl_trait_ref.repr(tcx));
43+
44+
let infcx = infer::new_infer_ctxt(tcx);
45+
46+
let trait_to_impl_substs = &impl_trait_ref.substs;
47+
48+
// Try to give more informative error messages about self typing
49+
// mismatches. Note that any mismatch will also be detected
50+
// below, where we construct a canonical function type that
51+
// includes the self parameter as a normal parameter. It's just
52+
// that the error messages you get out of this code are a bit more
53+
// inscrutable, particularly for cases where one method has no
54+
// self.
55+
match (&trait_m.explicit_self, &impl_m.explicit_self) {
56+
(&ty::StaticExplicitSelfCategory,
57+
&ty::StaticExplicitSelfCategory) => {}
58+
(&ty::StaticExplicitSelfCategory, _) => {
59+
tcx.sess.span_err(
60+
impl_m_span,
61+
format!("method `{}` has a `{}` declaration in the impl, \
62+
but not in the trait",
63+
token::get_name(trait_m.name),
64+
ppaux::explicit_self_category_to_str(
65+
&impl_m.explicit_self)).as_slice());
66+
return;
67+
}
68+
(_, &ty::StaticExplicitSelfCategory) => {
69+
tcx.sess.span_err(
70+
impl_m_span,
71+
format!("method `{}` has a `{}` declaration in the trait, \
72+
but not in the impl",
73+
token::get_name(trait_m.name),
74+
ppaux::explicit_self_category_to_str(
75+
&trait_m.explicit_self)).as_slice());
76+
return;
77+
}
78+
_ => {
79+
// Let the type checker catch other errors below
80+
}
81+
}
82+
83+
let num_impl_m_type_params = impl_m.generics.types.len(subst::FnSpace);
84+
let num_trait_m_type_params = trait_m.generics.types.len(subst::FnSpace);
85+
if num_impl_m_type_params != num_trait_m_type_params {
86+
span_err!(tcx.sess, impl_m_span, E0049,
87+
"method `{}` has {} type parameter{} \
88+
but its trait declaration has {} type parameter{}",
89+
token::get_name(trait_m.name),
90+
num_impl_m_type_params,
91+
if num_impl_m_type_params == 1 {""} else {"s"},
92+
num_trait_m_type_params,
93+
if num_trait_m_type_params == 1 {""} else {"s"});
94+
return;
95+
}
96+
97+
if impl_m.fty.sig.0.inputs.len() != trait_m.fty.sig.0.inputs.len() {
98+
span_err!(tcx.sess, impl_m_span, E0050,
99+
"method `{}` has {} parameter{} \
100+
but the declaration in trait `{}` has {}",
101+
token::get_name(trait_m.name),
102+
impl_m.fty.sig.0.inputs.len(),
103+
if impl_m.fty.sig.0.inputs.len() == 1 {""} else {"s"},
104+
ty::item_path_str(tcx, trait_m.def_id),
105+
trait_m.fty.sig.0.inputs.len());
106+
return;
107+
}
108+
109+
// This code is best explained by example. Consider a trait:
110+
//
111+
// trait Trait<'t,T> {
112+
// fn method<'a,M>(t: &'t T, m: &'a M) -> Self;
113+
// }
114+
//
115+
// And an impl:
116+
//
117+
// impl<'i, 'j, U> Trait<'j, &'i U> for Foo {
118+
// fn method<'b,N>(t: &'j &'i U, m: &'b N) -> Foo;
119+
// }
120+
//
121+
// We wish to decide if those two method types are compatible.
122+
//
123+
// We start out with trait_to_impl_substs, that maps the trait
124+
// type parameters to impl type parameters. This is taken from the
125+
// impl trait reference:
126+
//
127+
// trait_to_impl_substs = {'t => 'j, T => &'i U, Self => Foo}
128+
//
129+
// We create a mapping `dummy_substs` that maps from the impl type
130+
// parameters to fresh types and regions. For type parameters,
131+
// this is the identity transform, but we could as well use any
132+
// skolemized types. For regions, we convert from bound to free
133+
// regions (Note: but only early-bound regions, i.e., those
134+
// declared on the impl or used in type parameter bounds).
135+
//
136+
// impl_to_skol_substs = {'i => 'i0, U => U0, N => N0 }
137+
//
138+
// Now we can apply skol_substs to the type of the impl method
139+
// to yield a new function type in terms of our fresh, skolemized
140+
// types:
141+
//
142+
// <'b> fn(t: &'i0 U0, m: &'b) -> Foo
143+
//
144+
// We now want to extract and substitute the type of the *trait*
145+
// method and compare it. To do so, we must create a compound
146+
// substitution by combining trait_to_impl_substs and
147+
// impl_to_skol_substs, and also adding a mapping for the method
148+
// type parameters. We extend the mapping to also include
149+
// the method parameters.
150+
//
151+
// trait_to_skol_substs = { T => &'i0 U0, Self => Foo, M => N0 }
152+
//
153+
// Applying this to the trait method type yields:
154+
//
155+
// <'a> fn(t: &'i0 U0, m: &'a) -> Foo
156+
//
157+
// This type is also the same but the name of the bound region ('a
158+
// vs 'b). However, the normal subtyping rules on fn types handle
159+
// this kind of equivalency just fine.
160+
//
161+
// We now use these subsititions to ensure that all declared bounds are
162+
// satisfied by the implementation's method.
163+
//
164+
// We do this by creating a parameter environment which contains a
165+
// substition corresponding to impl_to_skol_substs. We then build
166+
// trait_to_skol_substs and use it to convert the predicates contained
167+
// in the trait_m.generics to the skolemized form.
168+
//
169+
// Finally we register each of these predicates as an obligation in
170+
// a fresh FulfillmentCtxt, and invoke select_all_or_error.
171+
//
172+
// FIXME(jroesch): Currently the error reporting is not correctly attached
173+
// to a span and results in terrible error reporting.
174+
175+
// Create a parameter environment that represents the implementation's
176+
// method.
177+
let impl_env = ty::ParameterEnvironment::for_item(tcx, impl_m.def_id.node);
178+
179+
// Create mapping from impl to skolemized.
180+
let impl_to_skol_substs = impl_env.free_substs.clone();
181+
182+
// Create mapping from trait to skolemized.
183+
let trait_to_skol_substs =
184+
trait_to_impl_substs
185+
.subst(tcx, &impl_to_skol_substs)
186+
.with_method(impl_to_skol_substs.types.get_slice(subst::FnSpace).to_vec(),
187+
impl_to_skol_substs.regions().get_slice(subst::FnSpace).to_vec());
188+
189+
// Create a fresh fulfillment context.
190+
let mut fulfill_cx = traits::FulfillmentContext::new();
191+
192+
// Create obligations for each predicate declared by the trait definition.
193+
for predicate in trait_m.generics.predicates.iter()
194+
.map(|p| p.subst(tcx, &trait_to_skol_substs)) {
195+
fulfill_cx.register_predicate(
196+
tcx,
197+
traits::Obligation::new(traits::ObligationCause::dummy(), predicate)
198+
)
199+
}
200+
201+
// Check that all obligations are satisfied by the implementation's
202+
// version.
203+
match fulfill_cx.select_all_or_error(&infcx, &impl_env, tcx) {
204+
Err(ref errors) => { traits::report_fulfillment_errors(&infcx, errors) }
205+
Ok(_) => {}
206+
}
207+
208+
// Compute skolemized form of impl and trait method tys.
209+
let impl_fty = ty::mk_bare_fn(tcx, None, impl_m.fty.clone());
210+
let impl_fty = impl_fty.subst(tcx, &impl_to_skol_substs);
211+
let trait_fty = ty::mk_bare_fn(tcx, None, trait_m.fty.clone());
212+
let trait_fty = trait_fty.subst(tcx, &trait_to_skol_substs);
213+
214+
// Check the impl method type IM is a subtype of the trait method
215+
// type TM. To see why this makes sense, think of a vtable. The
216+
// expected type of the function pointers in the vtable is the
217+
// type TM of the trait method. The actual type will be the type
218+
// IM of the impl method. Because we know that IM <: TM, that
219+
// means that anywhere a TM is expected, a IM will do instead. In
220+
// other words, anyone expecting to call a method with the type
221+
// from the trait, can safely call a method with the type from the
222+
// impl instead.
223+
debug!("checking trait method for compatibility: impl ty {}, trait ty {}",
224+
impl_fty.repr(tcx),
225+
trait_fty.repr(tcx));
226+
match infer::mk_subty(&infcx, false, infer::MethodCompatCheck(impl_m_span),
227+
impl_fty, trait_fty) {
228+
Ok(()) => {}
229+
Err(ref terr) => {
230+
span_err!(tcx.sess, impl_m_span, E0053,
231+
"method `{}` has an incompatible type for trait: {}",
232+
token::get_name(trait_m.name),
233+
ty::type_err_to_str(tcx, terr));
234+
ty::note_and_explain_type_err(tcx, terr);
235+
}
236+
}
237+
238+
// Finally, resolve all regions. This catches wily misuses of lifetime
239+
// parameters.
240+
infcx.resolve_regions_and_report_errors(impl_m_body_id);
241+
}

‎src/librustc_typeck/check/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ type parameter).
7878

7979
pub use self::LvaluePreference::*;
8080
pub use self::Expectation::*;
81+
pub use self::compare_method::compare_impl_method;
8182
use self::IsBinopAssignment::*;
8283
use self::TupleArgumentsFlag::*;
8384

@@ -105,7 +106,7 @@ use TypeAndSubsts;
105106
use middle::lang_items::TypeIdLangItem;
106107
use lint;
107108
use util::common::{block_query, indenter, loop_query};
108-
use util::ppaux::{self, UserString, Repr};
109+
use util::ppaux::{mod, Repr};
109110
use util::nodemap::{DefIdMap, FnvHashMap, NodeMap};
110111

111112
use std::cell::{Cell, Ref, RefCell};
@@ -134,8 +135,8 @@ mod upvar;
134135
pub mod wf;
135136
mod closure;
136137
mod callee;
138+
mod compare_method;
137139

138-
/// Fields that are part of a `FnCtxt` which are inherited by
139140
/// closures defined within the function. For example:
140141
///
141142
/// fn foo() {

‎src/test/run-pass/where-clause-bounds-inconsistency.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
trait Bound {}
12+
13+
trait Trait {
14+
fn a<T>(&self, T) where T: Bound;
15+
fn b<T>(&self, T) where T: Bound;
16+
fn c<T: Bound>(&self, T);
17+
fn d<T: Bound>(&self, T);
18+
}
19+
20+
impl Trait for bool {
21+
fn a<T: Bound>(&self, _: T) {} //~ This gets rejected but should be accepted
22+
fn b<T>(&self, _: T) where T: Bound {}
23+
fn c<T: Bound>(&self, _: T) {}
24+
fn d<T>(&self, _: T) where T: Bound {}
25+
}
26+
27+
fn main() {}

0 commit comments

Comments
 (0)