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

feature(proc_macro) breaks attributes on custom derive #39347

Closed
evestera opened this issue Jan 27, 2017 · 10 comments · Fixed by #39572
Closed

feature(proc_macro) breaks attributes on custom derive #39347

evestera opened this issue Jan 27, 2017 · 10 comments · Fixed by #39572
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)

Comments

@evestera
Copy link

Using #![feature(proc_macro)] in combination with attributes enabled by #[proc_macro_derive(MyTrait, attributes(mycrate))] (e.g. #[mycrate(some_option = "something")]) fails to compile on the current nightly with error: macro undefined: 'mycrate!'.

Definition of custom derive:

#![feature(proc_macro)]

use std::str::FromStr;

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_derive(MyTrait, attributes(mycrate))]
pub fn my_macro(_: TokenStream) -> TokenStream {
    TokenStream::from_str("").unwrap()
}

Use which fails:

#![feature(proc_macro)]

#[macro_use]
extern crate proc_macro_collision;

#[derive(MyTrait)]
#[mycrate(some_option = "something")]
struct Foo;

fn main() {
}

If #![feature(proc_macro)] is removed, compilation succeeds.

Compiling the above fails with the following output:

error: macro undefined: 'mycrate!'
 --> examples/with.rs:7:1
  |
7 | #[mycrate(some_option = "something")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Compilation fails on current nightly:

$ rustc --version --verbose
rustc 1.16.0-nightly (df8debf6d 2017-01-25)
binary: rustc
commit-hash: df8debf6d9afc431adbbd8311dcaf2b70eb9762e
commit-date: 2017-01-25
host: x86_64-apple-darwin
release: 1.16.0-nightly
LLVM version: 3.9

However, compilation succeeds on a week old nightly:

$ rustc --version --verbose
rustc 1.16.0-nightly (c07a6ae77 2017-01-17)
binary: rustc
commit-hash: c07a6ae77cd4ceb3cf591d34c5608ca91d1f75d4
commit-date: 2017-01-17
host: x86_64-apple-darwin
release: 1.16.0-nightly
LLVM version: 3.9

This is probably connected to #[proc_macro_attribute] which was merged in this timeframe (PR), and not the feature flag itself.

serde_derive-based example failing the same way:

#![feature(proc_macro)]

#[macro_use]
extern crate serde_derive;

extern crate serde_json;

#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
struct Point {
    x: i32,
    y: i32,
}

fn main() {
    let point = Point { x: 1, y: 2 };

    // Convert the Point to a JSON string.
    let serialized = serde_json::to_string(&point).unwrap();

    // Prints serialized = {"x":1,"y":2}
    println!("serialized = {}", serialized);

    // Convert the JSON string back to a Point.
    let deserialized: Point = serde_json::from_str(&serialized).unwrap();

    // Prints deserialized = Point { x: 1, y: 2 }
    println!("deserialized = {:?}", deserialized);
}

@alexcrichton
Copy link
Member

cc @jseyfried

@jseyfried jseyfried self-assigned this Jan 27, 2017
@abonander
Copy link
Contributor

@jseyfried I think I have an idea where to start on this but I'll need some guidance.

@jseyfried jseyfried added I-needs-decision Issue: In need of a decision. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) P-high High priority and removed P-high High priority labels Jan 28, 2017
@jseyfried
Copy link
Contributor

jseyfried commented Jan 28, 2017

This raises the following question:

// Given a `#[proc_macro_attribute]` `foo`
use attrs::foo;
// and a custom derive `#[proc_macro_derive(Trait, attributes(foo))]`,
use derives::Trait;

#[derive(Trait)] #[foo] struct A; // (1)
#[foo] #[derive(Trait)] struct B; // (2)

Should #[foo] resolve to the #[proc_macro_attribute] or the inert attribute from the proc_macro_derive? More specifically, #[foo]'s scope could be

  • (a) only the #[proc_macro_attribute]
  • (b) both, causing an ambiguity error
    • most future compatible
  • (c) both, and the inert attribute shadows
  • (d) both, and the #[proc_macro_attribute] shadows
    • probably not feasible to implement due to deadlocking

For (1), the scope could be (b) or (c) -- I weakly prefer (b).

