search

palantir / conjure

Strongly typed HTTP/JSON APIs for browsers and microservices
https://palantir.github.io/conjure/
Apache License 2.0
421 stars 66 forks source link

Discourage/deprecate set<double> and map<double, V> #178

Open sfackler opened 6 years ago

sfackler commented 6 years ago

Floating point keys are currently allowed as keys in map and set types. This poses particular problems for the Rust Conjure implementation, where floating point values can't be used as keys in set and map types. We could in theory fix this but only through pretty drastic measures like having our own custom collection types and associated traits. Instead, I'm just planning on not supporting set<double> and map<double, V> in generated code.

Beyond that specific language integration issue, floating point keys are a bit iffy generally IMO since it's so easy to drift just slightly off of the exact numeric value that you're expecting. It seems like it might be best to discourage the use of these types minimally.

ferozco commented 6 years ago

Adding a deprecation warning sounds reasonable to me. Long term it does seem desirable to disallow such usage. We've spoken about this exact issue before and from what I recall the only reason we didn't remove set<double> and map<double, V from the language was out of concern for breaking current usage.

bmoylan commented 5 years ago

+1 I am currently working on jumping through hoops to support these types in conjure-go (which does not natively support json-encoding of floating point map keys, and will straight-up fail to serialize NaN (which is not comparable) without a very custom implementation)

raiju commented 2 years ago

@carterkozak This might be an interesting one to add to the --strict flag?

sfackler commented 2 years ago

Follow up note here - conjure-rust does now support these types, but via a bit of a gross API and I think they're semantically a bit iffy in general due to precision issues: https://github.com/palantir/conjure-rust/pull/155