[golang-dev] Proposal: new package internal/strconv

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.

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.

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.

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

On 6 Jan 2015 09:14, "Mikio Hara" wrote:
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.

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.

On 6 Jan 2015 09:35, "Jonathan Rudenberg" wrote:

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

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.