Skip to content
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

Rustfmt drops 'default' from 'default fn …' using specialization #945

Closed
killercup opened this issue Apr 16, 2016 · 10 comments
Closed

Rustfmt drops 'default' from 'default fn …' using specialization #945

killercup opened this issue Apr 16, 2016 · 10 comments
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors p-high

Comments

@killercup
Copy link
Member

Since specialization is only available in nightly (and maybe still changing), it's not much of a priority, though.

cf. rust-lang/miri#6

@marcusklaas marcusklaas added good first issue Issues up for grabs, also good candidates for new rustfmt contributors bug Panic, non-idempotency, invalid code, etc. and removed bug Panic, non-idempotency, invalid code, etc. labels Apr 16, 2016
@zitsen
Copy link

zitsen commented May 19, 2016

I'm waiting for this impl.

@nrc nrc added the p-high label May 31, 2016
@nrc nrc added this to the 1.0 milestone May 31, 2016
@julienXX
Copy link
Contributor

I'd like to try to fix this one if no one is already working on it.

@regexident
Copy link
Contributor

line hint (spoiler alert! 😉 )

@lqd
Copy link
Member

lqd commented May 31, 2016

To me it looked like the syntex visitor didn't pass the defaultness ? (but I'm unfamiliar with both syntex and rustfmt so I could be very mistaken)

@regexident
Copy link
Contributor

To me it looked like the syntex visitor didn't pass the defaultness ? (but I'm unfamiliar with both syntex and rustfmt so I could be very mistaken)

That's true in fact, afaikt:

syntax::ast::TraitItemKind::Method(MethodSig, Option<P<Block>>)

pub struct MethodSig {
    pub unsafety: Unsafety,
    pub constness: Constness,
    pub abi: Abi,
    pub decl: P<FnDecl>,
    pub generics: Generics,
}

@lqd
Copy link
Member

lqd commented May 31, 2016

@julienXX salut Julien ;)

I think I'm close-ish to making a PR as I was talking about this issue with @nrc on IRC earlier today, would you mind if I took a crack at it ?

@julienXX
Copy link
Contributor

@lqd no problem 👍

marcusklaas pushed a commit that referenced this issue May 31, 2016
Adds support for Defaultness on impl methods.
Fixes #945
@nrc
Copy link
Member

nrc commented Jun 1, 2016

@julienXX if you want to work on a rustfmt, there are a bunch of good issues - https://github.com/rust-lang-nursery/rustfmt/issues?q=is%3Aopen+is%3Aissue+label%3Aeasy - I'm happy to mentor any of them - either ping me on irc or comment on the bug.

@julienXX
Copy link
Contributor

julienXX commented Jun 1, 2016

@nrc will do, thanks :)

@jaredthecoder
Copy link

@nrc I'd love to jump on some of those. I'll find you on IRC if I have any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors p-high
Projects
None yet
Development

No branches or pull requests

8 participants