Skip to content

Avoid instaiblity errors in code generated by syntax_ext::deriving::call_intrinsic() #36316

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

Merged
merged 2 commits into from
Sep 8, 2016

Conversation

jseyfried
Copy link
Contributor

#35957 made old-style attribute syntax extensions no longer allow internal instability.

r? @eddyb

@jseyfried
Copy link
Contributor Author

jseyfried commented Sep 7, 2016

cc @nrc @nox

callee: codemap::NameAndSpan {
format: codemap::MacroAttribute(intern(&format!("derive({})", tname))),
span: Some(titem.span),
allow_internal_unstable: true,
Copy link
Member

Choose a reason for hiding this comment

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

What about adding this block of code to call_intrinsic instead? It's about the same amount of work and wouldn't hide unstable feature usage like the previous implementation did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done.

@jseyfried jseyfried force-pushed the custom_derive_internal_unstable branch from 173b47d to 8eeb57c Compare September 7, 2016 04:13
@jseyfried jseyfried changed the title Revert old-style attribute syntax extensions back to allowing internal instability Avoid instaiblity errors in code generated by syntax_ext::deriving::call_intrinsic() Sep 7, 2016
@eddyb
Copy link
Member

eddyb commented Sep 7, 2016

You could probably now replace allow_internal_unstable: true which is done for builtin derives.
But r=me either way, thanks!

@jseyfried jseyfried force-pushed the custom_derive_internal_unstable branch 3 times, most recently from ad75287 to f5f11b3 Compare September 7, 2016 06:56
@jseyfried jseyfried force-pushed the custom_derive_internal_unstable branch from f5f11b3 to d6ea10e Compare September 7, 2016 07:39
@jseyfried
Copy link
Contributor Author

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Sep 7, 2016

📌 Commit d6ea10e has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Sep 8, 2016

⌛ Testing commit d6ea10e with merge 0b02ae0...

bors added a commit that referenced this pull request Sep 8, 2016
…ddyb

Avoid instaiblity errors in code generated by `syntax_ext::deriving::call_intrinsic()`

r? @eddyb
@bors bors merged commit d6ea10e into rust-lang:master Sep 8, 2016
@nrc
Copy link
Member

nrc commented Sep 9, 2016

cc @alexcrichton.

@alexcrichton
Copy link
Member

Gah, sorry about the regression! It's not clear to me here what actually regressed, but the solution looks fine.

@jseyfried jseyfried deleted the custom_derive_internal_unstable branch September 9, 2016 19:33
@jseyfried
Copy link
Contributor Author

@alexcrichton no worries.
What regressed was some "old-style" custom derives that were implemented using the framework in libsyntax_ext/deriving/generic/mod.rs, which sometimes uses call_intrinsic-generated code.

@alexcrichton
Copy link
Member

Ah I see, that'd explain the lack of regressions in the main repo!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants