Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow CreateString invocation with std::string_view #8069

Merged
merged 1 commit into from
Nov 18, 2023

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
49 checks passed
candysonya pushed a commit to candysonya/flatbuffers that referenced this pull request Jan 8, 2024
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