Skip to content

derive-new errors #66525

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
nikomatsakis opened this issue Nov 18, 2019 · 4 comments · Fixed by #66529
Closed

derive-new errors #66525

nikomatsakis opened this issue Nov 18, 2019 · 4 comments · Fixed by #66529
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

So @wycats pointed out to me that some recent changes in name resolution seem to have broken derive_new. They pointed me at this commit from nushell dealing with the fallout -- @jonathandturner maybe you can leave a few notes as to what errors you were seeing before this commit?

cc @petrochenkov -- could this be the recent changes around name resolution of helper attributes in macros? It seems the relevant code is stuff like:

#[derive(new)]
struct Foo {
    #[new(value = "Arc::new(22)")]
    bar: Arc<i32>
}
@nikomatsakis nikomatsakis added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 18, 2019
@nikomatsakis
Copy link
Contributor Author

(Nominated for @rust-lang/lang discussion around the breakage policy/implications here.)

@petrochenkov
Copy link
Contributor

Duplicate of #66508, I'll prepare a fix.

@petrochenkov
Copy link
Contributor

fix

Or rather a "tiebreaker" for the ambiguity, since both names are indeed in scope and create ambiguity, which is correctly detected after #64694.

@petrochenkov petrochenkov mentioned this issue Nov 18, 2019
Merged
@petrochenkov
Copy link
Contributor

Fixed in #66529.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 19, 2019
@JohnTitor
Rollup merge of rust-lang#66529 - petrochenkov:reshelp2, r=davidtwco
4acec41 
resolve: Give derive helpers highest priority during resolution

So they just shadow everything else and don't create ambiguity errors.
This matches the old pre-rust-lang#64694 behavior most closely.

---
The change doesn't apply to this "compatibility" case
```rust
#[trait_helper] // The helper attribute is used before it introduced.
                        // Sadly, compiles on stable, supported via hacks.
                        // I plan to make a compatibility warning for this.
#[derive(Trait)]
struct S;
```
, such attributes still create ambiguities, but rust-lang#64694 didn't change anything for this case.

Fixes rust-lang#66508
Fixes rust-lang#66525
@bors bors closed this as completed in 1f0f0ad Nov 19, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

resolve: Give derive helpers highest priority during resolution petrochenkov/rust
2 participants
@nikomatsakis @petrochenkov