Skip to content

Make syntex optional with nightly rustc #270

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 1 commit into from

Conversation

jdub
Copy link
Contributor

@jdub jdub commented Nov 15, 2016

The start of a change, suggested by @emilio, that will allow users of nightly rustc to drop syntex. It doesn't currently build -- probably requires aster and quasi updates. That might get hairy, so consider this a work in progress and a proposal that can be quickly refused if it's totally off base.

This depends on #269 so currently includes those commits.

@jdub
Copy link
Contributor Author

jdub commented Nov 15, 2016

Here's a compare and contrast, using cargo-tree

libbindgen with syntex:

libbindgen v0.17.0 (file:///tmp/rust-bindgen/libbindgen)
├── aster v0.33.1
│   └── syntex_syntax v0.48.0
│       ├── bitflags v0.7.0
│       ├── libc v0.2.17
│       ├── log v0.3.6
│       ├── rustc-serialize v0.3.21
│       ├── syntex_errors v0.48.0
│       │   ├── libc v0.2.17 (*)
│       │   ├── log v0.3.6 (*)
│       │   ├── rustc-serialize v0.3.21 (*)
│       │   ├── syntex_pos v0.48.0
│       │   │   └── rustc-serialize v0.3.21 (*)
│       │   ├── term v0.4.4
│       │   │   ├── kernel32-sys v0.2.2
│       │   │   │   └── winapi v0.2.8
│       │   │   └── winapi v0.2.8 (*)
│       │   └── unicode-xid v0.0.3
│       ├── syntex_pos v0.48.0 (*)
│       ├── term v0.4.4 (*)
│       └── unicode-xid v0.0.3 (*)
├── cexpr v0.2.0
│   └── nom v1.2.4
├── cfg-if v0.1.0
├── clang-sys v0.8.1
│   ├── bitflags v0.7.0 (*)
│   ├── glob v0.2.11
│   └── libc v0.2.17 (*)
├── env_logger v0.3.5
│   ├── log v0.3.6 (*)
│   └── regex v0.1.80
│       ├── aho-corasick v0.5.3
│       │   └── memchr v0.1.11
│       │       └── libc v0.2.17 (*)
│       ├── memchr v0.1.11 (*)
│       ├── regex-syntax v0.3.9
│       ├── thread_local v0.2.7
│       │   └── thread-id v2.0.0
│       │       ├── kernel32-sys v0.2.2 (*)
│       │       └── libc v0.2.17 (*)
│       └── utf8-ranges v0.1.3
├── lazy_static v0.1.16
├── libc v0.2.17 (*)
├── log v0.3.6 (*)
├── quasi v0.25.1
│   ├── syntex_errors v0.48.0 (*)
│   └── syntex_syntax v0.48.0 (*)
├── regex v0.1.80 (*)
├── rustc-serialize v0.3.21 (*)
└── syntex_syntax v0.48.0 (*)

libbindgen without syntex:

libbindgen v0.17.0 (file:///tmp/rust-bindgen/libbindgen)
├── aster v0.33.1
├── cexpr v0.2.0
│   └── nom v1.2.4
├── cfg-if v0.1.0
├── clang-sys v0.8.1
│   ├── bitflags v0.7.0
│   ├── glob v0.2.11
│   └── libc v0.2.17
├── lazy_static v0.1.16
├── libc v0.2.17 (*)
├── quasi v0.25.1
├── regex v0.1.80
│   ├── aho-corasick v0.5.3
│   │   └── memchr v0.1.11
│   │       └── libc v0.2.17 (*)
│   ├── memchr v0.1.11 (*)
│   ├── regex-syntax v0.3.9
│   ├── thread_local v0.2.7
│   │   └── thread-id v2.0.0
│   │       ├── kernel32-sys v0.2.2
│   │       │   └── winapi v0.2.8
│   │       └── libc v0.2.17 (*)
│   └── utf8-ranges v0.1.3
└── rustc-serialize v0.3.21

(Oops, that's slightly overstated, because --no-default-features also drops the logging feature. But hey, here's what you get with both turned off.)

@emilio
Copy link
Contributor

emilio commented Nov 16, 2016

From the travis logs it seems after updating quasi_codegen the macros we define are broken? gah, that's a nasty bug... we can probably work around it, but I'd prefer not.

@jdub
Copy link
Contributor Author

jdub commented Nov 16, 2016

Yeah, from helpers.rs:

    pub fn float_kind_rust_type(ctx: &BindgenContext,
                                fk: FloatKind)
                                -> P<ast::Ty> {
        macro_rules! raw {
            ($ty: ident) => {
                raw_type(ctx, stringify!($ty))
            }
        }
...
        match (fk, ctx.options().convert_floats) {
...

And from the generated codegen.rs:

        pub fn float_kind_rust_type(ctx: &BindgenContext, fk: FloatKind)
         -> P<ast::Ty> {
            match (fk, ctx.options().convert_floats) {
...

I'll contact upstream.

@jdub
Copy link
Contributor Author

jdub commented Nov 16, 2016

It's a known issue upstream: serde-rs/serde#608

@bors-servo
Copy link

☔ The latest upstream changes (presumably #269) made this pull request unmergeable. Please resolve the merge conflicts.

@jdub jdub force-pushed the nightly-syntax branch 3 times, most recently from 8246993 to 29f6c69 Compare November 17, 2016 07:35
@nox
Copy link
Contributor

nox commented Nov 17, 2016

I'm -1 on this, libsyntax is one of the most unstable crates in Rust private parts. Could we just switch to quote and friends?

@jdub
Copy link
Contributor Author

jdub commented Nov 17, 2016

Hmm, I'm not sure I'm clever enough to help with that… but then, I've enjoyed my little adventure through bindgen thus far, so I'll take a look.

@jdub
Copy link
Contributor Author

jdub commented Nov 17, 2016

Oh, maybe this isn't so hard. (Probably saying that too soon.)

@bors-servo
Copy link

☔ The latest upstream changes (presumably #275) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably #329) made this pull request unmergeable. Please resolve the merge conflicts.

@emilio
Copy link
Contributor

emilio commented Feb 13, 2017

I believe this can be closed for now. Not sure if it's still needed, we're going to transition from syntext to syn soon-ish, I guess :)

@emilio emilio closed this Feb 13, 2017
@dtolnay
Copy link
Member

dtolnay commented Feb 13, 2017

There is another parser that might be a better fit - https://github.com/shepmaster/strata-rust. It is pretty young at this point but looks super promising.

@jdub jdub deleted the nightly-syntax branch February 13, 2017 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants