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

proposal: Go 2: permit implicit conversion of constants to pointer types #37302

Closed
abuchanan-nr opened this issue Feb 19, 2020 · 13 comments
Closed
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Milestone

Comments

@abuchanan-nr
Copy link

Some libraries define structs where all fields are pointers, including fields with primitive types (int, float, string, etc). For example:

type Record struct {
	Name   *string
	Age    *int
	Income *float64
	Active *bool
}

This often occurs in libraries defining RPC message types – presumably libraries use this to tell the difference between a field that is unset vs a field set to the zero value. Examples include protobuf, AWS libraries, thrift, and more.

These types cannot be used as naturally as people would like, especially when building test data. For example, consider the following test code:

func TestProcessRecord(t *testing.T) {
	rec := Record{
		Name:   stringPtr("Alex"),
		Age:    intPtr(34),
		Income: float64Ptr(42.0),
		Active: boolPtr(false),
	}
	out := ProcessRecord(rec)
	// ...etc...
}

func stringPtr(s string) *string    { return &s }
func float64Ptr(f float64) *float64 { return &f }
func intPtr(i int) *int             { return &i }
func boolPtr(b bool) *bool          { return &b }

Proposal

I propose Go enable automatic conversion of constants to pointer types. For example, I think this should work:

var y *int64 = 1

The example test above could then be simplified to:

func TestProcessRecord(t *testing.T) {
	rec := Record{
		Name:   "Alex",
		Age:    34,
		Income: 42.0,
		Active: false,
	}
	out := ProcessRecord(rec)
	// ...etc...
}

This seems to fall in line with the power of constants and the work the compiler does to convert them to types naturally. This would simplify existing test code and remove clutter.

As far as I can tell, this is a backward compatible change.

Related Proposals

This has been mentioned in other proposals: #9097 #19966

This proposal is similar to, but still significantly different from the related proposals. I think this proposal results in the most natural, minimal syntax possible, at least for the simple case of setting a variable or field to constant.

@gopherbot gopherbot added this to the Proposal milestone Feb 19, 2020
@abuchanan-nr
Copy link
Author

abuchanan-nr commented Feb 19, 2020

This might also apply to conditions? e.g. using the Record type above

rec := Record{}
if rec.Age == 10 {
}

@ianlancetaylor ianlancetaylor changed the title proposal: convert constants to pointer types proposal: Go 2: permit implicit conversion of constants to pointer types Feb 20, 2020
@ianlancetaylor ianlancetaylor added v2 An incompatible library change LanguageChange Suggested changes to the Go language labels Feb 20, 2020
@ianlancetaylor
Copy link
Member

In general Go avoids implicit conversions, as they can easily cause confusion. I believe the only implicit conversions in Go are from values of some defined type to an interface that that defined type implements. Also constants are untyped and as such can be used with a range of compatible types.

Also, if we are going to do this implicit conversion, why restrict it to constants? I agree that constants are a common case, but the same issues arise with values held in variables.

@ianlancetaylor
Copy link
Member

For language change proposals, please fill out the template at https://go.googlesource.com/proposal/+/refs/heads/master/go2-language-changes.md .

When you are done, please reply to the issue with @gopherbot please remove label WaitingForInfo.

Thanks!

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 20, 2020
@abuchanan-nr
Copy link
Author

Thanks Ian. I was looking for that "go2 language change" template, but couldn't find it. Maybe it's worth adding to https://github.com/golang/go/blob/master/CONTRIBUTING.md ?

Language change answers:

Would you consider yourself a novice, intermediate, or experienced Go programmer?

experienced

What other languages do you have experience with?

Javascript, Python, Java, Scala, R, PHP, Perl, Lua, and more.

Would this change make Go easier or harder to learn, and why?

Slightly easier, I believe. Setting a pointer-to-primitive field to a constant would "just work", without needing to learn the pattern of creating helper functions, or why you need to create a variable and reference it

