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

Add #[inline] to IpAddr methods #83831

Merged
merged 1 commit into from
Apr 5, 2021
Merged

Add #[inline] to IpAddr methods #83831

merged 1 commit into from
Apr 5, 2021

Conversation

AngelicosPhosphoros
Copy link
Contributor

Add some inlines to trivial methods of IpAddr
Closes #77583

Add some inlines to trivial methods of IpAddr
Closes #77583
@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 3, 2021
@AngelicosPhosphoros
Copy link
Contributor Author

Code from issue:

use std::net::{Ipv4Addr, Ipv6Addr};

pub fn ipv4_bitand_mask(ipv4: Ipv4Addr, mask: u32) -> Ipv4Addr {
    let ipv4_u32 = u32::from_ne_bytes(ipv4.octets());
    Ipv4Addr::from((ipv4_u32 & mask).to_ne_bytes())
}

pub fn ipv6_bitand_mask(ipv6: Ipv6Addr, mask: u128) -> Ipv6Addr {
    let ipv6_u128 = u128::from_ne_bytes(ipv6.octets());
    Ipv6Addr::from((ipv6_u128 & mask).to_ne_bytes())
}

Generated asm:

_ZN9inline_ip16ipv4_bitand_mask17h21a1d58399eeef05E:
	movl	%ecx, %eax
	andl	%edx, %eax
	retq

_ZN9inline_ip16ipv6_bitand_mask17h84a3a89719ccdfc1E:
	movq	%rcx, %rax
	andq	%r8, %rax
	andq	%r9, %rdx
	retq

I think, this change is very trivial.
cc @LeSeulArtichaut @mati865

@AngelicosPhosphoros
Copy link
Contributor Author

r? @LeSeulArtichaut

@LeSeulArtichaut
Copy link
Contributor

I'm not a reviewer. Maybe r? @m-ou-se

@AngelicosPhosphoros
Copy link
Contributor Author

I'm not a reviewer.

Offtop: where can I find list of reviewers?

@m-ou-se
Copy link
Member

m-ou-se commented Apr 4, 2021

Thanks!

@bors r+ rollup

Offtop: where can I find list of reviewers?

The list of reviewers that the high five bot will pick from is here:
https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json#L14-L20

Everyone with bors review rights specifically for the library is listed here: https://www.rust-lang.org/governance/teams/library
(But some of them might only review changes related to a specific topic.)

(And people of the compiler team sometimes also approve library changes that don't affect the public api.)

@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit a3d0fa8 has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@AngelicosPhosphoros
Copy link
Contributor Author

Thanks for the reply.

Offtop: where can I find list of reviewers?

The list of reviewers that the high five bot will pick from is here:
https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json#L14-L20

Everyone with bors review rights specifically for the library is listed here: https://www.rust-lang.org/governance/teams/library
(But some of them might only review changes related to a specific topic.)

(And people of the compiler team sometimes also approve library changes that don't affect the public api.)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 5, 2021
…ine-for-ip, r=m-ou-se

Add `#[inline]` to IpAddr methods

Add some inlines to trivial methods of IpAddr
Closes rust-lang#77583
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 5, 2021
…ine-for-ip, r=m-ou-se

Add `#[inline]` to IpAddr methods

Add some inlines to trivial methods of IpAddr
Closes rust-lang#77583
jyn514 pushed a commit to jyn514/rust that referenced this pull request Apr 5, 2021
…ine-for-ip, r=m-ou-se

Add `#[inline]` to IpAddr methods

Add some inlines to trivial methods of IpAddr
Closes rust-lang#77583
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#81922 (Let `#[allow(unstable_name_collisions)]` work for things other than function)
 - rust-lang#82483 (Use FromStr trait for number option parsing)
 - rust-lang#82739 (Use the beta compiler for building bootstrap tools when `download-rustc` is set)
 - rust-lang#83650 (Update Source Serif to release 4.004)
 - rust-lang#83826 (List trait impls before deref methods in doc's sidebar)
 - rust-lang#83831 (Add `#[inline]` to IpAddr methods)
 - rust-lang#83863 (Render destructured struct function param names as underscore)
 - rust-lang#83865 (Don't report disambiguator error if link would have been ignored)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 445aa40 into rust-lang:master Apr 5, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 5, 2021
@AngelicosPhosphoros AngelicosPhosphoros deleted the issue-77583-inline-for-ip branch April 5, 2021 16:00
@pickfire
Copy link
Contributor

Should there be a test?

@AngelicosPhosphoros
Copy link
Contributor Author

@pickfire You mean codegen test?

@pickfire
Copy link
Contributor

Not sure what is it called but to test the generated assembly.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 16, 2021

I don't see much use in testing that every single #[inline] function in std is actually inlined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poorly optimized assembly generation around Ipv4Addr, Ipv6Addr
8 participants