Change #[inline(always)] to #[inline(force)]
#6987
Conversation
|
What criteria are you using to determine when |
|
I do sort of like how "force" sounds more dire than "always", to imply that this is a tool that you should use with caution. But I'd like a dev to determine if this naming change is desirable. |
|
I like it too. A related possibility would be to rename |
|
@bstrie my criteria was based on whether it should be forced, rather than whether it shouldn't. Most functions that implemented the for protocol were forced, as LLVM has issues there. The majority of operator overloads are forced, a couple of functions that had a comment saying that they should be forced. Everything else is marked with just inline. The major reason for many of the inlined functions is to get it to output the AST for cross-crate inlining. LLVM will potentially inline any function if it thinks it will help. |
|
@glehel technically that does work as expected, as if we don't recognise the contents of the inline, we just treat it as a regular inline hint. |
|
Why are you changing 'always' to 'force'? |
|
@brson as @bstrie pointed out, 'force' sounds more "dire". The idea is that 'force' actually better describes what your are doing. I mean, sure they mean the same thing, but you are actually forcing the compiler to inline the function. 'always' still seems like a request, it's too off-hand with respect to what you are asking. 'always' also suggests that the developer knows what they are doing, and when it comes to this kind of optimization. They normally don't. Most tools that have a 'force' option, have it as a way to override some default safety check, be it pushing a diverged branch in Lastly, people have abused the feature, marking rather large functions as tl;dr: (P.S. I will rebase if/when this gets provisionally approved) |
|
Can I get an update here? The longer this waits the more painfully it's going to be to rebase. |
|
@Aatch Try bringing it up on the mailing list or ask that it be discussed in the meeting today. |
|
I don't favor the change to |
|
I prefer using the same names that LLVM uses. |
|
This is probably implicit in Niko's remark: LLVM uses the names "always" and "never". (At least, that's what I garner from the docs for InlineCost.h) |
Don't trigger `same_item_push` if the vec is used in the loop body fixes rust-lang#6987 changelog: `same_item_push`: Don't trigger if the `vec` is used in the loop body
This pull request changes
#[inline(always)]to#[inline(force)]. The old version now emits a warning, but still maintains thealways_inlinebehaviour.The libraries all have been changed, so no warning there. I also de-forced a lot of functions/methods, though none have had the
inlineattribute removed completely. I also added#[inline]to several functions that seemed like good candidates.I haven't checked to see if this changes the compilation time, I don't think it will.