Skip to content

Add inline to every String function #20859

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

Merged
merged 1 commit into from
Jan 11, 2015
Merged

Add inline to every String function #20859

merged 1 commit into from
Jan 11, 2015

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Jan 10, 2015

Closes #20822

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@@ -138,6 +138,7 @@ impl String {
/// let output = String::from_utf8_lossy(input);
/// assert_eq!(output.as_slice(), "Hello \u{FFFD}World");
/// ```
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced this should be marked inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there were an attribute #[can_be_inlined] I would have used that. Adding #[inline] is the only way to let llvm decide if it should be inlined.

@nikomatsakis
Copy link
Contributor

I think I agree with @pnkfelix; the one-line functions seem like no brainers, those that contain non-trivial loops, less clearly so. I think I'm inclined to leave the #[inline] of those fns for now. (Do you have an example of some real program that is slower as a result or something?)

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 10, 2015

Updated

bors added a commit that referenced this pull request Jan 10, 2015
bors added a commit that referenced this pull request Jan 11, 2015
@bors bors merged commit a03ae68 into rust-lang:master Jan 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark String::as_mut_vec inline
5 participants