Skip to content

Use lifetime elision in impl headers for generated impls #44

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.

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ matrix:
- cargo test --verbose || travis_terminate 1
- name: "Build on beta"
rust: beta
script: cargo build --verbose
script:
- cargo build --verbose
- cargo build --verbose --examples

env:
global:
Expand Down
24 changes: 11 additions & 13 deletions examples/names.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
//! Example to demonstrate how `auto_impl` chooses a name for the type and
//! lifetime parameter.
//! Example to demonstrate how `auto_impl` chooses a name for the type
//! parameter.
//!
//! For documentation and compiler errors it would be nice to have very simple
//! names for type and lifetime parameters:
//! names for the type parameter:
//!
//! ```rust
//! // not nice
//! impl<'auto_impl_lifetime, AutoImplT> Foo for &'auto_impl_lifetime AutoImplT { ...}
//! impl<AutoImplT> Foo for &AutoImplT { ...}
//!
//! // better
//! impl<'a, T> Foo for &'a T { ... }
//! impl<T> Foo for &T { ... }
//! ```
//!
//! `auto_impl` tries the full alphabet, picking a name that is not yet taken.
Expand All @@ -18,13 +18,11 @@
//! in the trait def -- apart from names only appearing in the body of provided
//! methods.
//!
//! In the trait below, a bunch of names are already "taken":
//! - type names: T--Z and A--G (H is not taken, because it only appear in the
//! default method body)
//! - lifetime names: 'a--'c
//! In the trait below, a bunch of type names are already "taken": T--Z and
//! A--H. Thus, the name `I` will be used.
//!
//! Thus, the names `H` and `'d` are used. Run `cargo expand --example names`
//! to see the output.
//! Thus, the name `H` is used. Run `cargo expand --example names` to see the
//! output.


// This code is really ugly on purpose...
Expand All @@ -45,12 +43,12 @@ struct G<T>(Vec<T>);
struct H {}

#[auto_impl(&)]
trait U<'a, V> {
trait U<'a, T, V> {
const W: Option<Box<&'static X>>;

type Y: Z;

