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

structs with public members get extra pub prefix #95

Closed
jonhoo opened this issue Jun 4, 2015 · 5 comments
Closed

structs with public members get extra pub prefix #95

jonhoo opened this issue Jun 4, 2015 · 5 comments
Labels
bug Panic, non-idempotency, invalid code, etc.

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Jun 4, 2015

rustfmt adds an extra pub modifier to public fields in structs, causing the code to no longer compile.

// before rustfmt
struct X {
    pub x : u64,
}

// after rustfmt
struct X {
    pub    pub x: u64,
}
@nrc nrc added the bug Panic, non-idempotency, invalid code, etc. label Jun 4, 2015
@chellmuth
Copy link
Contributor

I think the root cause is a bug in rustc. It looks like the span for StructField starts at the field name instead of the pub keyword.

After I applied this patch to libsyntax/parse/parser.rs in rustc, rustfmt handled public struct fields correctly:

diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
     /// Parse a structure field
     fn parse_name_and_ty(&mut self, pr: Visibility,
                          attrs: Vec<Attribute> ) -> PResult<StructField> {
-        let lo = self.span.lo;
+        let lo = match pr {
+            Inherited => { self.span.lo }
+            Public => { self.last_span.lo }
+        };
         if !self.token.is_plain_ident() {
             return Err(self.fatal("expected ident"));
         }
         let name = try!(self.parse_ident());
         try!(self.expect(&token::Colon));
         let ty = try!(self.parse_ty_sum());
         Ok(spanned(lo, self.last_span.hi, ast::StructField_ {
             kind: NamedField(name, pr),
             id: ast::DUMMY_NODE_ID,
             ty: ty,
             attrs: attrs,
         }))
     }

@nrc: Can you confirm this is a rustc bug and not intentional behavior? It seems like a bug to me, but I'm new to compiler internals. I can open a pull request to get this change applied.

@nrc
Copy link
Member

nrc commented Jun 7, 2015

@chellmuth nice find! Yeah, rustc looks wrong here. Please could you open a PR against Rust?

@chellmuth
Copy link
Contributor

Issue: rust-lang/rust#26083
PR: rust-lang/rust#26084

@chellmuth
Copy link
Contributor

This is fixed on the latest nightly, #100 adds a regression test

@nrc
Copy link
Member

nrc commented Jun 10, 2015

Fixed by #100

@nrc nrc closed this as completed Jun 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

3 participants