-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: context: add generic Key type #49189
Comments
This seems to have the potential to cause confusion if multiple packages use the type |
I thought of that, too, but the type-safety that this affords could still be gained by adding two types to it: type Key[K comparable, V any] struct {
key K
}
// This could be weird because of type inference not handling `K` due to `V` having to be manual.
func NewKey[K comparable, V any](key K) Key[K, V] {
return Key[K, V]{key: key}
}
func (k Key[K, V]) WithValue(ctx context.Context, val V) context.Context {
return WithValue(ctx, k.key, val)
}
func (k Key[K, V]) Value(ctx context.Context) (V, bool) {
v, ok := ctx.Value(k.key).(V)
return v, ok
} Since the standard practice is to just expose access via functions that do this manually for each type anyways, though, I'm not sure that this really gains that much. |
@dsnet A minor nit: With an implementation like Should we instead implement it like |
Possibly. My hope is that users gets accustomed to the idea that each key returned by
Great idea. |
What about dropping keys entirely and using the type as the key? Something like:
|
I have mixed feelings. I like the simplicity, but it increases the chance of two packages both operating on a common type (i.e., a |
It's not clear to me why NewKey takes a string. Why not something like?
|
The provided |
Okay, I see. I guess |
Keeping NewKey also adds the benefit of an additional constraint to be passed in to the value-retrieval function. this helps with type-safety, especially when there may be more than one value for a given type:
// vs
|
Something nice to mention is that this addresses 2 of the 4 things mentioned in #28342 (namely type-safety and documentation) |
The more I look at @dsnet's proposed implementation, the more I like it. My only suggestion is to change WithValue to panic if name is nil, to prevent accidental misuse of the zero value. |
On hold for understanding generics better. |
Placed on hold. |
Well, in the assumption of possibility to break some backward compatibility in Go2 it does not seem bad to just forbid non-named types in |
There should be one more aspect considered. Right now a common pattern is to have the context key private within the package, and then provide a public function which may do more than just check the given context and return For example this function which will return a new (or a default) logger if one isn't set on the context. The idea is, get value from context functions sometimes have to be a bit smarter. type ctxKeyLogger struct{}
func CtxLogger(ctx context.Context) Logger {
logger, ok := ctx.Value(ctxKeyLogger{})
if !ok {
logger = NewLogger()
}
return logger
} What I was thinking of is having something along the lines of: type Key[T comparable] struct {
valueFunc keyValueFunc[T]
}
type keyValueFunc[T comparable] func(ctx context.Context, k Key[T]) (T, bool)
func defaultValueFunc[T comparable](ctx context.Context, k Key[T]) (T, bool) {
value, ok := ctx.Value(k).(T)
return value, ok
}
func NewKey[T comparable](f keyValueFunc[T]) Key[T] {
if f == nil {
f = defaultValueFunc[T]
}
return Key[T]{valueFunc: f}
}
func (k Key[T]) Value(ctx context.Context) (T, bool) {
return k.valueFunc(ctx, k)
}
func (k Key[T]) WithValue(ctx context.Context, value T) context.Context {
return context.WithValue(ctx, k, value)
} I've played around with an interface version of type Key[T comparable] interface {
Value(ctx context.Context) (T, bool)
WithValue(ctx context.Context, value T) context.Context
} Honestly, from both my ideas, I am not 100% sold on them myself, but maybe it gives someone a spark of imagination! |
@dsnet Could we create a package for this? I think @ianlancetaylor has a point in #57025 (comment):
I've been copy and pasting your sample code for many times now. Creating a package so that we can re-use and see how many people does the same seems useful to me. |
There's https://pkg.go.dev/tailscale.com/util/ctxkey, but we provide no guarantees about API compatibility. |
The
Context
API suffers from the lack of type safety since keys and values are bothinterface{}
.With generics coming in Go 1.18, I propose the addition of:
For type safety, the value type is associated with the key type, and type safe variants of
Context.Value
andWithValue
are hung off ofKey
as methods.Example usage:
\cc @josharian
The text was updated successfully, but these errors were encountered: