Add option to use std::string_view

[techleeksnap]

No description provided.

[paulocoutinhox]

A question: There is any substancial advantages?

[techleeksnap]

A question: There is any substancial advantages?

There are various reasons you want to use std::string_view for function parameters and especially constexpr constant literals. See https://abseil.io/tips/1

The problem is it's awkward to migrate your existing project to use std::string_view when the libraries you depend on are still using const std::string& for their parameters because it means you have to explicity create a std::string to call those methods. Having this option unblock such migrations.

[paulocoutinhox]

Nice. In your tests, the string_view never appear:
https://github.com/Snapchat/djinni/pull/201/files#diff-f59b59e48a4585c01e5f0ff60c7bb52e28276ac2a3b91f85d8df4c48e01ab471R16

[li-feng-sc]

The main problem is that string and string_view are not really interchangeable. string implies memory ownership while string_view does not.

Imagine you have a Djinni record which contains a Djinni string. When you pass this record from Java to C++, C++ will receive a record that is only valid within the function scope. If you keep that record beyond the scope of the function, you just created a dangled string_view in the C++ struct.

This is too big a risk for average C++ programmers and could be a surprise for experienced C++ programmers.

[li-feng-sc]

Ok I understand now. This is only changing parameter passing from const string& to string_view, but member types are unchanged.

So what this means is string objects (either in user code or in djinni records) will be passed as string_view instead of const string&. This does not break ownership as const string& does not have ownership either.

[li-feng-sc]

downsides i can think of from the top of my head:

• breaks existing code (all the implementation code previously overriding the const string& version now won't compile)
• calling functions that takes const string& using string_view would make a copy. so to avoid this more downstream code needs to be fixed
• lose some consistency. previously the general rule of non-primitive parameter type in c++ is const T&

[techleeksnap]

• breaks existing code (all the implementation code previously overriding the const string& version now won't compile)

That's the case with most flag options, right?

• calling functions that takes const string& using string_view would make a copy. so to avoid this more downstream code needs to be fixed

Yes, but I think that's fine as long as the new options is not the default.

On the other hand, const std::string& will make copies inconspicuously. One of the main problems std::string_view tries to solve is

constexpr auto kConstant = "some foo literals";

void someMethod(const std::string& param);

someMethod(kConstant);  // This creates a copy!

Most C++ devs didn't realize that the above code will allocate a new string. That's why intentionally, std::string doesn't construct implicitly from std::string_view. One must write std::string(some_string_view), for instance.

• lose some consistency. previously the general rule of non-primitive parameter type in c++ is const T&

That's fair. But I think that's just part of the evolution of C++. Use of std::span has the same issue.

[li-feng-sc]

The only case this change affects is when a string is passed as a method parameter. If a string is passed as a part of record then there is no change (still passed as std::string member of the struct). This creates further inconsistency, a string is sometimes std::string, but sometimes std::string_view.

[techleeksnap]

The only case this change affects is when a string is passed as a method parameter. If a string is passed as a part of record then there is no change (still passed as std::string member of the struct). This creates further inconsistency, a string is sometimes std::string, but sometimes std::string_view.

@li-feng-sc

I'm not sure if that should be considered inconsistent. In that case, the parameter is a reference to a record, not a string.

Similarly, an int method parameter is passed by value. But an int field in a record is "passed" by reference when the method parameter is a record. In the latter case, the observed value of the int could change if the caller changed the record concurrently. This is just as "inconsistent".
In both cases, I think that's okay because it should be understood that the parameter type is a record, not the underlying individual fields.