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: stdlib: provide a standard implementation of the Optional / Maybe generic type #48702

Closed
glenjamin opened this issue Sep 30, 2021 · 26 comments

Comments

@glenjamin
Copy link

I was able to find similar discussions on stdlib generic Slice and Map packages, but I was unable to find anything about Optional. If this was just a case of poor searching (or poor indexing), then my apologies - please close this.


At the moment, there are two ways to handle optionality - either through the type's zero value (eg Time), or by using a pointer type.

However, using a pointer type creates a degree of ambiguity. Sometimes a pointer is chosen for mutability reasons, and nil is never intended to be present. Likewise sometimes a pointer is chosen only for performance reasons.

It has been noted in many places that with the new Go generics system, creating an Optional[T any] type to represent a value that may or may not be filled is rather trivial.

However, if the creation and use of such types is left up to the ecosystem, it is likely that at least a handful of incompatible implementations may surface - whereas if the stdlib held the main implementation then everyone else could build on top of it as needed.

@gopherbot gopherbot added this to the Proposal milestone Sep 30, 2021
@ianlancetaylor ianlancetaylor changed the title proposal: Provide a stdlib implementation of the Optional / Maybe generic type proposal: provide a stdlib implementation of the Optional / Maybe generic type Sep 30, 2021
@ianlancetaylor
Copy link
Contributor

Can you propose a package and a specific API? Thanks.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Sep 30, 2021
@glenjamin
Copy link
Author

glenjamin commented Sep 30, 2021

I don't personally have a strong preference for a specific API, but I'm happy to propose a strawman. I have not yet given this serious thought - I'd mostly expected to find an already active thread on the topic, but this should start the ball rolling.

Package is an interesting one, it feels too small to need its own package, but I don't see an obvious alternative.

package optional

type Value[T any] struct {
  value T
  ok bool
}

func Ok[T any](value T) Value[T] {
  return Value{value: value, ok: true}
}
func (v Value[T]) Empty() bool {
  return !v.ok
}
func (v Value[T]) Get() (T, bool) {
  return v.value, v.ok
}
func (v Value[T]) Default(fallback T) T {
  if v.Empty() {
    return fallback
  }
  return v.value
}

Edit: It would probably also be useful to implement some common stdlib interfaces like fmt, json and sql

@DeedleFake
Copy link

DeedleFake commented Sep 30, 2021

Whatever API is decided on, encoding/json needs to have good support for the new type. json.Marshaler implementation isn't good enough for this because it can't be used to programmatically completely omit a field, so encoding/json itself would have to have support for it without API changes. The current solution for this is to use pointers and nil for optionality, but that doesn't allow for flexibility in terms of JSON's null and complete omission when unmarshalling, and for marshalling omitempty has to be specified at compile-time. This could allow fairly easy differentiation, though, via an optional pointer, such as optional.Value[*string].

it feels too small to need its own package

I don't think that that's a problem. errors was, and honestly still is, a very small package for quite a long time, with only a single exported function and one unexported type with a single method. It's gotten a bit bigger over the years, but not by much.

In terms of actual API, I'd like a Must() or Unwrap() method that panics if ok is false or just returns value. I'm also not partial to either, per se, but Rust's equivalent of the proposed Default() method, with Rust calls or(), takes and returns another optional type, not the underlying type directly. In other words, func (v Value[T]) Or(other Value[T]) Value[T] instead. Returning an optional wrapper instead of the underlying type also allows it to couple with various ways of extracting it, such as the aforementioned Must(): v.Or(optional.Ok(0)).Must().

On the other end of things, a top-level function, maybe Of(), to wrapping existing two-return functions into an optional.Value could be useful:

func Of[T any](v T, ok bool) Value[T] {
  return Value[T]{v: v, ok: ok}
}

// In another package:

someEnvVariable := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).Default("default value").Must()

@deanveloper
Copy link

I'm honestly unsure how useful this is compared to the typical , ok pattern. One of the things that I like about Go is that complexity typically moves vertically, rather than horizontally. For instance, I personally find

envVar, ok := os.LookupEnv("SOME_ENV_VARIABLE")
if !ok {
    envVar = "default value"
}

much more readable than

envVar := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).Default("default value").Must()

I guess what I am getting at is, what are the benefits of Optional[T] over (T, bool)? Are these benefits worth introducing 2 patterns that (possibly) do the same thing?

@glenjamin
Copy link
Author

My rough thinking was that it's better to stick to a minimal API that allows those other functions to be written in packages, than to include them all initially.

As long as the underlying type is consistent in the ecosystem and exposes enough information to write the other functions, I don't think there's a big downside to having those be 3rd party functions.

@ernado
Copy link
Contributor

ernado commented Oct 1, 2021

Somehow related: #11939 (proposal: encoding/json, encoding/xml: support zero values of structs with omitempty)

@ernado
Copy link
Contributor

ernado commented Oct 1, 2021

@deanveloper

I found optionals extremely useful for encoding/decoding:

// optional package
type Value[T any] struct {
	Value T
	Set   bool
}

func (v Value[T]) Get() (T, bool) {
	return v.Value, v.Set
}

