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

Jump to bottom

proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr #56286

Closed
zhlsunshine opened this issue Oct 18, 2022 · 8 comments
Closed

proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr #56286

zhlsunshine opened this issue Oct 18, 2022 · 8 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@zhlsunshine
Copy link

zhlsunshine commented Oct 18, 2022 •
edited

Abstract
As there are many advantages for using package net/netip instead of net IP related, many projects need to do such conversion since golang 1.18. More detail can be refer to blog

Background
There are structs in package net to handle IP address and port, such as net.IP, net.IPNet, net.IPAddr and net.TCPAddr, and there is an interface named Addr in package net with 2 APIs: Network() and String() to handle above structs.
And there are structs named Addr, Prefix and AddrPort in package net/netip as well, and these struct have only implemented function String().

Design
Generally, implementation for networking address in package net can be replaced by net/netip with many advantages, and want package net/netip can be used in more, such as using for Listener interface in package net, they should be implemented the interface Addr in net.
Implement the interface Addr in package net can help net/netip being used in more, so just implement the function Network() for structs Addr and AddrPort in net/netip, like PR: #56264
@gopherbot gopherbot added this to the Proposal milestone Oct 18, 2022
@zhlsunshine zhlsunshine changed the title proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr non-proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr Oct 18, 2022
@zhlsunshine zhlsunshine changed the title non-proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr proposal: net/netip: Add function Network() for netip Addr to make it support the interface of net.Addr Oct 18, 2022
@zhlsunshine zhlsunshine reopened this Oct 18, 2022
@ianlancetaylor
Copy link
Contributor

CC @josharian @bradfitz @neild

@zhlsunshine
Copy link
Author

Hi @josharian @bradfitz and @neild, is there any comment or progress for this proposal? Thanks!

@rsc
Copy link
Contributor

rsc commented Oct 26, 2022

net.IP does not have a Network() string method, so it does not seem like netip.Addr should have one either,
since netip.Addr is supposed to be a better net.IP.

It is confusing that netip.Addr is not a net.Addr, but it's not.

@rsc
Copy link
Contributor

rsc commented Oct 26, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@zhlsunshine
Copy link
Author

zhlsunshine commented Nov 2, 2022 •
edited

Hi @rsc , Thanks! Both net.IP and netip.Addr are unnecessary to have Network(), but netip.Prefix and netip.AddrPort may be need to implement the interface net.Addr.
BTW, besides net.IP, netip.Addr may need to compare with net.IPAddr as well.

@rsc
Copy link
Contributor

rsc commented Nov 2, 2022

net.Addr exists to be returned from methods in net.Conn, specifically LocalAddr and RemoteAddr.
If there are no implementations of Conn that return a type T from one of those methods, then T should not implement net.Addr.

For the specific case of AddrPort and Prefix, there is no Network() string to return. TCPAddr returns "tcp", for example.

We could make netip.TCPAddrPort, but net.TCPConn would still not return that type.

@rsc
Copy link
Contributor

rsc commented Nov 2, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Nov 9, 2022
@golang golang locked and limited conversation to collaborators Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
4 participants
@rsc @ianlancetaylor @zhlsunshine @gopherbot