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

Guard unstable proc_macro things behind nightly feature #26

Merged
merged 13 commits into from
Aug 23, 2018
Merged

Guard unstable proc_macro things behind nightly feature #26

merged 13 commits into from
Aug 23, 2018

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Jul 30, 2018

This does most of the work for #13. The crate now has a nightly feature that is disabled by default. With that feature disabled, we use not-so-nice diagnostics and call-site hygiene. With the feature enabled, we use nice diagnostics and def-site hygiene.

It still doesn't compile on stable, as it uses several Rust 2018 features (as described in #13). But again, those will be stabilized soon. And even if not, they are easy to replace.

The second commit adds two examples that showcase error messages. It's a bit of a hack... I don't know if this is OK like that. (EDIT: I think I don't quite like it. I would probably instead try to add an option to the compile-fail script to just compile a specific example without capturing stdout. But maybe we can still merge it like this and I can change it later.)

This is most of the work for #13. This crate now compiles in two modes:

With "nightly" feature enable, `proc_macro_diagnostic` and
`proc_macro_span` are enable to improve our error messages.

Without the "nightly" feature, we don't use those unstable features
and instead build a workaround for the lack of unstable features
(something like a poly fill). Most of the workaround can be found in
the `diag` module.

The crate doesn't compile with the stable compiler quite yet. There
are still three feature gates activated. But those three features
will be stabilized for Rust 2018 and they are easy to remove by only
rewriting a few lines in the whole crate.
The examples are a bit awkward for some reason: examples are compiled when
one executes `cargo test`. This is nice, but we actually want those examples
to fail. We can't just exclude them, so we only compile the interesting
part when the `fail` cfg is set.

I hope to find a better solution for this...
- no extern crate anymore (except for quote because of #[macro_use])
- replace `pub(crate)` with `crate`
- use lifetime elision in impl
- crate is tested with and without 'nightly' feature
- warnings lead to a build failure
Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

So this looks good to me! I'll try spend a little more time understanding how we're shimming the stable vs nightly APIs and what our plan to unify them is.

src/diag.rs Outdated
use proc_macro::{Diagnostic, Span};
//! This module has two purposes:
//!
//! 1. Provide the convenienve method `emit_with_attr_note` and add it via
Copy link
Member

Choose a reason for hiding this comment

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

nit: little typo here :)

@LukasKalbertodt
Copy link
Member Author

I'll try spend a little more time understanding how we're shimming the stable vs nightly APIs and what our plan to unify them is.

Sure! If you have any questions or discuss anything, you can also just write via email or ping me on Discord (I'm rarely on IRC, sorry!).

But yes, I'm not completely satisfied with these changes. In particular, fewer #[cfg] attributes would be nice. But yeah that's the best I came up with and I figured it's fine for now.


Regarding the examples that are supposed to show an error message: what do you think about removing this cfg hack and instead just comment-out the line that's causing the error. So something like:

    // Add the line below to see the error message.
    //#[auto_impl(Boxxi)]
    trait Foo {
        fn foo(&self) -> u32;
    }

Then we could also combine the fail examples into one example error_messages or so.

I think I would like to change that still. What do you think?

- Do not use in-band lifetimes, as they are probably not stabilized
  in the current form (and thus are not part of Rust 2018)
- Remove `#![feature(rust_2018_preview)]` as it's default now
`extern crate auto_impl` could also be removed :)
@LukasKalbertodt
Copy link
Member Author

@KodrAus I added a bunch of new commits. The commit messages should explain everything, but the main change is to update several files to a newer nightly version (in particular because of rust-lang/rust#50911). Basically, we have fewer #![feature(...)] and fewer extern crate now.

The last commit simplifies the examples. It does what I suggested in the last comment. I think I'm happier with it this way.

Now, the user has to simply add a line that has been commented
to see the error. A lot simpler.
@KodrAus
Copy link
Member

KodrAus commented Aug 23, 2018

Yeh this looks nice and clear 👍 Feel free to merge whenever you're ready!

@LukasKalbertodt
Copy link
Member Author

Wonderful :)

@LukasKalbertodt LukasKalbertodt merged commit 706b86a into auto-impl-rs:master Aug 23, 2018
@LukasKalbertodt LukasKalbertodt mentioned this pull request Aug 23, 2018
7 tasks
@LukasKalbertodt LukasKalbertodt deleted the make-stable branch October 5, 2018 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants