getml / reflect-cpp

A C++20 library for fast serialization, deserialization and validation using reflection. Supports JSON, BSON, CBOR, flexbuffers, msgpack, TOML, XML, YAML / msgpack.org[C++20]
https://getml.github.io/reflect-cpp/
MIT License
900 stars 76 forks source link

Unnecessary field name string copy? #8

Closed travnick closed 10 months ago

travnick commented 10 months ago

Since string literals have static storage duration, is the copy here required? It should be enough o simply initialize a string view with pointer and size.

https://github.com/getml/reflect-cpp/blob/8f7c84954c7ce541ea75e1b820d74d29ac587f04/include/rfl/internal/StringLiteral.hpp#L17

liuzicheng1987 commented 10 months ago

I'm pretty sure it is. It's not only static, it is constexpr static.

It is necessary to support this syntax:

rfl::Field<std::string, "firstName">

So at compile time, the string "firstName" is copied into the class itself. I do not think that "firstName" would be persisted so I am pretty sure you couldn't just create a view.

Also, it is very important that this is all happening at compile time. So there really is no runtime overhead either way.

travnick commented 10 months ago

Looks like https://godbolt.org/ tricked me - string_view used to work once, but not any more.

Maybe in the future C++ versions it will work.

After translation phase 6, a string-literal that does not begin with an encoding-prefix is an ordinary string
literal. An ordinary string literal has type “array of n const char” where n is the size of the string as defined
below, has static storage duration (6.7.5), and is initialized with the given characters
liuzicheng1987 commented 10 months ago

Yes, I gave it a try myself and couldn't make it work either.