Skip to content

Conversation

@reshilkin
Copy link
Contributor

Allow to create strings using std::string_view

@github-actions github-actions bot added the c++ label Aug 11, 2023
@greenrobot
Copy link
Contributor

While I like using data() instead of c_str(), note that string view is supported here:

// clang-format off
  #ifdef FLATBUFFERS_HAS_STRING_VIEW
  /// @brief Store a string in the buffer, which can contain any binary data.
  /// @param[in] str A const string_view to copy in to the buffer.
  /// @return Returns the offset in the buffer where the string starts.
  template<template <typename> class OffsetT = Offset>
  OffsetT<String>CreateString(flatbuffers::string_view str) {
    return CreateString<OffsetT>(str.data(), str.size());
  }
  #endif // FLATBUFFERS_HAS_STRING_VIEW
  // clang-format on

Here, the parameter is passed by value (string view style), not const reference (string style) as the code.
I guess keeping both is fine. Alternatively, there could be a second templated function OffsetT<String> CreateString(T str) instead instead of the flatbuffers::string_view one.

@reshilkin
Copy link
Contributor Author

note that string view is supported here

But what if we have struct derived from std::string_view? OffsetT<String> CreateString(const T &str) is more prioritized in this case.

Alternatively, there could be a second templated function OffsetT<String> CreateString(T str)

Wouldn't this lead to ambig with OffsetT<String> CreateString(const T& str)?

@dbaileychess
Copy link
Collaborator

.data() should be fine, since .c_str() and that are the same post c++11, which is the oldest standard we support.

@dbaileychess dbaileychess merged commit 386b635 into google:master Nov 18, 2023
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants