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

[cbor] Unable to use numbers as map keys #35

Open
dignifiedquire opened this issue Sep 24, 2018 · 5 comments
Open

[cbor] Unable to use numbers as map keys #35

dignifiedquire opened this issue Sep 24, 2018 · 5 comments

Comments

@dignifiedquire
Copy link
Contributor

I am trying to marshal a type map[uint64]*somestruct, but cannot do so, as I get the error messags unsupported map key type "uint64". This is really unfortunate, because one of the great things about cbor is that I can use numbers as keys in maps.

Code that throws the error: https://github.com/polydawn/refmt/blob/master/obj/marshalMapWildcard.go#L24-L55

@warpfork
Copy link
Member

You've already seen it, but I'll reproduce the comment in there here for posterity/context:

// note: stdlib json.marshal supports all the int types here as well, and will
// tostring them. but this is not supported symmetrically; so we simply... don't.

This is a feature I had some questions about implementing because if you use it, you're giving up on the ability to drop in a json encoding, and I think it's worrying to do that without making it clear. And in particular, I still don't approve of an implementation that silently coerces ints to strings in a lossy one-way trip.

I think I'm onboard with expanding this, though. The right way to do it is to have the obj/* code understand numeric map keys just fine (like they do, say, byte slices), and have the json/* code reject that, and the cbor/* code also accept it. These comments about json problems being in the obj package are... Yeah, old and stale and misplaced and should be fixed.

@warpfork
Copy link
Member

I think this should be pretty easy to fix to handle numerics, and I can try to get to it sometime next week.

@warpfork
Copy link
Member

warpfork commented Oct 6, 2018

Out of curiosity, are you using the entire range of uint64? That seems like some interesting code, if so.

I'm brought to wonder because I looked at implementing numeric keys yesterday, and balked because the deterministic ordering sort code as currently written and modified in the most obvious way would get pretty ugly -- especially since it would require both uint64, and int64, all of them -- to work correctly for inputs in the highest half of uint64 -- and probably ought to have int/uint paths for that common case to save casting time as well.

I'm pondering whether it's going to turn out better to implement a whole specialized marshalMachine for each numeric type. But that's not without its drawbacks either. Kind of hoping for a better implementation idea to strike me than the ones I've got so far.

@dignifiedquire
Copy link
Contributor Author

dignifiedquire commented Oct 6, 2018 via email

@whyrusleeping
Copy link
Collaborator

@warpfork I need this bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants