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

Add test for expansion inside of cfg_attr #382

Closed
wants to merge 1 commit into from
Closed

Add test for expansion inside of cfg_attr #382

wants to merge 1 commit into from

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jun 12, 2016

Test for #148. Requires serde-deprecated/syntex#71 + Syntex release.

@erickt
Copy link
Member

erickt commented Jun 16, 2016

r=me once serde-deprecated/syntex#71 lands.

@dtolnay dtolnay added the wip label Jun 20, 2016
@dtolnay
Copy link
Member Author

dtolnay commented Jun 20, 2016

I marked this WIP until we get a Syntex release to depend on. As far as I know nobody is waiting urgently for this so we can just wait for the next libsyntax break.

@erickt
Copy link
Member

erickt commented Jun 23, 2016

@homu: r+ 675caa8

homu added a commit that referenced this pull request Jun 23, 2016
Add test for expansion inside of cfg_attr

Test for #148. Requires serde-deprecated/syntex#71 + Syntex release.
@erickt
Copy link
Member

erickt commented Jun 23, 2016

@dtolnay: syntex is updated, but this is failing on stable and beta with things like:

/Users/erickt/projects/rust/serde/serde/serde_tests/target/debug/build/serde_tests-0dc644810d6621ba/out/test.rs:16260:9: 16260:22 error: the trait bound `test_macros::NamedUnit: std::cmp::PartialEq` is not satisfied [E0277]
/Users/erickt/projects/rust/serde/serde/serde_tests/target/debug/build/serde_tests-0dc644810d6621ba/out/test.rs:16260         assert_tokens(&NamedUnit, vec!(Token :: UnitStruct ( "NamedUnit" )));

Do you know what's up? Maybe the bug hasn't been fixed in syntex yet.

@dtolnay
Copy link
Member Author

dtolnay commented Jun 23, 2016

That's so strange. It works on the nightly compiler using Syntex, but not on the beta compiler using Syntex. What would be the difference between those?

The generated code looks correct in both cases:

#[cfg_attr(all(), derive(PartialEq, Debug))]
struct NamedUnit;
#[cfg(all())]
const _IMPL_DESERIALIZE_FOR_NamedUnit: () = { /* ... */ };
#[cfg(all())]
const _IMPL_SERIALIZE_FOR_NamedUnit: () = { /* ... */ };

@dtolnay
Copy link
Member Author

dtolnay commented Jun 23, 2016

Interesting, cargo expand on beta shows #[derive(PartialEq, Debug)] but on nightly it shows them expanded. That must have something to do with it. Now to figure out why...

@dtolnay
Copy link
Member Author

dtolnay commented Jun 23, 2016

#[cfg_attr(all(), cfg_attr(all(), derive(Debug)))]
struct S;

fn main() {
    println!("{:?}", S);
}

This works on nightly but not stable/beta. Playground link

I am going to write this off as a bug that needed to be fixed in both Syntex and rustc. We can add the test later when we support only versions that contain the rustc side of the fix.

@dtolnay dtolnay removed the wip label Jun 23, 2016
@dtolnay dtolnay closed this Jun 23, 2016
@dtolnay dtolnay deleted the cfg branch June 23, 2016 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants