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

Don't throw away item attributes before trans_fn() for generic functions and others. #30661

Merged
merged 2 commits into from Jan 4, 2016

Conversation

Projects
None yet
5 participants
@michaelwoerister
Copy link
Contributor

michaelwoerister commented Dec 31, 2015

So far librustc::trans::base::trans_fn() and trans_closure() have been passed the list of attributes on the function being translated only if the function was local and non-generic. For generic functions, functions inlined from other crates, functions with foreign ABI and for closures, only an empty list of attributes was ever passed to trans_fn().
This led to the case that generic functions marked with #[rustc_mir] where not actually translated via MIR but via the legacy translation path.

This PR makes function/closure attributes always be passed to trans_fn() and disables the one test where this makes a difference.

If there is an actual reason why attributes were not passed along in these cases, let me know.

cc @rust-lang/compiler
cc @luqmana regarding the test case

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Dec 31, 2015

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@michaelwoerister michaelwoerister force-pushed the michaelwoerister:trans_fn_attrs branch from 43844a2 to ff93fc8 Dec 31, 2015

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Dec 31, 2015

Looks generally good to me, but I’d like a separate issue for that MIR thing filled (it is much easier to work when you can just pick an arbitrary issue with A-mir label to work on).

@michaelwoerister

This comment has been minimized.

Copy link
Contributor Author

michaelwoerister commented Jan 1, 2016

@nagisa Good idea. I've created issue #30674 describing the problem.

@nrc

This comment has been minimized.

Copy link
Member

nrc commented Jan 4, 2016

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 4, 2016

📌 Commit ff93fc8 has been approved by nrc

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 4, 2016

⌛️ Testing commit ff93fc8 with merge 41611ba...

bors added a commit that referenced this pull request Jan 4, 2016

Auto merge of #30661 - michaelwoerister:trans_fn_attrs, r=nrc
So far `librustc::trans::base::trans_fn()` and `trans_closure()` have been passed the list of attributes on the function being translated *only* if the function was local and non-generic. For generic functions, functions inlined from other crates, functions with foreign ABI and for closures, only an empty list of attributes was ever passed to `trans_fn()`.
This led to the case that generic functions marked with `#[rustc_mir]` where not actually translated via MIR but via the legacy translation path.

This PR makes function/closure attributes always be passed to `trans_fn()` and disables the one test where this makes a difference.

If there is an actual reason why attributes were not passed along in these cases, let me know.

cc @rust-lang/compiler
cc @luqmana regarding the test case

@bors bors merged commit ff93fc8 into rust-lang:master Jan 4, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.