Has this idea, or one like it, been proposed before?

Yes, as noted in the first comment.

If so, how does this proposal differ?

This proposal relies on constant type conversion instead of introducing a type of syntax.

Who does this proposal help, and why?

Mostly, this helps developers using data structures which define fields as pointers to primitive types, which are found in commonly used libraries such as Thrift and the AWS Go SDK (example)

What is the proposed change?

Add new rules to the conversion of untyped constants, so that an untyped constant may be converted to a pointer-to-primitive.

Is this change backward compatible?

Yes

Show example code before and after the change.

See first comment

What is the cost of this proposal? (Every language change has a cost).

- `gopls` and any IDE/inspection tools would need to be updated to recognize this conversion
- `vet` might need to be updated to recognize this conversion, I'm not sure
- What is the compile time cost? trivial cost, as far as I know
- What is the run time cost? There might be an allocation cost, when the constant is allocated in memory that may be referenced via a pointer. That same allocation would be necessary today, but I suppose it is hidden by the constant conversion proposed here, which is a negative.
- This makes the implementation of constant type resolution more complex, which may end up prohibiting future, unrelated enhancements of constant types in Go. That's a risk.
- Given this is a language + spec change, any other implementations of Go (I don't know of any explicitly, TinyGo maybe?) would need to support this.

Can you describe a possible implementation?

When converting a constant value x to a type *T (pointer to T), the compiler would follow the existing rules for determining if x is representable by T. If so, the compiler would generate code to allocate memory for a value of T, and then store the pointer to that value in x.

This is potentially hiding an allocation behind a constant conversion. Maybe that is a negative point against this proposal. I'm not sure if that is true, and if so, if it's acceptable.

How would the language spec change?

As far as I can tell, only the following part of the "Conversions" section would need to be modified:

Currently it says:

A constant value x can be converted to type T if x is representable by a value of T. As a special case, an integer constant x can be explicitly converted to a string type using the same rule as for non-constant x. 

Perhaps a note would be added to describe the conversion of x to *T (pointer to T).

https://golang.org/ref/spec#Conversions

Orthogonality: how does this change interact or overlap with existing features?

This proposal adds new rules to the existing rules for conversion of untyped constants.

I proposed this because I believe it fits in nicely with the existing automatic conversion of untyped constants; I always find that Go converts an untyped constant to what I wanted.

Is the goal of this change a performance improvement?

No

Does this affect error handling?

No

Is this about generics?

Nope

@abuchanan-nr
Copy link
Author

In general Go avoids implicit conversions, as they can easily cause confusion. I believe the only implicit conversions in Go are from values of some defined type to an interface that that defined type implements. Also constants are untyped and as such can be used with a range of compatible types.

By "implicit conversion", do you mean the fact that this could, for example, convert an untyped number to a pointer? I ask to clarify, because converting an untyped number 1 to a float64 might also be considered an implicit conversion.

I agree, prevention confusion is more important. In my mind, this is a natural extension to the existing rules for converting untyped constants, but others may find ways it's confusion.

A couple potential confusions I've since thought of:

  • I could see how someone could interpret var x *int64 = 5 as setting the pointer address to 5.
  • Would this prevent comparing a pointer address to a constant? e.g.
var ptr *Data
ptr = getDataPointer()
if ptr == 0x0 {
  log.Print("ptr address is zero")
}

Seems like this proposal could change the meaning of that if-statement.

Also, if we are going to do this implicit conversion, why restrict it to constants? I agree that constants are a common case, but the same issues arise with values held in variables.

I only run into this problem with constants, personally. Taking the address of existing variables is already simple and natural, so I'm not sure I understand those issues.

Restricting this to constants seemed to fit in well with the existing conversions of untyped constants, which also seemed simple and less risky than a broader change.

@abuchanan-nr
Copy link
Author

@gopherbot please remove label WaitingForInfo

@gopherbot gopherbot removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 20, 2020
@beoran
Copy link

beoran commented Feb 20, 2020

This problem can be solved in 5 minutes by making a small package and use that as follows:

/*
MIT license.
Copyright 2020 Beoran.

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

/*
 * This package allows to construct pointers easily from values.
 */
package p2

func Bool(v bool) *bool                   { return &v }
func Byte(v byte) *byte                   { return &v }
func Complex128(v complex128) *complex128 { return &v }
func Complex64(v complex64) *complex64    { return &v }
func Error(v error) *error                { return &v }
func Float32(v float32) *float32          { return &v }
func Float64(v float64) *float64          { return &v }
func Int(v int) *int                      { return &v }
func Int16(v int16) *int16                { return &v }
func Int32(v int32) *int32                { return &v }
func Int64(v int64) *int64                { return &v }
func Int8(v int8) *int8                   { return &v }
func Rune(v rune) *rune                   { return &v }
func String(v string) *string             { return &v }
func Uint(v uint) *uint                   { return &v }
func Uint16(v uint16) *uint16             { return &v }
func Uint32(v uint32) *uint32             { return &v }
func Uint64(v uint64) *uint64             { return &v }
func Uint8(v uint8) *uint8                { return &v }
func Uintptr(v uintptr) *uintptr          { return &v }

Then the code above becomes

func TestProcessRecord(t *testing.T) {
	rec := Record{
		Name:   p2.Stringr("Alex"),
		Age:    p2.Int(34),
		Income: p2.Float64(42.0),
		Active: p2.Bool(false),
	}
	out := ProcessRecord(rec)
	// ...etc...
}

Which is quite readable without any thorny language issues. If so desired the package name could be different but let's not bikeshed it here. Since this is a common use case, it could be considered to add such a package to the standard library.

@abuchanan-nr
Copy link
Author

Regarding my concern about this being confusing:

var ptr *Data
ptr = getDataPointer()
if ptr == 0x0 {
  log.Print("ptr address is zero")
}

I'm pretty sure that's not allowed by the compiler. I'm not familiar with direct pointer manipulation in Go, so I can't tell if this would cause issues there. Maybe that happens via uintptr and/or unsafe?
https://play.golang.org/p/lbagXJut32L

@abuchanan-nr
Copy link
Author

@beoran Functions such as the ones you listed are the status quo. They are often copied into the packages where they are used, as private helper functions, but are sometimes pulled in from an external package as well.

The purpose of this proposal is to remove the need for such helper functions, along with the clutter they create.

@ianlancetaylor
Copy link
Member

If we are going to permit an untyped constant to convert to a pointer type, then p == 0 (where p has some pointer type) will also be permitted. If that is not permitted, then we would need to introduce a new concept to the language, to describe the cases that permit implicit conversion to pointer types and the cases that do not. Unfortunately p == 0 will not behave as anybody expects; it will allocate a new memory location, set it to 0, and then compare the address of that location to p. In other words p == 0 will always be false.

The non-constant case comes up with code like

	rec := Record{
		Name:   CurrentUser()
		Income: 42.0 + baseIncome(),
	}

@beoran
Copy link

beoran commented Feb 21, 2020

It is true that these functions get copied around everywhere, they are even in the Go standard library! Mostly they are used for used in testing: e.g.: src/encoding/asn1/asn1_test.go, src/encoding/json/decode_test.go, etc, and the definition is often redundant, and even named differently.

While I know the FAQ about X in standard library, and I know that that the bar is high, this suggests to me that a package such as the p2, perhaps with a different name, can really belong in the Go standard library, since it will improve the code of the standard library itself, removing the redundancy and the differently named re-definitions everywhere. Not to mention the hundreds of other projects that use such functions.

@ianlancetaylor
Copy link
Member

I would be inclined to push on #9097 before adding such a package to the standard library.

@abuchanan-nr
Copy link
Author

Thanks Ian. The case where p == 0 is always false feels like a fatal flaw to me. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

4 participants