Skip to content

Span doesn't implement PartialEq and Eq #1320

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
c410-f3r opened this issue Mar 6, 2019 · 3 comments
Closed

Span doesn't implement PartialEq and Eq #1320

c410-f3r opened this issue Mar 6, 2019 · 3 comments
Labels

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented Mar 6, 2019

Span doesn't implement PartialEq and Eq, making the extra-traits feature broken.
Should PartialEq and Eq be removed or is it better to wait for an implementation?

Related: #154

#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))]
#[derive(Clone)]
pub struct Function {
pub name: String,
pub name_span: Span,
pub renamed_via_js_name: bool,
pub arguments: Vec<syn::ArgCaptured>,
pub ret: Option<syn::Type>,
pub rust_attrs: Vec<syn::Attribute>,
pub rust_vis: syn::Visibility,
}

@c410-f3r c410-f3r added the bug label Mar 6, 2019
@alexcrichton
Copy link
Contributor

Oh dear this does seem bad! We probably want to remove this feature for now in that case? (removing PartialEq and Eq also seems fine to me)

@dtolnay
Copy link

dtolnay commented Apr 22, 2019

I notice that ast::Function stores name: String and name_span: Span as separate fields. I would recommend storing these as one field of type proc_macro2::Ident which does have a PartialEq impl, or if the name is not guaranteed to be a legal Rust identifier/keyword then writing your own type similar to Ident to hold the name and span and provide PartialEq and any other traits you need.

@c410-f3r
Copy link
Contributor Author

@dtolnay Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants