refactor(context): refactor `Keys` type to `map[any]any`
- ⚠️ Destructive adjustments require evaluation.
- Enable
Keysto support more types.
package main
import "github.com/gin-gonic/gin"
func main() {
var ctx *gin.Context
type key struct{}
ctx.Set(111, "111")
ctx.Get(111)
ctx.Get(key{})
ctx.Get("111")
// ……
}
The first step in solving https://github.com/gin-gonic/gin/issues/1123#issuecomment-876642837
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.05%. Comparing base (
3dc1cd6) to head (0336111). Report is 59 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #3963 +/- ##
==========================================
- Coverage 99.21% 99.05% -0.16%
==========================================
Files 42 44 +2
Lines 3182 2751 -431
==========================================
- Hits 3157 2725 -432
+ Misses 17 15 -2
- Partials 8 11 +3
| Flag | Coverage Δ | |
|---|---|---|
? |
||
| -tags "sonic avx" | 99.04% <100.00%> (?) |
|
| -tags go_json | 99.04% <100.00%> (?) |
|
| -tags nomsgpack | 99.03% <100.00%> (?) |
|
| go-1.18 | ? |
|
| go-1.19 | ? |
|
| go-1.20 | ? |
|
| go-1.21 | 99.05% <100.00%> (-0.16%) |
:arrow_down: |
| go-1.22 | 99.05% <100.00%> (?) |
|
| macos-latest | 99.05% <100.00%> (-0.16%) |
:arrow_down: |
| ubuntu-latest | 99.05% <100.00%> (-0.16%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Why not just make it comparable, or just any?
Why not just make it
comparable, or justany?
I think it's just like the key in context.WithValue(ctx, key, value).
It can be supported, so why not?
Moreover, when the key is set to struct{}, it saves more memory, ex:
type customKey struct{}
ctx.Set(customKey{}, "xxx")
package main
import (
"fmt"
"unsafe"
)
func main() {
var a int
var b string
var c struct{}
fmt.Println("a:", unsafe.Sizeof(a))
fmt.Println("b:", unsafe.Sizeof(b))
fmt.Println("c:", unsafe.Sizeof(c))
}
output:
a: 8
b: 16
c: 0
In Golang, maps can only have keys of types that are considered comparable. This means the type needs to support the == and != operators for comparison.
which means that structs have to have nothing but comparable types as their fields.
it introduces confusion when someone tries to use a key that is not comparable.
*comparable *is broken as far as I am concerned, not being able to provide a function that makes any struct comparable is a serious oversight and flaw with the language.
I have ended up having to write a library that generates a unique hash of any struct so that I can pass an array of structs I want to use as keys and array of values and generate a map with consistent hashing of the identity (fingerprint) of a struct as a key and the struct as the value. This allows me to generate keys from structs and look them up in the map without having to worry about if something I want to store in a map key is a valid key or not.
On Tue, May 14, 2024 at 11:24 AM Flc゛ @.***> wrote:
Why not just make it comparable, or just any?
I think it's just like the key in context.WithValue(ctx, key, value).
It can be supported, so why not?
Moreover, when the key is set to struct{}, it saves more memory, ex:
type customKey struct{} ctx.Set(customKey{}, "xxx")
— Reply to this email directly, view it on GitHub https://github.com/gin-gonic/gin/pull/3963#issuecomment-2110524488, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABF7754OZ67JDDN5LWVVBDZCIUERAVCNFSM6AAAAABHQT7FSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJQGUZDINBYHA . You are receiving this because you commented.Message ID: @.***>
-- Jarrod Roberson 678.551.2852
In Golang, maps can only have keys of types that are considered comparable.
This means the type needs to support the == and != operators for comparison.
which means that structs have to have nothing but comparable types as
their fields.
it introduces confusion when someone tries to use a key that is not
comparable.
*comparable *is broken as far as I am concerned, not being able to provide
a function that makes any struct comparable is a serious oversight and
flaw with the language.
I have ended up having to write a library that generates a unique hash of
any struct so that I can pass an array of structs I want to use as keys and
array of values and generate a map with consistent hashing of the identity
(fingerprint) of a struct as a key and the struct as the value. This allows
me to generate keys from structs and look them up in the map without having
to worry about if something I want to store in a map key is a valid key or
not.
On Tue, May 14, 2024 at 11:24 AM Flc゛ @.***> wrote:
Why not just make it comparable, or just any?
I think it's just like the key in context.WithValue(ctx, key, value).
It can be supported, so why not?
Moreover, when the key is set to struct{}, it saves more memory, ex:
type customKey struct{}
ctx.Set(customKey{}, "xxx")
—
Reply to this email directly, view it on GitHub
https://github.com/gin-gonic/gin/pull/3963#issuecomment-2110524488, or
unsubscribe
.
You are receiving this because you commented.Message ID:
@.***>
--
Jarrod Roberson
678.551.2852
Great reminder, I will add some single test trials.
@jarrodhroberson It is indeed recommended to change to map[comparable]any for better practice, but unfortunately, Go does not support this format. After checking the official documentation, no better solution has been found. Therefore, I have maintained the map[any]any format and added some unit test scripts.
https://github.com/gin-gonic/gin/pull/3963/files#diff-e6ce689a25eaef174c2dd51fe869fabbe04a6c6afbd416b23eda138c82e761baR220-R247
Some references: https://github.com/golang/go/issues/51384
@jarrodhroberson It is indeed recommended to change to
map[comparable]anyfor better practice, but unfortunately, Go does not support this format. After checking the official documentation, no better solution has been found. Therefore, I have maintained themap[any]anyformat and added some unit test scripts.https://github.com/gin-gonic/gin/pull/3963/files#diff-e6ce689a25eaef174c2dd51fe869fabbe04a6c6afbd416b23eda138c82e761baR220-R247
Some references: golang/go#51384
Yep went over source and there's no exposed func that could be turned into interface type with the given signature. Closest thing was cmp package but builtin types don't conform to its Equal.
@jarrodhroberson It is indeed recommended to change to
map[comparable]anyfor better practice, but unfortunately, Go does not support this format. After checking the official documentation, no better solution has been found. Therefore, I have maintained themap[any]anyformat and added some unit test scripts. https://github.com/gin-gonic/gin/pull/3963/files#diff-e6ce689a25eaef174c2dd51fe869fabbe04a6c6afbd416b23eda138c82e761baR220-R247 Some references: golang/go#51384Yep went over source and there's no exposed
functhat could be turned into interface type with the given signature. Closest thing wascmppackage but builtin types don't conform to itsEqual.
That is one of the inherit flaws of Go in reaction to all the insanely stupid things that other languages (JS cough cough) allow you to do. The inability to define an comparable function for your own types is one of them. Thanks for all the work on trying to so some kind of qualify life enhancements. If the Gin team would just make this an Interface it would make all this irrelevant.
break backwards. looks not good for me. if somebody write code like this, this pr make it build fail
func handleTest(c *gin.Context) {
var cpKeys = make(map[string]any, len(c.Keys))
for s, a := range c.Keys {
cpKeys[s] = a
}
// ....
}
if somebody write code like this, this pr make it build fail
But gin.Context is an implementation of context.Context.
And it is a broken implementation, because keys should be any, like in the original context.Context.
And so this change is more important, because it fixes this bug.
Yes, the official context source code: https://cs.opensource.google/go/go/+/refs/tags/go1.24.0:src/context/context.go;drc=f966695ccea356e4e4e8cc0328276e2d00c9fc1e;l=162