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

additional check for absl::string_view availability #7897

Merged
merged 3 commits into from
Apr 28, 2023

Conversation

ocpalo
Copy link
Contributor

@ocpalo ocpalo commented Apr 7, 2023

absl::string_view is uses std::string_view when available. It already checks if std::string_view is available in the earlier code. It should only use absl::string_view implementation.

If some user, installs abseil system-wide with C++17 and ABSL_USES_STD_STRING_VIEW enabled, flatbuffers tries to use C++17 std::string_view if compiled with C++14. To avoid that, i propose to add the additional check for the abseil flag ABSL_USES_STD_STRING_VIEW.

Cons of this approach, abseil library might update the flag name ABSL_USES_STD_STRING_VIEW or change the location of the config file. So in this case, it will fail to find absl::string_view for C++14. It is unlikely but possible.

Pros of this pull request is, there are a lot of open source libraries relies on the flatbuffers. The same issue I described earlier exists on that projects too. For example OpenCV issue, where user installed abseil system-wide(and abseil uses std::string_view) and tries to compile OpenCV with C++14. Flatbuffers tries to include <string_view> because of that.

This issue can be solved by adding that additional check.

absl::string_view is uses std::string_view when available. It already checks if std::string_view is available in the earlier code.
It should only use absl::string_view implementation.
@github-actions github-actions bot added the c++ label Apr 7, 2023
@dbaileychess dbaileychess enabled auto-merge (squash) April 28, 2023 16:41
@dbaileychess dbaileychess merged commit c192ab4 into google:master Apr 28, 2023
ltoenning added a commit to ltoenning/BehaviorTree.CPP that referenced this pull request May 20, 2024
Backport/update from upstream flatbuffers repository.
Changes taken from google/flatbuffers#7897.
ltoenning added a commit to ltoenning/BehaviorTree.CPP that referenced this pull request May 20, 2024
Backport/update from upstream flatbuffers repository.
Changes taken from google/flatbuffers#7897.
ltoenning added a commit to ltoenning/BehaviorTree.CPP that referenced this pull request May 20, 2024
Original author of change: ocpalo
Backport/update from upstream flatbuffers repository.
Changes taken from google/flatbuffers#7897.
facontidavide pushed a commit to BehaviorTree/BehaviorTree.CPP that referenced this pull request May 30, 2024
* From flatbuffers upstream: Fix compiler error

Original author of change: avaliente-bc
Backport/update from upstream flatbuffers repository.
Change taken from google/flatbuffers#7227

* From flatbuffers upstream: Fix include of string_view with C++17 abseil

Original author of change: ocpalo
Backport/update from upstream flatbuffers repository.
Changes taken from google/flatbuffers#7897.
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
absl::string_view is uses std::string_view when available. It already checks if std::string_view is available in the earlier code.
It should only use absl::string_view implementation.

Co-authored-by: Derek Bailey <[email protected]>
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
absl::string_view is uses std::string_view when available. It already checks if std::string_view is available in the earlier code.
It should only use absl::string_view implementation.

Co-authored-by: Derek Bailey <[email protected]>
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.

2 participants