Skip to content

Remove #[inline] from generic functions in core::range#155420

Closed
tbu- wants to merge 1 commit intorust-lang:mainfrom
tbu-:pr_ranges_inline
Closed

Remove #[inline] from generic functions in core::range#155420
tbu- wants to merge 1 commit intorust-lang:mainfrom
tbu-:pr_ranges_inline

Conversation

@tbu-
Copy link
Copy Markdown
Contributor

@tbu- tbu- commented Apr 17, 2026

They shouldn't be needed according to https://std-dev-guide.rust-lang.org/policy/inline.html.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 17, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 17, 2026

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @scottmcm, libs
  • @scottmcm, libs expanded to 7 candidates
  • Random selection from Mark-Simulacrum, jhpratt, scottmcm

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 17, 2026

☔ The latest upstream changes (presumably #155416) made this pull request unmergeable. Please resolve the merge conflicts.

Comment thread library/core/src/range.rs
/// assert!(!Range::from(0.0..f32::NAN).contains(&0.5));
/// assert!(!Range::from(f32::NAN..1.0).contains(&0.5));
/// ```
#[inline]
Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 17, 2026

Choose a reason for hiding this comment

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

I know that the Rust stdlibs dev guide tells you that you shouldn't need to #[inline] methods that have generics involved, but I do see that the previous Range types inline for methods like these.

Unsure if this would cause performance regression and if the rust compiler inlines this automatically without this attribute.

View changes since the review

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Apr 18, 2026

While I believe you're correct, having the attribute present doesn't harm anything and is clearer to anyone reading the code.

@jhpratt jhpratt closed this Apr 18, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2026
@tbu-
Copy link
Copy Markdown
Contributor Author

tbu- commented Apr 18, 2026

@jhpratt Should the dev guide be updated so it doesn't recommend me to do this?

@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Apr 18, 2026

I don't see anything saying the attribute should be actively removed, only that it's not necessary on a technical level. Feel free to bring it up in Zulip if you'd like; I don't feel strongly about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants