improve logic around free-variables to account for false answers

This commit is contained in:
Niko Matsakis 2018-07-16 15:44:59 +03:00 committed by Markus Westerlind
parent 9d37210c45
commit 5bfa1af9fa
2 changed files with 122 additions and 60 deletions

View File

@ -1,39 +1,66 @@
use grammar::parse_tree::{self, TypeParameter};
use grammar::parse_tree::{self, Lifetime, TypeParameter};
use grammar::repr;
use std::iter;
use string_cache::DefaultAtom as Atom;
/// Finds the set of "free variables" in something -- that is, the
/// type/lifetime parameters that appear and are not bound. For
/// example, `T: Foo<U>` would return `[T, U]`.
pub trait FreeVariables {
fn free_variables(&self) -> Vec<TypeParameter>;
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter>;
}
/// Subtle: the free-variables code sometimes encounter ambiguous
/// names. For example, we might see `Vec<Foo>` -- in that case, we
/// look at the list of declared type parameters to decide whether
/// `Foo` is a type parameter or just some other type name.
fn free_type(type_parameters: &[TypeParameter], id: &Atom) -> Vec<TypeParameter> {
let tp = TypeParameter::Id(id.clone());
if type_parameters.contains(&tp) {
vec![tp]
} else {
vec![]
}
}
/// Same as above: really, the only lifetime where this is relevant is
/// `'static`, but it doesn't hurt to be careful.
fn free_lifetime(type_parameters: &[TypeParameter], lt: &Lifetime) -> Vec<TypeParameter> {
let tp = TypeParameter::Lifetime(lt.clone());
if type_parameters.contains(&tp) {
vec![tp]
} else {
vec![]
}
}
impl<T: FreeVariables> FreeVariables for Option<T> {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
None => vec![],
Some(t) => t.free_variables(),
Some(t) => t.free_variables(type_parameters),
}
}
}
impl<T: FreeVariables> FreeVariables for Vec<T> {
fn free_variables(&self) -> Vec<TypeParameter> {
self.into_iter().flat_map(|e| e.free_variables()).collect()
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
self.into_iter()
.flat_map(|e| e.free_variables(type_parameters))
.collect()
}
}
impl FreeVariables for repr::TypeRepr {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
repr::TypeRepr::Tuple(tys) => tys.free_variables(),
repr::TypeRepr::Nominal(data) => data.free_variables(),
repr::TypeRepr::Tuple(tys) => tys.free_variables(type_parameters),
repr::TypeRepr::Nominal(data) => data.free_variables(type_parameters),
repr::TypeRepr::Associated {
type_parameter,
id: _,
} => vec![TypeParameter::Id(type_parameter.clone())],
repr::TypeRepr::Lifetime(l) => vec![TypeParameter::Lifetime(l.clone())],
} => free_type(type_parameters, type_parameter),
repr::TypeRepr::Lifetime(l) => free_lifetime(type_parameters, l),
repr::TypeRepr::Ref {
lifetime,
mutable: _,
@ -41,55 +68,55 @@ impl FreeVariables for repr::TypeRepr {
} => lifetime
.iter()
.map(|id| TypeParameter::Lifetime(id.clone()))
.chain(referent.free_variables())
.chain(referent.free_variables(type_parameters))
.collect(),
}
}
}
impl FreeVariables for repr::WhereClause {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
repr::WhereClause::Forall { binder, clause } => clause
.free_variables()
.free_variables(type_parameters)
.into_iter()
.filter(|tp| !binder.contains(tp))
.collect(),
repr::WhereClause::Bound { subject, bound } => subject
.free_variables()
.free_variables(type_parameters)
.into_iter()
.chain(bound.free_variables())
.chain(bound.free_variables(type_parameters))
.collect(),
}
}
}
impl FreeVariables for parse_tree::Path {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
// A path like `foo::Bar` is considered no free variables; a
// single identifier like `T` is a free variable `T`. Note
// that we can't distinguish type parameters from random names
// like `String`.
match self.as_id() {
Some(id) => vec![TypeParameter::Id(id)],
Some(id) => free_type(type_parameters, &id),
None => vec![],
}
}
}
impl FreeVariables for repr::NominalTypeRepr {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
let repr::NominalTypeRepr { path, types } = self;
path.free_variables()
path.free_variables(type_parameters)
.into_iter()
.chain(types.free_variables())
.chain(types.free_variables(type_parameters))
.collect()
}
}
impl<T: FreeVariables> FreeVariables for parse_tree::WhereClause<T> {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
parse_tree::WhereClause::Lifetime { lifetime, bounds } => {
iter::once(TypeParameter::Lifetime(lifetime.clone()))
@ -98,9 +125,9 @@ impl<T: FreeVariables> FreeVariables for parse_tree::WhereClause<T> {
}
parse_tree::WhereClause::Type { forall, ty, bounds } => ty
.free_variables()
.free_variables(type_parameters)
.into_iter()
.chain(bounds.free_variables())
.chain(bounds.free_variables(type_parameters))
.filter(|tp| !forall.contains(tp))
.collect(),
}
@ -108,29 +135,29 @@ impl<T: FreeVariables> FreeVariables for parse_tree::WhereClause<T> {
}
impl<T: FreeVariables> FreeVariables for parse_tree::TypeBoundParameter<T> {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
parse_tree::TypeBoundParameter::Lifetime(l) => vec![TypeParameter::Lifetime(l.clone())],
parse_tree::TypeBoundParameter::TypeParameter(t) => t.free_variables(),
parse_tree::TypeBoundParameter::Lifetime(l) => free_lifetime(type_parameters, l),
parse_tree::TypeBoundParameter::TypeParameter(t) => t.free_variables(type_parameters),
parse_tree::TypeBoundParameter::Associated(..) => vec![],
}
}
}
impl<T: FreeVariables> FreeVariables for parse_tree::TypeBound<T> {
fn free_variables(&self) -> Vec<TypeParameter> {
fn free_variables(&self, type_parameters: &[TypeParameter]) -> Vec<TypeParameter> {
match self {
parse_tree::TypeBound::Lifetime(l) => vec![TypeParameter::Lifetime(l.clone())],
parse_tree::TypeBound::Lifetime(l) => free_lifetime(type_parameters, l),
parse_tree::TypeBound::Fn {
forall,
path,
parameters,
ret,
} => path
.free_variables()
.free_variables(type_parameters)
.into_iter()
.chain(parameters.free_variables())
.chain(ret.free_variables())
.chain(parameters.free_variables(type_parameters))
.chain(ret.free_variables(type_parameters))
.filter(|tp| !forall.contains(tp))
.collect(),
parse_tree::TypeBound::Trait {
@ -138,9 +165,9 @@ impl<T: FreeVariables> FreeVariables for parse_tree::TypeBound<T> {
path,
parameters,
} => path
.free_variables()
.free_variables(type_parameters)
.into_iter()
.chain(parameters.free_variables())
.chain(parameters.free_variables(type_parameters))
.filter(|tp| !forall.contains(tp))
.collect(),
}

View File

@ -68,17 +68,50 @@ impl<'codegen, 'grammar, W: Write, C> CodeGenerator<'codegen, 'grammar, W, C> {
}
}
/// The nonterminal type needs to be parameterized by all the type
/// parameters that actually appear in the types of nonterminals.
/// We can't just use *all* type parameters because that would
/// leave unused lifetime/type parameters in some cases.
/// We often create meta types that pull together a bunch of
/// user-given types -- basically describing (e.g.) the full set
/// of return values from any nonterminal (and, in some cases,
/// terminals). These types need to carry generic parameters from
/// the grammar, since the nonterminals may include generic
/// parameters -- but we don't want them to carry *all* the
/// generic parameters, since that can be unnecessarily
/// restrictive.
///
/// In particular, consider something like this:
///
/// ```notrust
/// grammar<'a>(buffer: &'a mut Vec<u32>);
/// ```
///
/// Here, we likely do not want the `'a` in the type of `buffer` to appear
/// in the nonterminal result. That's because, if it did, then the
/// action functions will have a signature like:
///
/// ```ignore
/// fn foo<'a, T>(x: &'a mut Vec<T>) -> Result<'a> { ... }
/// ```
///
/// In that case, we would only be able to call one action fn and
/// will in fact get borrowck errors, because Rust would think we
/// were potentially returning this `&'a mut Vec<T>`.
///
/// Therefore, we take the full list of type parameters and we
/// filter them down to those that appear in the types that we
/// need to include (those that appear in the `tys` parameter).
///
/// In some cases, we need to include a few more than just that
/// obviously appear textually: for example, if we have `T::Foo`,
/// and we see a where-clause `T: Bar<'a>`, then we need to
/// include both `T` and `'a`, since that bound may be important
/// for resolving `T::Foo` (in other words, `T::Foo` may expand to
/// `<T as Bar<'a>>::Foo`).
pub fn filter_type_parameters_and_where_clauses(
grammar: &Grammar,
referenced_ty_params: impl IntoIterator<Item = TypeRepr>,
tys: impl IntoIterator<Item = TypeRepr>,
) -> (Vec<TypeParameter>, Vec<WhereClause>) {
let referenced_ty_params: Set<_> = referenced_ty_params
let referenced_ty_params: Set<_> = tys
.into_iter()
.flat_map(|t| t.free_variables())
.flat_map(|t| t.free_variables(&grammar.type_parameters))
.collect();
let filtered_type_params: Vec<_> = grammar
@ -88,31 +121,33 @@ impl<'codegen, 'grammar, W: Write, C> CodeGenerator<'codegen, 'grammar, W, C> {
.cloned()
.collect();
// FIXME: this is wrong. We are currently including something
// like `P: Foo<'a>` only if both `P` and `'a` are referenced;
// but if we have `P::Bar` that may come from the `Foo<'a>`
// bound. The problem is that we don't want `P: 'a` to force
// `'a` to be included.
// If `T` is referenced in the types we need to keep, then
// include any bounds like `T: Foo`. This may be needed for
// the well-formedness conditions on `T` (e.g., maybe we have
// `T: Hash` and a `HashSet<T>` or something) but it may also
// be needed because of `T::Foo`-like types.
//
// Do not however include a bound like `T: 'a` unless both `T`
// **and** `'a` are referenced -- same with bounds like `T:
// Foo<U>`. If those were needed, then `'a` or `U` would also
// have to appear in the types.
debug!("filtered_type_params = {:?}", filtered_type_params);
let referenced_where_clauses: Set<_> = grammar
.where_clauses
.iter()
.filter(|wc| {
debug!("wc = {:?} free_variables = {:?}", wc, wc.free_variables());
wc.free_variables()
.iter()
.any(|p| filtered_type_params.contains(p))
})
.cloned()
.collect();
debug!("referenced_where_clauses = {:?}", referenced_where_clauses);
let filtered_where_clauses: Vec<_> = grammar
.where_clauses
.iter()
.filter(|wc| referenced_where_clauses.contains(wc))
.filter(|wc| {
debug!(
"wc = {:?} free_variables = {:?}",
wc,
wc.free_variables(&grammar.type_parameters)
);
wc.free_variables(&grammar.type_parameters)
.iter()
.all(|p| referenced_ty_params.contains(p))
})
.cloned()
.collect();
debug!("filtered_where_clauses = {:?}", filtered_where_clauses);
(filtered_type_params, filtered_where_clauses)
}