// code usage:

type Error struct {
	Type        string
	Description string
}

type Response struct {
	Name  optional.Value[string] `json:"name,omitempty"`
	Error optional.Value[Error]  `json:"error,omitempty"`
}

func parse(data []byte) {
	var r Response
	json.Unmashal(data, &r)
	
	// Using fields:
	r.Name.Value // access to value, will be zero value of not set
	if r.Name.Set {
		// Explicit check for name to be set.
		fmt.Println("Name:", r.Name.Value)
	}
	
	// Using methods:
	if v, ok := r.Error.Get(); ok {
		fmt.Println("Got error:", v.Type)	
	}
}

This will eliminate a whole class of boilerplate, like aws.String() and friends.
Every complex REST (and even gRPC, all of them) API binding in go uses pointers for optionals, because concept of zero value is not so universal in practice.

aws/aws-sdk-go#363
kubernetes/enhancements#1173
graph-gophers/graphql-go#308

I've personally implemented code generation for optionals and will be very happy not to do so again, it is too ad-hoc and looks like footgun.

@glenjamin
Copy link
Author

I'm honestly unsure how useful this is compared to the typical , ok pattern.

@deanveloper is this in response to the proposal as a whole, or the Of function proposed in #48702 (comment) ?

I agree that for map-like APIs the existing pattern works fine.

@DeedleFake
Copy link

DeedleFake commented Oct 1, 2021

@deanveloper

One of the things that I like about Go is that complexity typically moves vertically, rather than horizontally.

I agree, generally, and, for something like this, most languages would wrap the calls from line to line, but Go's semicolon insertion rules make that look kind of strange:

envVar := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).
	Default("default value").
	Must()

Compare that with something like Rust:

let envvar = env::var_os("SOME_ENV_VARIABLE")
	.or(Some("default value"))
	.unwrap();

Unfortunately, I get the feeling that this awkwardness will come up a lot more with generics than it did before, as generics enable an entire class of method chaining patterns that were infeasible previously.

Edit: Also, like @glenjamin said, I think that for normal functions like LookupEnv, there's little reason to use this in this way. I was just using it as an easy example. Sometimes conversion might be useful, though, like when setting up a struct with an optional.Value in it for, for example, encoding:

data := PredefinedEncodableStruct{
	AnOptionalString: optional.Of(SomethingThatUsesTheOKBooleanPattern("arg")).Default("default"),
}
err := enc.Encode(data)
// ...

Edit 2: Something else that should be noted: optional.Of() will only work for functions that return exactly two values. That is by far the most common case, but it's definitely not the only one.

@AndrewHarrisSPU
Copy link

AndrewHarrisSPU commented Oct 1, 2021

An observation on Option[T] vis-a-vis record-oriented encoding/decoding I wonder about: the common, proven-to-be-useful schemes all tend to leak a bit about underlying state machines. Option[T] is not as precise as something like Omittable[T] or Nullable[T] would be for JSON. There are similar problems for any scheme - e.g. NA[T], NaN[T], Inf[T] for "missing" values in R. I think a similar observation is made where Rust's Serde warns:

Users tend to have different expectations around the Option enum compared to other enums.

I agree that there should be some opportunities to realize better APIs with generics around encoding. Record-to-record is one sense of an API direction ... maybe a Swiss Army Record type? I feel like the sense of state-machine-to-state-machine is just fundamentally hard to capture generically ... I can see Option[T] making sense for an elaborate solution that is generic-over-state-machines rather than generic-over-records here but not escaping library code.

@zephyrtronium
Copy link
Contributor

The only "real" examples so far of where Option[T] would be helpful are for encoding and decoding. Does it make sense to put it in package encoding? I think that would help to illustrate that the intent is not to replace the , ok idiom.

@DeedleFake
Copy link

DeedleFake commented Oct 1, 2021

@AndrewHarrisSPU

An observation on Option[T] vis-a-vis record-oriented encoding/decoding I wonder about: the common, proven-to-be-useful schemes all tend to leak a bit about underlying state machines. Option[T] is not as precise as something like Omittable[T] or Nullable[T] would be for JSON.

This is the problem with the current pointer-based approach as well. Pointer nullability is a side effect of the way pointers work, not the primary purpose, and yet a lot of times pointers are used simply to signify optionality. The is a particular issue because dereferencing can lead to crash bugs, meaning that extra handling has to be done for dealing with incoming data in particular and there's no compile-time safety around it. Optional values provide that safety, as you're really dealing with two completely separate types with no real equiavalance of the automatic dereference of the . operator. A method like Must() or Unwrap() could still cause a crash, but it's a lot more explicit. I think it's actually in a pretty similar vein to the way that error handling is explicit in Go, at least for functions with more than one return. If you want that other return value, you have to explicitly ignore the error with _.

@zephyrtronium

The only "real" examples so far of where Option[T] would be helpful are for encoding and decoding. Does it make sense to put it in package encoding? I think that would help to illustrate that the intent is not to replace the , ok idiom.

I can definitely see the argument for this, but I'm a bit worried that if it winds up being useful elsewhere than it'll be quite awkward for it to be in there, as it can't really be cleanly moved later.

@deanveloper
Copy link

deanveloper commented Oct 1, 2021

I can definitely see the argument for this, but I'm a bit worried that if it winds up being useful elsewhere than it'll be quite awkward for it to be in there, as it can't really be cleanly moved later.

If it ends up being useful elsewhere, then it should be copied (rather than shared) since it represents a different concept. In my eyes, Option in the context of encoding should only mean "something which may be omitted when (un)marshaling". Optional arguments mean something significantly different than this, and they should not be a shared type.

In terms of optional arguments to functions, I'm a bit torn. Ideally, all zero values should be useful, and functions should see zero values as the "empty" case for an optional argument. However, especially when interacting with other systems (ie databases or other languages), we need a value that is "more zero than zero" (ie NULL for SQL). I'm not sure if a concept like Option[T] is the right tool to handle these cases, and I almost think that each package defining their own idea of "Option" is a better idea, just as it is for separating the idea of encoding.Option[T] from an optional argument. For instance, database/sql could define a sql.Nullable[T] type, which is a significantly different idea than something like aws.Option[T] (to be used in aws structs).

TLDR - I think it might be a better idea to allow each package to define their own Option type, since in each package they typically represent different ideas.

@ernado
Copy link
Contributor

ernado commented Oct 6, 2021

After playing with go1.18 generics I agree that single Optional is not enough for encoding. because json field can be undefined, nil or with value and all cases with different semantics (e.g. nil can be "delete field" in update request).

Also, current generics design has bad ergonomics with pointer receiver, and currently the only way to implement it is with newtype (for Settable[T]) and [T, *T]:

var v Optional[String, *String]

@rsc
Copy link
Contributor

rsc commented Oct 6, 2021

It seems like we need more experience with generics, which may turn out to show us that this is not something we can do for all packages in one place. But either way, we need more experience, so we should probably decline this.

@deanveloper
Copy link

Also, current generics design has bad ergonomics with pointer receiver, and currently the only way to implement it is with newtype (for Settable[T]) and [T, *T]

I don't think this is a big problem with Option, right? Option is just a container and doesn't do anything with its contents, unlike the FromStrings example which calls a method.

@rsc
Copy link
Contributor

rsc commented Oct 6, 2021

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

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 6, 2021
@ernado
Copy link
Contributor

ernado commented Oct 6, 2021

Agree with @rsc, we need more time to decide how to better implement this.

@rsc rsc changed the title proposal: provide a stdlib implementation of the Optional / Maybe generic type proposal: stdlib: provide a standard implementation of the Optional / Maybe generic type Oct 6, 2021
@rsc
Copy link
Contributor

rsc commented Oct 6, 2021

And not just how but whether.

@deefdragon
Copy link

But either way, we need more experience, so we should probably decline this.

While I agree with needing experience to determine how/if, I thought that would mean this is put on hold, not declined. Is putting on hold only for shorter term? The process specifies hold when "requires [...] additional information that will not be available for a couple weeks or more"

The absolute earliest this would be reconsidered I assume would be post 1.19 release, and more likely post 1.20 release, nearly a year and a half from now, which is a rather long "or more".

@rsc
Copy link
Contributor

rsc commented Oct 13, 2021

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

@rsc rsc moved this from Active to Likely Decline in Proposals (old) Oct 13, 2021
@tandr
Copy link

tandr commented Oct 14, 2021

(A bit OT)
@rsc wrote

It seems like we need more experience with generics, which may turn out to show us that this is not something we can do for all packages in one place. But either way, we need more experience, so we should probably decline this.

Shouldn't it be "postponed until later" instead of outright "decline"? Or "decline" means "decline now" and proposals could be "revived" later, once "stars align" properly - new experience gathered, need is common, etc?

@glenjamin
Copy link
Author

One thing I will take away from this discussion is that if we do see a variety of third-party Optional[T] implementations in the wild, as long as they have equivalents of these methods, it will be fairly easy to convert between them

func (v Value[T]) Get() (T, bool) {
  return v.value, v.ok
}

func Of[T any](v T, ok bool) Value[T] {
  return Value[T]{v: v, ok: ok}
}

@mvdan
Copy link
Member

mvdan commented Oct 14, 2021

@tandr I understand the rejection to mean "not now" indeed.

I definitely agree that some form of generic optional type would benefit encoding libraries, if only to provide an alternative to "just use a pointer". But I also agree that offering a standard optional type could encourage everyone to start overusing it enthusiastically, so I can't disagree with a "for now" rejection :)

I also agree with @glenjamin that libraries could support arbitrary optional-like types via generic interfaces, as long as there's agreement on the shape or mechanism of those.

@pete-woods
Copy link

pete-woods commented Oct 18, 2021

I would also add this this issue is more cross-sectional than just JSON - the same applies to database support.

At present you're just about okay if you're using one of the supported types, e.g. https://pkg.go.dev/database/sql#NullString. But then you have sql-specific imports leaking out.

But if you're storing any kind of custom type, you need to either use a pointer, or change your type's definition and marshalling to support nullability.

@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Oct 20, 2021
@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests