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

Macros 1.1 broken on nightly-2016-10-20 #485

Closed
shssoichiro opened this Issue Oct 21, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@shssoichiro
Contributor

shssoichiro commented Oct 21, 2016

If I try to compile a program which uses diesel 0.8.0, using rustc nightly-2016-10-20, the compilation fails in my crate with the following errors:

error: `Identifiable` is a derive mode
  --> src/models/account.rs:16:1
   |
16 | pub struct Account {
   | ^

error: `Queryable` is a derive mode
  --> src/models/account.rs:16:1
   |
16 | pub struct Account {
   | ^

error: `Insertable` is a derive mode
  --> src/models/account.rs:45:1
   |
45 | pub struct NewAccount {
   | ^

The program compiles and runs successfully on nightly-2016-10-19. Unless this is a bug in the latest nightly, the assumption that macros 1.1 would keep diesel from breaking with nightly updates may have been incorrect.

@sgrif

This comment has been minimized.

Member

sgrif commented Oct 22, 2016

I think this is a bug in the latest nightly, but I will investigate further.

@oeb25

This comment has been minimized.

oeb25 commented Oct 23, 2016

FYI using the macro around the struct results in the same error eg.

// src/models.rs
Queryable! { struct Thing { ... } }
error: `Queryable` is a derive mode
 --> src/models.rs:3:1
  |
3 | Queryable! { struct Thing { ... } }
  | ^^^^^^^^^

having

// src/lib.rs
extern crate dotenv;

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

use diesel::prelude::*;
use diesel::pg::PgConnection;
use dotenv::dotenv;
use std::env;

pub mod schema;
pub mod models;

Any update on this? I read jseyfried's comment on the issue on the rust repo, but I'm not entirely sure what this means for Diesel.

Does this require a change in Diesel, user code or are we waiting for a fix in rust?

@SergioBenitez

This comment has been minimized.

Contributor

SergioBenitez commented Oct 25, 2016

Any update on this? Just hit this bug.

@sgrif

This comment has been minimized.

Member

sgrif commented Oct 30, 2016

I'm hoping to get this fixed upstream.

@jseyfried jseyfried referenced this issue Oct 31, 2016

Closed

Tracking issue for "Macros 1.1" (RFC #1681) #35900

50 of 53 tasks complete
@SergioBenitez

This comment has been minimized.

Contributor

SergioBenitez commented Oct 31, 2016

@sgrif That seems like a direction that might take some time, if it is resolved in your favor at all. The issue seems to be that a recent change put macros and derives in the same namespace, thus Queryable! and [derive(Queryable)] collide. It does not seem like it would be onerous to rename one of the two, presumably the macro, to something different. Alternatively, you could provide a duplicate macro with a different name (say, Queryable_) that is only used for code generation so that Queryable can still be used by others. This would resolve the conflict while keeping the ergonomics you're presumably aiming for.

sgrif added a commit that referenced this issue Nov 1, 2016

Fix failures on latest nightlies
Macros 1.1 was changed so that bang macros and custom derive macros
share the same namespace. This causes problems for us, since we expose
and use a bang macro with the same name as the custom derive macro. I
was hoping we could have this changed upstream, but
rust-lang/rust#35900 (comment)
makes it seem like that's not going to happen. I'm not sure what my long
term plan for the bang macros are, but in the short term we can just
expose them under another name internally. Those who are using the bang
macros are likely not using them in conjunction with `diesel_codegen`.

Fixes #485.

@sgrif sgrif closed this in 4f38cc8 Nov 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment