You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "felipecrv (via GitHub)" <gi...@apache.org> on 2023/10/11 01:28:33 UTC
[PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
felipecrv opened a new pull request, #38192:
URL: https://github.com/apache/arrow/pull/38192
### Rationale for this change
To use a more modern version of `flatc` in Arrow.
### What changes are included in this PR?
1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues
### Are these changes tested?
Tested by building everything correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1765394488
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 01b42d51ba97cb12c17e5b001c65b38741ea83ba.
There were 3 benchmark results indicating a performance regression:
- Commit Run on `ursa-i9-9960x` at [2023-10-13 11:50:49Z](https://conbench.ursa.dev/compare/runs/d8e1f73c854e4d2fa7dcc67f9b28dc53...e6c83373db21434f940879904d46fbfc/)
- [`file-read` (R) with compression=uncompressed, dataset=fanniemae_2016Q4, file_type=feather, language=R, output_type=dataframe](https://conbench.ursa.dev/compare/benchmarks/0652917762ec7699800036f81b83cebe...065293e3b48171e78000a02f8b601696)
- [`file-read` (R) with compression=lz4, dataset=fanniemae_2016Q4, file_type=feather, language=R, output_type=table](https://conbench.ursa.dev/compare/benchmarks/06529178274b7cff800094429e04ee88...065293e4bb9f7a3f8000d1b54280773b)
- and 1 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/17759641993) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1759982310
CI failures look unrelated, I'll merge. Thanks @felipecrv !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1759116667
@github-actions crossbow submit -g cpp -g python
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1759122286
Revision: b01925426559a9cf83183581969423f478eabe4c
Submitted crossbow builds: [ursacomputing/crossbow @ actions-2c6d2bba81](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2c6d2bba81)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871236/job/17632662296)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871340/job/17632662660)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871231/job/17632662276)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/runs/17632663908)|
|test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871452/job/17632663000)|
|test-conda-python-3.10-cython2|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871415/job/17632663019)|
|test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871339/job/17632662667)|
|test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871407/job/17632663005)|
|test-conda-python-3.10-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871442/job/17632663033)|
|test-conda-python-3.10-pandas-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871540/job/17632663456)|
|test-conda-python-3.10-spark-v3.5.0|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871583/job/17632663627)|
|test-conda-python-3.10-substrait|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871646/job/17632664177)|
|test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871630/job/17632663827)|
|test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871915/job/17632666330)|
|test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871610/job/17632664184)|
|test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871627/job/17632663845)|
|test-conda-python-3.11-pandas-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871681/job/17632664191)|
|test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871741/job/17632664395)|
|test-conda-python-3.8|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871677/job/17632664185)|
|test-conda-python-3.8-pandas-1.0|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871791/job/17632664926)|
|test-conda-python-3.8-spark-v3.5.0|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871770/job/17632664413)|
|test-conda-python-3.9|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871764/job/17632664397)|
|test-conda-python-3.9-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871768/job/17632664403)|
|test-cuda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871819/job/17632664948)|
|test-cuda-python|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871839/job/17632665490)|
|test-debian-11-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871978/job/17632666474)|
|test-debian-11-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871971/job/17632665744)|
|test-debian-11-python-3|[](https://github.com/ursacomputing/crossbow/runs/17632663906)|
|test-fedora-35-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872063/job/17632666405)|
|test-fedora-35-python-3|[](https://github.com/ursacomputing/crossbow/runs/17632663963)|
|test-ubuntu-20.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872058/job/17632665941)|
|test-ubuntu-20.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872060/job/17632665935)|
|test-ubuntu-20.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/6492871981/job/17632666348)|
|test-ubuntu-20.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872074/job/17632666478)|
|test-ubuntu-20.04-python-3|[](https://github.com/ursacomputing/crossbow/runs/17632663421)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872135/job/17632666521)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872171/job/17632666612)|
|test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872153/job/17632666574)|
|test-ubuntu-22.04-python-3|[](https://github.com/ursacomputing/crossbow/actions/runs/6492872223/job/17632666664)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1773393820
@felipecrv It seems that this broke `test-skyhook-integration`:
https://github.com/ursacomputing/crossbow/actions/runs/6502407264/job/17661381250#step:6:2369
```text
FAILED: src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o
/usr/local/bin/sccache /usr/bin/c++ -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_MIMALLOC -DARROW_NO_DEPRECATED_API -DARROW_WITH_RE2 -DARROW_WITH_TIMING_TESTS -DARROW_WITH_UTF8PROC -DBOOST_ALL_NO_LIB -Isrc -I/arrow/cpp/src -I/arrow/cpp/src/generated -isystem /arrow/cpp/thirdparty/flatbuffers/include -isystem /arrow/cpp/thirdparty/hadoop/include -isystem google_cloud_cpp_ep-install/include -isystem absl_ep-install/include -isystem crc32c_ep-install/include -isystem orc_ep-install/include -isystem protobuf_ep-install/include -isystem awssdk_ep-install/include -isystem opentelemetry_ep-install/include -isystem xsimd_ep/src/xsimd_ep-install/include -isystem jemalloc_ep-prefix/src -isystem mimalloc_ep/src/mimalloc_ep/include/mimalloc-2.0 -Wno-noexcept-type -fdiagnostics-color=always -Wall -Wno-conversion -Wno-sign-conversion -Wunused-result -Wdate-time -fno-semantic-interposition -msse4.2 -g
-Werror -O0 -ggdb -g1 -fPIC -pthread -std=c++17 -MD -MT src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o -MF src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o.d -o src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o -c /arrow/cpp/src/skyhook/protocol/skyhook_protocol.cc
In file included from /arrow/cpp/src/skyhook/protocol/skyhook_protocol.cc:21:
/arrow/cpp/src/skyhook/protocol/ScanRequest_generated.h: In member function 'bool org::apache::arrow::flatbuf::ScanRequest::Verify(arrow_vendored_private::flatbuffers::Verifier&) const':
/arrow/cpp/src/skyhook/protocol/ScanRequest_generated.h:45:55: error: no matching function for call to 'org::apache::arrow::flatbuf::ScanRequest::VerifyField<int64_t>(arrow_vendored_private::flatbuffers::Verifier&, org::apache::arrow::flatbuf::ScanRequest::FlatBuffersVTableOffset) const'
45 | VerifyField<int64_t>(verifier, VT_FILE_SIZE) &&
| ^
In file included from /arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffer_builder.h:42,
from /arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h:35,
from /arrow/cpp/src/skyhook/protocol/skyhook_protocol.cc:19:
/arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/table.h:132:8: note: candidate: 'bool arrow_vendored_private::flatbuffers::Table::VerifyField(const arrow_vendored_private::flatbuffers::Verifier&, arrow_vendored_private::flatbuffers::voffset_t, size_t) const [with T = long int; arrow_vendored_private::flatbuffers::voffset_t = short unsigned int; size_t = long unsigned int]'
132 | bool VerifyField(const Verifier &verifier, voffset_t field,
| ^~~~~~~~~~~
/arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/table.h:132:8: note: candidate expects 3 arguments, 2 provided
In file included from /arrow/cpp/src/skyhook/protocol/skyhook_protocol.cc:21:
/arrow/cpp/src/skyhook/protocol/ScanRequest_generated.h:46:57: error: no matching function for call to 'org::apache::arrow::flatbuf::ScanRequest::VerifyField<int16_t>(arrow_vendored_private::flatbuffers::Verifier&, org::apache::arrow::flatbuf::ScanRequest::FlatBuffersVTableOffset) const'
46 | VerifyField<int16_t>(verifier, VT_FILE_FORMAT) &&
| ^
In file included from /arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffer_builder.h:42,
from /arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h:35,
from /arrow/cpp/src/skyhook/protocol/skyhook_protocol.cc:19:
/arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/table.h:132:8: note: candidate: 'bool arrow_vendored_private::flatbuffers::Table::VerifyField(const arrow_vendored_private::flatbuffers::Verifier&, arrow_vendored_private::flatbuffers::voffset_t, size_t) const [with T = short int; arrow_vendored_private::flatbuffers::voffset_t = short unsigned int; size_t = long unsigned int]'
132 | bool VerifyField(const Verifier &verifier, voffset_t field,
| ^~~~~~~~~~~
/arrow/cpp/thirdparty/flatbuffers/include/flatbuffers/table.h:132:8: note: candidate expects 3 arguments, 2 provided
```
Could you re-generate `cpp/src/skyhook/protocol/ScanRequest_generated.h` too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1773384411
Revision: b01925426559a9cf83183581969423f478eabe4c
Submitted crossbow builds: [ursacomputing/crossbow @ actions-2eb8774cee](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2eb8774cee)
|Task|Status|
|----|------|
|test-skyhook-integration|[](https://github.com/ursacomputing/crossbow/actions/runs/6592619924/job/17913619466)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #38192:
URL: https://github.com/apache/arrow/pull/38192#discussion_r1355073260
##########
cpp/thirdparty/flatbuffers/README.md:
##########
@@ -75,48 +117,229 @@ index fccce42f6..a00d5b0fd 100644
#if (!defined(_MSC_VER) || _MSC_VER > 1600) && \
(!defined(__GNUC__) || (__GNUC__ * 100 + __GNUC_MINOR__ >= 407)) || \
-@@ -201,16 +211,20 @@ namespace flatbuffers {
- // Check for std::string_view (in c++17)
- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
- #include <string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- // Check for std::experimental::string_view (in c++14, compiler-dependent)
- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
- #include <experimental/string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::experimental::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- #endif
- #endif // __has_include
-@@ -278,6 +292,7 @@ template<typename T> FLATBUFFERS_CONSTEXPR inline bool IsConstTrue(T t) {
+@@ -216,37 +227,13 @@ namespace flatbuffers {
+ #endif
+
+ #ifndef FLATBUFFERS_HAS_STRING_VIEW
+- // Only provide flatbuffers::string_view if __has_include can be used
+- // to detect a header that provides an implementation
+- #if defined(__has_include)
+- // Check for std::string_view (in c++17)
+- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
+- #include <string_view>
+- namespace flatbuffers {
+- typedef std::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for std::experimental::string_view (in c++14, compiler-dependent)
+- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
+- #include <experimental/string_view>
+- namespace flatbuffers {
+- typedef std::experimental::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for absl::string_view
+- #elif __has_include("absl/strings/string_view.h") && \
+- __has_include("absl/base/config.h") && \
+- (__cplusplus >= 201411)
+- #include "absl/base/config.h"
+- #if !defined(ABSL_USES_STD_STRING_VIEW)
+- #include "absl/strings/string_view.h"
+- namespace flatbuffers {
+- typedef absl::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- #endif
+- #endif
+- #endif // __has_include
++ #include <string_view>
Review Comment:
Reference: https://github.com/apache/arrow/pull/12204
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #38192:
URL: https://github.com/apache/arrow/pull/38192#discussion_r1354853607
##########
cpp/thirdparty/flatbuffers/README.md:
##########
@@ -75,48 +117,229 @@ index fccce42f6..a00d5b0fd 100644
#if (!defined(_MSC_VER) || _MSC_VER > 1600) && \
(!defined(__GNUC__) || (__GNUC__ * 100 + __GNUC_MINOR__ >= 407)) || \
-@@ -201,16 +211,20 @@ namespace flatbuffers {
- // Check for std::string_view (in c++17)
- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
- #include <string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- // Check for std::experimental::string_view (in c++14, compiler-dependent)
- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
- #include <experimental/string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::experimental::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- #endif
- #endif // __has_include
-@@ -278,6 +292,7 @@ template<typename T> FLATBUFFERS_CONSTEXPR inline bool IsConstTrue(T t) {
+@@ -216,37 +227,13 @@ namespace flatbuffers {
+ #endif
+
+ #ifndef FLATBUFFERS_HAS_STRING_VIEW
+- // Only provide flatbuffers::string_view if __has_include can be used
+- // to detect a header that provides an implementation
+- #if defined(__has_include)
+- // Check for std::string_view (in c++17)
+- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
+- #include <string_view>
+- namespace flatbuffers {
+- typedef std::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for std::experimental::string_view (in c++14, compiler-dependent)
+- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
+- #include <experimental/string_view>
+- namespace flatbuffers {
+- typedef std::experimental::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for absl::string_view
+- #elif __has_include("absl/strings/string_view.h") && \
+- __has_include("absl/base/config.h") && \
+- (__cplusplus >= 201411)
+- #include "absl/base/config.h"
+- #if !defined(ABSL_USES_STD_STRING_VIEW)
+- #include "absl/strings/string_view.h"
+- namespace flatbuffers {
+- typedef absl::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- #endif
+- #endif
+- #endif // __has_include
++ #include <string_view>
Review Comment:
Is this change actually necessary or would the original "Check for std::string_view (in c++17)" snippet above work for us? We probably want to minimize the patch size.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #38192:
URL: https://github.com/apache/arrow/pull/38192#discussion_r1354851130
##########
cpp/thirdparty/flatbuffers/include/flatbuffers/allocator.h:
##########
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2021 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FLATBUFFERS_ALLOCATOR_H_
+#define FLATBUFFERS_ALLOCATOR_H_
+
+// Move this vendored copy of flatbuffers to a private namespace,
+// but continue to access it through the "flatbuffers" alias.
+namespace arrow_vendored_private {
Review Comment:
If we switch to C++17-style `arrow_vendored_private::flatbuffers` it probably reduces the patch size and may make automation more doable as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1773381274
@github-actions crossbow submit test-skyhook-integration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #38192:
URL: https://github.com/apache/arrow/pull/38192#issuecomment-1775396903
> Could you re-generate cpp/src/skyhook/protocol/ScanRequest_generated.h too?
Sure. Let me create an issue and submit a PR soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #38192:
URL: https://github.com/apache/arrow/pull/38192#discussion_r1355070993
##########
cpp/thirdparty/flatbuffers/README.md:
##########
@@ -75,48 +117,229 @@ index fccce42f6..a00d5b0fd 100644
#if (!defined(_MSC_VER) || _MSC_VER > 1600) && \
(!defined(__GNUC__) || (__GNUC__ * 100 + __GNUC_MINOR__ >= 407)) || \
-@@ -201,16 +211,20 @@ namespace flatbuffers {
- // Check for std::string_view (in c++17)
- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
- #include <string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- // Check for std::experimental::string_view (in c++14, compiler-dependent)
- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
- #include <experimental/string_view>
-+ namespace arrow_vendored_private {
- namespace flatbuffers {
- typedef std::experimental::string_view string_view;
- }
-+ }
- #define FLATBUFFERS_HAS_STRING_VIEW 1
- #endif
- #endif // __has_include
-@@ -278,6 +292,7 @@ template<typename T> FLATBUFFERS_CONSTEXPR inline bool IsConstTrue(T t) {
+@@ -216,37 +227,13 @@ namespace flatbuffers {
+ #endif
+
+ #ifndef FLATBUFFERS_HAS_STRING_VIEW
+- // Only provide flatbuffers::string_view if __has_include can be used
+- // to detect a header that provides an implementation
+- #if defined(__has_include)
+- // Check for std::string_view (in c++17)
+- #if __has_include(<string_view>) && (__cplusplus >= 201606 || (defined(_HAS_CXX17) && _HAS_CXX17))
+- #include <string_view>
+- namespace flatbuffers {
+- typedef std::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for std::experimental::string_view (in c++14, compiler-dependent)
+- #elif __has_include(<experimental/string_view>) && (__cplusplus >= 201411)
+- #include <experimental/string_view>
+- namespace flatbuffers {
+- typedef std::experimental::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- // Check for absl::string_view
+- #elif __has_include("absl/strings/string_view.h") && \
+- __has_include("absl/base/config.h") && \
+- (__cplusplus >= 201411)
+- #include "absl/base/config.h"
+- #if !defined(ABSL_USES_STD_STRING_VIEW)
+- #include "absl/strings/string_view.h"
+- namespace flatbuffers {
+- typedef absl::string_view string_view;
+- }
+- #define FLATBUFFERS_HAS_STRING_VIEW 1
+- #endif
+- #endif
+- #endif // __has_include
++ #include <string_view>
Review Comment:
You're right. Since this version doesn't check for `absl::string_view` first, we don't need it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35497: [C++] Use the latest tagged version of flatbuffers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #38192:
URL: https://github.com/apache/arrow/pull/38192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org