FAQ
Hi,

I propose creating a new internal package to provide often used low level
string conversion methods within the go std lib.

Within the go code base many different versions of common string conversion
functions such as itoa are implemented.
With the mechanism of internal packages these could be consolidated into
one package.

An internal package that can be used in other std lib packages would be
beneficial from my point of view in many ways - e.g. for the itoa function:

1. reduce duplicated code (itoa/itod is/was implemented in os, syscall (2x)
,net, ... )
2. can provide tested versions without duplication of tests in each package
and unify existing tests
3. avoid errors made while reimplementing these functions (e.g. infinite
recursion on edge cases for itoa/uitoa)
4. standardize naming of functions (e.g. uitoa vs itod)
5. no confusion over what signature itoa has in each package and what cases
it can handle or not handle correctly (e.g. when a specific itoa does not
handle negative integers)
6. apply bug fixes and optimizations (even if not performance critical) to
all use cases of the functions in the code base by patching them in one
place

- Martin Möhrmann

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Search Discussions

  • Brad Fitzpatrick at Jan 5, 2015 at 9:31 pm
    I would like to see this done as well.

    On Mon, Jan 5, 2015 at 12:00 PM, wrote:

    Hi,

    I propose creating a new internal package to provide often used low level
    string conversion methods within the go std lib.

    Within the go code base many different versions of common string
    conversion functions such as itoa are implemented.
    With the mechanism of internal packages these could be consolidated into
    one package.

    An internal package that can be used in other std lib packages would be
    beneficial from my point of view in many ways - e.g. for the itoa function:

    1. reduce duplicated code (itoa/itod is/was implemented in os, syscall
    (2x) ,net, ... )
    2. can provide tested versions without duplication of tests in each
    package and unify existing tests
    3. avoid errors made while reimplementing these functions (e.g. infinite
    recursion on edge cases for itoa/uitoa)
    4. standardize naming of functions (e.g. uitoa vs itod)
    5. no confusion over what signature itoa has in each package and what
    cases it can handle or not handle correctly (e.g. when a specific itoa does
    not handle negative integers)
    6. apply bug fixes and optimizations (even if not performance critical) to
    all use cases of the functions in the code base by patching them in one
    place

    - Martin Möhrmann

    --
    You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Dave Cheney at Jan 5, 2015 at 9:34 pm
    I appreciate the work you've done on the various itoa implementations,
    but now that work is done I'm of the opinion that adding a package
    dependency is an overeaction. The reason these functions are declared
    in their package is to avoid a dependency in a low level package, so
    adding one, even an internal one, runs counter to the original
    requirement.

    How many functions, in addition to {u,}itoa, are we talking about here ?

    Thanks

    Dave
    On Tue, Jan 6, 2015 at 7:00 AM, wrote:
    Hi,

    I propose creating a new internal package to provide often used low level
    string conversion methods within the go std lib.

    Within the go code base many different versions of common string conversion
    functions such as itoa are implemented.
    With the mechanism of internal packages these could be consolidated into one
    package.

    An internal package that can be used in other std lib packages would be
    beneficial from my point of view in many ways - e.g. for the itoa function:

    1. reduce duplicated code (itoa/itod is/was implemented in os, syscall (2x)
    ,net, ... )
    2. can provide tested versions without duplication of tests in each package
    and unify existing tests
    3. avoid errors made while reimplementing these functions (e.g. infinite
    recursion on edge cases for itoa/uitoa)
    4. standardize naming of functions (e.g. uitoa vs itod)
    5. no confusion over what signature itoa has in each package and what cases
    it can handle or not handle correctly (e.g. when a specific itoa does not
    handle negative integers)
    6. apply bug fixes and optimizations (even if not performance critical) to
    all use cases of the functions in the code base by patching them in one
    place

    - Martin Möhrmann

    --
    You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Ian Lance Taylor at Jan 5, 2015 at 9:39 pm

    On Mon, Jan 5, 2015 at 1:34 PM, Dave Cheney wrote:
    I appreciate the work you've done on the various itoa implementations,
    but now that work is done I'm of the opinion that adding a package
    dependency is an overeaction. The reason these functions are declared
    in their package is to avoid a dependency in a low level package, so
    adding one, even an internal one, runs counter to the original
    requirement.
    The issue, as I recall, is not quite to avoid any dependency in a
    low-level package, it's that the strconv package depends on a few
    other packages (errors, math, unicode/utf8), and it adds up. I think
    it's generally OK to add a dependency on a very small internal
    package.

    Ian

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Mikio Hara at Jan 5, 2015 at 10:14 pm

    On Tue, Jan 6, 2015 at 5:00 AM, wrote:

    6. apply bug fixes and optimizations (even if not performance critical) to
    all use cases of the functions in the code base by patching them in one
    place
    yup, applying the fix for https://github.com/golang/go/issues/8332 to
    syscall, os. net and log packages separately seems not nice.

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Dave Cheney at Jan 5, 2015 at 10:21 pm
    Sure. Again thank you Martin for fixing this, but now the bug is fixed and
    is unlikely to require additional attention.

    This proposal aims to reduce duplication of itoa implementations but I feel
    it will do the opposite, instead provide two strconv packages, one visible
    to user code, the other not visible, yet both are used apparently
    interchangeably the standard library.

    The bug is fixed, itoa will no longer bother us, can't we leave it at that?
    On 6 Jan 2015 09:14, "Mikio Hara" wrote:
    On Tue, Jan 6, 2015 at 5:00 AM, wrote:

    6. apply bug fixes and optimizations (even if not performance critical) to
    all use cases of the functions in the code base by patching them in one
    place
    yup, applying the fix for https://github.com/golang/go/issues/8332 to
    syscall, os. net and log packages separately seems not nice.

    --
    You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Jonathan Rudenberg at Jan 5, 2015 at 10:35 pm

    On Jan 5, 2015, at 5:21 PM, Dave Cheney wrote:

    Sure. Again thank you Martin for fixing this, but now the bug is fixed and is unlikely to require additional attention.

    This proposal aims to reduce duplication of itoa implementations but I feel it will do the opposite, instead provide two strconv packages, one visible to user code, the other not visible, yet both are used apparently interchangeably the standard library.
    Could the public strconv package use the internal implementation?

    Jonathan

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Dave Cheney at Jan 5, 2015 at 10:42 pm
    This is exactly what I would not want to see.

    We started with a few hand rolled itoa implementations, they had various
    bugs, they are fixed now.

    Next is the proposal to unify them all into a new package. But that act of
    unification creates its own duplication.

    In order to resolve this new duplication the public strconv now has to
    depend on the internal strconv. Now there is an additional cost for anyone
    calling the forwarded methods from the public to the internal package, loss
    of in lining potential,etc.

    Yes duplication is bad, but I think in this case it is the lesser evil.
    On 6 Jan 2015 09:35, "Jonathan Rudenberg" wrote:

    On Jan 5, 2015, at 5:21 PM, Dave Cheney wrote:

    Sure. Again thank you Martin for fixing this, but now the bug is fixed
    and is unlikely to require additional attention.
    This proposal aims to reduce duplication of itoa implementations but I
    feel it will do the opposite, instead provide two strconv packages, one
    visible to user code, the other not visible, yet both are used apparently
    interchangeably the standard library.

    Could the public strconv package use the internal implementation?

    Jonathan
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Minux at Jan 5, 2015 at 10:45 pm
    I did a little research on this.

    Here are the copies of {u,}itoa in std packages:
    log/log.go:func itoa(buf *[]byte, i int, wid int)
    net/parse.go:func itoa(i int) string
    net/parse.go:func itod(i uint) string
    runtime/os1_plan9.go:func itoa(buf []byte, val uint64) []byte
    syscall/str.go:func itoa(val int) string
    syscall/str.go:func uitoa(val uint) string
    strconv/itoa.go:func Itoa(i int) string

    The runtime one is slightly harder to use internal package, because
    we also need to change cmd/dist to build internal/strconv before
    runtime.

    strconv.Itoa is implemented as FormatInt(int64(i), 10), so unless we
    move FormatInt to internal/strconv, we can't use internal/strconv to
    implement strconv.Itoa.

    However, even if we can't remove the copy in strconv, using
    internal/strconv is still beneficial in removing duplicates.

    Removing duplicated implementation is one thing, removing and
    unifying the tests is perhaps more important. Right now, nearly
    each package has separate test for their own itoa implementation..

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Robert Griesemer at Jan 5, 2015 at 10:57 pm
    I'm in favor of factoring out this duplicate, as long as we keep it very
    targeted (basically a form of (u)itoa, exact name tbd) that does the most
    basic conversion with good tests and thus removes the chance for the same
    errors in every place.
    - gri
    On Mon, Jan 5, 2015 at 2:44 PM, minux wrote:

    I did a little research on this.

    Here are the copies of {u,}itoa in std packages:
    log/log.go:func itoa(buf *[]byte, i int, wid int)
    net/parse.go:func itoa(i int) string
    net/parse.go:func itod(i uint) string
    runtime/os1_plan9.go:func itoa(buf []byte, val uint64) []byte
    syscall/str.go:func itoa(val int) string
    syscall/str.go:func uitoa(val uint) string
    strconv/itoa.go:func Itoa(i int) string

    The runtime one is slightly harder to use internal package, because
    we also need to change cmd/dist to build internal/strconv before
    runtime.

    strconv.Itoa is implemented as FormatInt(int64(i), 10), so unless we
    move FormatInt to internal/strconv, we can't use internal/strconv to
    implement strconv.Itoa.

    However, even if we can't remove the copy in strconv, using
    internal/strconv is still beneficial in removing duplicates.

    Removing duplicated implementation is one thing, removing and
    unifying the tests is perhaps more important. Right now, nearly
    each package has separate test for their own itoa implementation..

    --
    You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Martisch at Jan 5, 2015 at 11:26 pm
    This topic arose due to two CL that are still pending where the idea came
    up to actually have one place with these functions rather than several:
    https://go-review.googlesource.com/#/c/2215/
    https://go-review.googlesource.com/#/c/2212/

    Also the 'Go 1.4 "Internal" Packages' proposal did mention itoa as a
    motivating example.
    https://docs.google.com/document/d/1e8kOo3r51b2BWtTs_1uADIA5djfXhPT36s6eHVRIvaU/edit


    Adding to the lists of implementations there is also:
    os/str.go with itoa and uitoa

    os actually had two implementations itoa and itod with different build tags
    that were merged in this CL:
    https://go-review.googlesource.com/#/c/2213/

    I would be in favor of keeping the package (if there will be one) minimal
    to some basic often needed functions in very low level form. Also i would
    not want to make the normal strconv package use it.
    It also does not need to be named strconv and could be named more generally
    to also encompass other utility functions that might be used often.

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Russ Cox at Jan 7, 2015 at 4:58 pm
    The duplication that exists is fine. The time spent discussing this
    costs more than the savings.

    Russ

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJan 5, '15 at 9:29p
activeJan 7, '15 at 4:58p
posts12
users9
websitegolang.org

People

Translate

site design / logo © 2021 Grokbase