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

#[inline] is no longer accepted on closures #49632

Closed
Amanieu opened this issue Apr 4, 2018 · 4 comments
Closed

#[inline] is no longer accepted on closures #49632

Amanieu opened this issue Apr 4, 2018 · 4 comments
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Apr 4, 2018

Since #49291 #[inline] is no longer accepted on closures, despite the fact that this attribute does have an effect on code generation.

The generated LLVM IR (in debug builds) for this example is different if you remove #[inline(always)]:

#![feature(stmt_expr_attributes)] 
#![crate_type="rlib"]

pub fn main() {
    let x = #[inline(always)] || {};
    x();
}
@sfackler sfackler added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 4, 2018
@sanxiyn
Copy link
Member

sanxiyn commented Apr 5, 2018

While I think this should be fixed, this is not exactly a regression, since stmt_expr_attributes is unstable and this code never worked on stable.

@sanxiyn sanxiyn removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Apr 5, 2018
@tejom
Copy link
Contributor

tejom commented Apr 6, 2018

I can make a change, since I wrote that original patch. I'm curious if a better change would be to have closures always inlined? That seems like it might have been the expected behavior and the reason why it was suggested to me to write the check like this in the first place?

@Amanieu
Copy link
Member Author

Amanieu commented Apr 6, 2018

I believe closures are currently always effectively #[inline]. However in some cases I need to force inlining with #[inline(always)] even when LLVM thinks it is a bad idea.

@tejom
Copy link
Contributor

tejom commented Apr 6, 2018

I would just change the check_expr_attributes function in librustc/hir/check_attr.rs to check the variant of hir::Expr passed in when the attribute is inline. if its ExprClosure then don't emit an error. But Id prefer to have someone who does the reviews say this is what they want instead of having a surprise at the pull request stage.

Amanieu added a commit to Amanieu/rust that referenced this issue Apr 27, 2018
kennytm added a commit to kennytm/rust that referenced this issue Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants