search

google / jsonnet

Jsonnet - The data templating language
http://jsonnet.org
Apache License 2.0
6.92k stars 437 forks source link

enumeration value 'binary' not handled in switch #1031

Closed jcpunk closed 6 months ago

jcpunk commented 1 year ago

Just making sure someone saw this warning

/builddir/build/BUILD/jsonnet-0.19.1/core/vm.cpp:1665:16: warning: enumeration value 'binary' not handled in switch [-Wswitch]
 1665 |         switch (v.type()) {
      |                ^
sparkprime commented 1 year ago

I think the deal here is if you have a more recent version of that library then there is an additional enum value. There is a PR that fixed it, but I rolled it back because the version in the third_party was older and didn't have it.

sparkprime commented 1 year ago

"that library" being json.hpp

johnbartholomew commented 6 months ago

Since the build is configured to turn on ~all the warnings, IMHO it would be nice to do something about this. Some options:

  1. Disable the warning ¯\_(ツ)_/¯
  2. Upgrade the bundled json.hpp and add a case for binary. This creates the reverse problem that people using a system-provided json library must have a version recent enough to have this enum value. It looks like this was added to nlohmann json in 2019: https://github.com/nlohmann/json/pull/1662 and released in v3.8.0, as of June 2020. For reference, Debian Buster (current supported LTS version) appears to have json v3.5.0: https://packages.debian.org/buster/nlohmann-json3-dev (the next version, Bullseye, has v3.9.1).
  3. #if logic in the switch statement to define a case for value_t::binary only if we're using a json lib version >= 3.8. I would prefer not to do this.
  4. Add a default case and raise an error. This should be both forward and backward compatible.