search

google / jsonnet

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

Does jsonnet require C++17 since 0.20.0? #1075

Closed toge closed 1 year ago

toge commented 1 year ago

jsonnet/0.20.0 uses nested namespace which is supported since C++17. https://github.com/google/jsonnet/commit/27e6e02d31e56c9b930d3be1d7dbc705bc0112af

Has supported C++ standard been changed?

sparkprime commented 1 year ago

Do we document somewhere which version of C++ we support?

toge commented 1 year ago

CMakeLists.txt defines that the sources are compiled in C++11. https://github.com/google/jsonnet/blob/master/CMakeLists.txt#L46

You are correct, there is no mention of using C++11 in the documentation. But as it is, 0.20.0 cannot be built with older compilers that do not support C++17 nested namespace.

I think this is one of the breaking changes.

sparkprime commented 1 year ago

Did you hit this because you use such a compiler?

I'll accept a PR to put the namespaces in with C++11 style

sparkprime commented 1 year ago

Also have you considered using the Go version? It's better

toge commented 1 year ago

@sparkprime Thank you for your comment. Since I'm using jsonnet as a library, I need to use the C++ version. While I am using jsonnet through Conan package manager, I need to support the older compilers that Conan's CI requires.

There are two ways to solve this.

  1. stop using nested namespace and keep C++11
  2. keep using nested namespace and change to C++17

After I read your comment, I think 2. is the way to go. As you know, most libraries are moving to the newer C++ standard.

Which proposal would you prefer? As I have no trouble with either, I will try to create a PR that is in line with your decision,

sparkprime commented 1 year ago

That's not actually true, there is a native wrapper for go-jsonnet based on the same libjsonnet.h so it's a drop-in replacement and has better performance. This already used for the Python binding https://pypi.org/project/gojsonnet/

I'm happy to switch to C++17 though.

toge commented 1 year ago

Thank you for this important information.

I had missed the opening sentence in README.md. I would like to use go-jsonnet from C/C++.

By the way, I created PR to use C++17. Please review it.

sparkprime commented 1 year ago

If you clone and build the go-jsonnet repo, in the c-bindings dir there should be a .a, .so, and .h file that should drop right in. Let me know how it works out for you