fn A<'b, 'c>(&self, B: C, D: E<&[F; 1]>) -> G<fn((H,))> {
fn A(&self, B: C, D: E<&[F; 1]>) -> G<fn((H,))> {
let H = ();
unimplemented!()
}
Expand Down
42 changes: 10 additions & 32 deletions src/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,43 +17,32 @@ use syn::{
/// changed.
const PROXY_TY_PARAM_NAME: &str = "__AutoImplProxyT";

/// The lifetime parameter used in the proxy type if the proxy type is `&` or
/// `&mut`. For more information see `PROXY_TY_PARAM_NAME`.
const PROXY_LT_PARAM_NAME: &str = "'__auto_impl_proxy_lifetime";
Copy link
Member

Choose a reason for hiding this comment

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

🎉



/// We need to introduce our own type and lifetime parameter. Regardless of
/// what kind of hygiene we use for the parameter, it would be nice (for docs
/// and compiler errors) if the names are as simple as possible ('a and T, for
/// example).
/// We need to introduce our own type parameter. Regardless of what kind of
/// hygiene we use for the parameter, it would be nice (for docs and compiler
/// errors) if the name is as simple as possible (`T`, for example).
///
/// This function searches for names that we can use. Such a name must not
/// conflict with any other name we'll use in the `impl` block. Luckily, we
/// know all those names in advance.
///
/// The idea is to collect all names that might conflict with our names, store
/// them in a set and later check which name we can use. If we can't use a simple
/// name, we'll use the ugly `PROXY_TY_PARAM_NAME` and `PROXY_LT_PARAM_NAME`.
///
/// This method returns two idents: (type_parameter, lifetime_parameter).
pub(crate) fn find_suitable_param_names(trait_def: &ItemTrait) -> (Ident, Lifetime) {
/// them in a set and later check which name we can use. If we can't use a
/// simple name, we'll use the ugly `PROXY_TY_PARAM_NAME`.
pub(crate) fn find_suitable_param_name(trait_def: &ItemTrait) -> Ident {
// Define the visitor that just collects names
struct IdentCollector<'ast> {
ty_names: HashSet<&'ast Ident>,
lt_names: HashSet<&'ast Ident>,
}

impl<'ast> Visit<'ast> for IdentCollector<'ast> {
fn visit_ident(&mut self, i: &'ast Ident) {
self.ty_names.insert(i);
}

// We overwrite this to make sure to put lifetime names into
// `lt_names`. We also don't recurse, so `visit_ident` won't be called
// for lifetime names.
fn visit_lifetime(&mut self, lt: &'ast Lifetime) {
self.lt_names.insert(&lt.ident);
}
// We overwrite this to make sure to not recurse, so `visit_ident`
// won't be called for lifetime names.
fn visit_lifetime(&mut self, _: &'ast Lifetime) {}

// Visiting a block just does nothing. It is the default body of a method
// in the trait. But since that block won't be in the impl block, we can
Expand All @@ -64,7 +53,6 @@ pub(crate) fn find_suitable_param_names(trait_def: &ItemTrait) -> (Ident, Lifeti
// Create the visitor and visit the trait
let mut visitor = IdentCollector {
ty_names: HashSet::new(),
lt_names: HashSet::new(),
};
visit_item_trait(&mut visitor, trait_def);

Expand All @@ -82,17 +70,7 @@ pub(crate) fn find_suitable_param_names(trait_def: &ItemTrait) -> (Ident, Lifeti
.find(|i| !visitor.ty_names.contains(i))
.unwrap_or_else(|| Ident::new(PROXY_TY_PARAM_NAME, param_span()));

// Find suitable lifetime name ('a..='z)
let lt_name = (b'a'..=b'z')
.map(char_to_ident)
.find(|i| !visitor.lt_names.contains(i))
.unwrap_or_else(|| Ident::new(PROXY_LT_PARAM_NAME, param_span()));
let lt = Lifetime {
apostrophe: param_span(),
ident: lt_name,
};

(ty_name, lt)
ty_name
}

/// On nightly, we use `def_site` hygiene which puts our names into another
Expand Down
69 changes: 35 additions & 34 deletions src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use crate::proc_macro::Span;
use proc_macro2::TokenStream as TokenStream2;
use quote::{ToTokens, TokenStreamExt};
use syn::{
FnArg, Ident, ItemTrait, Lifetime, MethodSig, Pat, PatIdent, TraitItem, TraitItemConst,
FnArg, Ident, ItemTrait, MethodSig, Pat, PatIdent, TraitItem, TraitItemConst,
TraitItemMethod, TraitItemType,
};

use crate::{
analyze::find_suitable_param_names,
analyze::find_suitable_param_name,
attr::{is_our_attr, parse_our_attr, OurAttr},
diag::{DiagnosticExt, SpanExt},
proxy::ProxyType,
Expand All @@ -24,11 +24,11 @@ pub(crate) fn gen_impls(
) -> Result<TokenStream2, ()> {
let mut tokens = TokenStream2::new();

let (proxy_ty_param, proxy_lt_param) = find_suitable_param_names(trait_def);
let proxy_ty_param = find_suitable_param_name(trait_def);

// One impl for each proxy type
for proxy_type in proxy_types {
let header = gen_header(proxy_type, trait_def, &proxy_ty_param, &proxy_lt_param)?;
let header = gen_header(proxy_type, trait_def, &proxy_ty_param)?;
let items = gen_items(proxy_type, trait_def, &proxy_ty_param)?;

tokens.append_all(quote! {
Expand All @@ -45,7 +45,6 @@ fn gen_header(
proxy_type: &ProxyType,
trait_def: &ItemTrait,
proxy_ty_param: &Ident,
proxy_lt_param: &Lifetime,
) -> Result<TokenStream2, ()> {
// Generate generics for impl positions from trait generics.
let (impl_generics, trait_generics, where_clause) = trait_def.generics.split_for_impl();
Expand All @@ -57,54 +56,56 @@ fn gen_header(

// Here we assemble the parameter list of the impl (the thing in
// `impl< ... >`). This is simply the parameter list of the trait with
// one or two parameters added. For a trait `trait Foo<'x, 'y, A, B>`,
// one parameter added. For a trait `trait Foo<'x, 'y, A, B>`,
// it will look like this:
//
// '{proxy_lt_param}, 'x, 'y, A, B, {proxy_ty_param}
// 'x, 'y, A, B, {proxy_ty_param}
//
// The `'{proxy_lt_param}` in the beginning is only added when the proxy
// type is `&` or `&mut`.
let impl_generics = {
// Determine if our proxy type needs a lifetime parameter
let (mut params, ty_bounds) = match proxy_type {
ProxyType::Ref | ProxyType::RefMut => {
(quote! { #proxy_lt_param, }, quote! { : #proxy_lt_param + #trait_path })
}
ProxyType::Box | ProxyType::Rc | ProxyType::Arc => (quote!{}, quote! { : #trait_path }),
// Determine the correct proxy type bound
let ty_bounds = match proxy_type {
ProxyType::Ref
| ProxyType::RefMut
| ProxyType::Box
| ProxyType::Rc
| ProxyType::Arc => quote! { : #trait_path },

ProxyType::Fn | ProxyType::FnMut | ProxyType::FnOnce => {
let fn_bound = gen_fn_type_for_trait(proxy_type, trait_def)?;
(quote!{}, quote! { : #fn_bound })
quote! { : #fn_bound }
}
};

// Append all parameters from the trait. Sadly, `impl_generics`
// includes the angle brackets `< >` so we have to remove them like
// this.
let mut tts = impl_generics.into_token_stream()
.into_iter()
.skip(1) // the opening `<`
.collect::<Vec<_>>();
tts.pop(); // the closing `>`
params.append_all(&tts);

// Append proxy type parameter (if there aren't any parameters so far,
// we need to add a comma first).
let comma = if params.is_empty() || tts.is_empty() {
quote!{}
} else {
quote! { , }
};
params.append_all(quote! { #comma #proxy_ty_param #ty_bounds });
let impl_generics_ts = impl_generics.into_token_stream();

params
if impl_generics_ts.is_empty() {
quote! { #proxy_ty_param #ty_bounds }
} else {
// If the token stream is not empty, it contains `< >`. We need to
// remove those to add parameters (and we are adding angle brackets
// later anyway).
let mut tokens = impl_generics_ts.into_iter()
.skip(1) // the opening `<`
.collect::<Vec<_>>();
tokens.pop(); // the closing `>`

// Add our parameter with a comma
tokens.extend(quote! { , #proxy_ty_param #ty_bounds });

// Convert back into a `TokenStream`
tokens.into_iter().collect()
}
};


// The tokens after `for` in the impl header (the type the trait is
// implemented for).
let self_ty = match *proxy_type {
ProxyType::Ref => quote! { & #proxy_lt_param #proxy_ty_param },
ProxyType::RefMut => quote! { & #proxy_lt_param mut #proxy_ty_param },
ProxyType::Ref => quote! { &#proxy_ty_param },
ProxyType::RefMut => quote! { &mut #proxy_ty_param },
ProxyType::Arc => quote! { ::std::sync::Arc<#proxy_ty_param> },
ProxyType::Rc => quote! { ::std::rc::Rc<#proxy_ty_param> },
ProxyType::Box => quote! { ::std::boxed::Box<#proxy_ty_param> },
Expand Down