For (2), the scope could be (a), (b), or (c) in descending order of my preference.
However, the inert attribute is currently in scope for (2) (this about to land in stable), so (a) is not backwards compatible. However, (b) is backwards compatible and it is straightforward to transition from (b) to (a) with an indefinitely long warning cycle.

cc @nrc @keeperofdakeys

@nrc
Copy link
Member

nrc commented Jan 30, 2017

Hmm, I would go for 'd' in both cases (the user has explicitly imported the name, whereas the inert attr is implicit from the user's POV). But it sounds like this can't be done.

In which case I prefer 'b' for (1) because this feels like the least silent failure. And 'a' for (2) since it looks out of scope for the derive. In both cases the user can address the issue by renaming the import (which is good).

@jseyfried
Copy link
Contributor

jseyfried commented Jan 30, 2017

We decided to

  • implement (b) for (1) and (2) ASAP to fix this issue
  • transition (2) from (b) to (a) with a warning cycle
  • maybe allow unexpanded attribute proc macro names to shadow inert attribute names
    • this is a compromised version of (d) to mitigate deadlocking

@jseyfried jseyfried removed the I-needs-decision Issue: In need of a decision. label Jan 30, 2017
@abonander
Copy link
Contributor

@jseyfried I'm ready to get started working on this but I'm not sure where to start.

@jseyfried
Copy link
Contributor

Fixed in #39572.

bors added a commit that referenced this issue Feb 12, 2017
macros: fix inert attributes from `proc_macro_derives` with `#![feature(proc_macro)]`

This PR refactors collection of `proc_macro_derive` invocations to fix #39347.

After this PR, the input to a `#[proc_macro_derive]` function no longer sees `#[derive]`s on the underlying item. For example, consider:
```rust
extern crate my_derives;
use my_derives::{Trait, Trait2};

#[derive(Copy, Clone)]
#[derive(Trait)]
#[derive(Trait2)]
struct S;
```

Today, the input to the `Trait` derive is `#[derive(Copy, Clone, Trait2)] struct S;`, and the input to the `Trait2` derive is `#[derive(Copy, Clone)] struct S;`. More generally, a `proc_macro_derive` sees all builtin derives, as well as all `proc_macro_derive`s listed *after* the one being invoked.

After this PR, both `Trait` and `Trait2` will see `struct S;`.
This is a [breaking-change], but I believe it is highly unlikely to cause breakage in practice.

r? @nrc
@LukasKalbertodt
Copy link
Member

@abonander I think this bug reappeared. Example:

Cargo.toml

[package]
name = "issue39347"
version = "0.1.0"
authors = ["me"]

[dependencies]
diesel = { version = "0.15.0", features = ["postgres"] }
diesel_codegen = { version = "0.15.0", features = ["postgres"] }

main.rs

#[macro_use] extern crate diesel;
#[macro_use] extern crate diesel_codegen;

table!(
    sessions(id) {
        id -> Integer,
        user_id -> Integer,
    }
);
table!(
    users(id) {
        id -> Integer,
    }
);

#[derive(Debug, Clone, Eq, PartialEq, Identifiable, Queryable, Associations)]
#[belongs_to(User)]
pub struct Session {
    id: i64,
    pub user_id: i64,
}


#[derive(Debug, Clone, Eq, PartialEq, Identifiable, Queryable)]
pub struct User {
    pub id: i64,
}

fn main() {}

The above compiles. But when adding #![feature(proc_macro)] to the top of main.rs, I get this error:

error: cannot find attribute macro `belongs_to` in this scope
  --> src/main.rs:20:3
   |
20 | #[belongs_to(User)]
   |   ^^^^^^^^^^

Tested with rustc 1.21.0-nightly (7ac979d8c 2017-08-16).

Or is this something else?

@abonander
Copy link
Contributor

@jseyfried would know more about this one. I never actually started on it.

@kardeiz
Copy link

kardeiz commented Sep 28, 2017

I am also encountering the issue/bug described by @LukasKalbertodt above, in almost exactly the same context, with a recent nightly rustc 1.22.0-nightly (0e6f4cf51 2017-09-27). Is there any workaround available or resolution in the works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants