This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new a9a8626e83 ARROW-17280: [C++] Move vendored flatbuffers to private
namespace (#13775)
a9a8626e83 is described below
commit a9a8626e8342e1145ec1b60ffb50154693b9ef92
Author: Corey Kosak <[email protected]>
AuthorDate: Mon Aug 8 03:39:18 2022 -0400
ARROW-17280: [C++] Move vendored flatbuffers to private namespace (#13775)
When a user's C++ program links to both Arrow and an installation of the
Flatbuffers library, the program can crash
or send corrupt Arrow messages.
The reason for this is version incompatibility between the vendored (and
trimmed-down) version of Flatbuffers that lives inside Arrow, and whatever
version of Flatbuffers the user is using.
The community seems to be aware of this issue, at least as it impacts Java:
ARROW-5579
In C++, the problem is especially pernicious because it is not even
diagnosed at build time (e.g. by duplicate linker symbols). The methods being
used are templates and so their definitions are emitted as weak symbols by the
compiler. As we all know, when a weak symbol is defined in two different
compilation units, the linker assumes their definitions are identical and it
will just pick one. Here, the result is that either Arrow or the user program
gets different Flatbuffers code than [...]
Arrow doesn't even advertise the version of Flatbuffers that it vendored so
it's impossible for the user to even ameliorate this problem. In any case, it
would be a little unfriendly to force the user to use that exact version of
Flatbuffers even if it could be identified.
The good news is that there is an easy workaround. Arrow C++ doesn't export
Flatbuffers as part of its public interface. Instead, it just uses it
internally, as an implementation detail. Therefore it is easy to just move the
vendored Flatbuffers from the namespace "flatbuffers" to some other private
namespace. In my PR, I change the namespace to `arrow_thirdparty_flatbuffers`.
Then I create a namespace alias which makes `flatbuffers` an alias for
`arrow_thirdparty_flatbuffers`. The ne [...]
You might prefer a nested namespace instead, such as
`arrow::thirdparty::flatbuffers`, or some other choice.
Authored-by: Corey Kosak <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/thirdparty/flatbuffers/README.md | 137 ++++++++++++++++++++-
.../flatbuffers/include/flatbuffers/base.h | 16 +++
.../flatbuffers/include/flatbuffers/flatbuffers.h | 10 ++
.../include/flatbuffers/stl_emulation.h | 10 ++
4 files changed, 172 insertions(+), 1 deletion(-)
diff --git a/cpp/thirdparty/flatbuffers/README.md
b/cpp/thirdparty/flatbuffers/README.md
index e955adba4c..447ad818a1 100644
--- a/cpp/thirdparty/flatbuffers/README.md
+++ b/cpp/thirdparty/flatbuffers/README.md
@@ -18,7 +18,8 @@
-->
This directory contains a vendored version of Flatbuffers
-(unknown changeset), with the following patch for ARROW-15388:
+(unknown changeset), with two patches: the first patch
+for ARROW-15388 and the second patch for ARROW-17280.
```diff
diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
@@ -40,3 +41,137 @@ index 955738067..fccce42f6 100644
#endif // __has_include
#endif // !FLATBUFFERS_HAS_STRING_VIEW
```
+
+```diff
+diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
+index fccce42f6..a00d5b0fd 100644
+--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
++++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
+@@ -1,6 +1,14 @@
+ #ifndef FLATBUFFERS_BASE_H_
+ #define FLATBUFFERS_BASE_H_
+
++// Move this vendored copy of flatbuffers to a private namespace,
++// but continue to access it through the "flatbuffers" alias.
++namespace arrow_vendored_private {
++namespace flatbuffers {
++}
++}
++namespace flatbuffers = arrow_vendored_private::flatbuffers;
++
+ // clang-format off
+
+ // If activate should be declared and included first.
+@@ -144,10 +152,12 @@
+ #define FLATBUFFERS_VERSION_REVISION 0
+ #define FLATBUFFERS_STRING_EXPAND(X) #X
+ #define FLATBUFFERS_STRING(X) FLATBUFFERS_STRING_EXPAND(X)
++namespace arrow_vendored_private {
+ namespace flatbuffers {
+ // Returns version as string "MAJOR.MINOR.REVISION".
+ const char* FLATBUFFERS_VERSION();
+ }
++}
+
+ #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) {
+ /// @endcond
+
+ /// @file
++namespace arrow_vendored_private {
+ namespace flatbuffers {
+
+ /// @cond FLATBUFFERS_INTERNAL
+@@ -388,4 +403,5 @@ inline size_t PaddingBytes(size_t buf_size, size_t
scalar_size) {
+ }
+
+ } // namespace flatbuffers
++} // namespace arrow_vendored_private
+ #endif // FLATBUFFERS_BASE_H_
+diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
+index c4dc5bcd0..2f7eb5fcf 100644
+--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
++++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
+@@ -23,6 +23,15 @@
+ # include <cmath>
+ #endif
+
++// Move this vendored copy of flatbuffers to a private namespace,
++// but continue to access it through the "flatbuffers" alias.
++namespace arrow_vendored_private {
++namespace flatbuffers {
++}
++}
++namespace flatbuffers = arrow_vendored_private::flatbuffers;
++
++namespace arrow_vendored_private {
+ namespace flatbuffers {
+ // Generic 'operator==' with conditional specialisations.
+ // T e - new value of a scalar field.
+@@ -2777,6 +2786,7 @@ volatile __attribute__((weak)) const char
*flatbuffer_version_string =
+ }
+ /// @endcond
+ } // namespace flatbuffers
++} // namespace arrow_vendored_private
+
+ // clang-format on
+
+diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
+index 8bae61bfd..7e5a95233 100644
+--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
++++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
+@@ -25,6 +25,14 @@
+ #include <memory>
+ #include <limits>
+
++// Move this vendored copy of flatbuffers to a private namespace,
++// but continue to access it through the "flatbuffers" alias.
++namespace arrow_vendored_private {
++namespace flatbuffers {
++}
++}
++namespace flatbuffers = arrow_vendored_private::flatbuffers;
++
+ #if defined(_STLPORT_VERSION) && !defined(FLATBUFFERS_CPP98_STL)
+ #define FLATBUFFERS_CPP98_STL
+ #endif // defined(_STLPORT_VERSION) && !defined(FLATBUFFERS_CPP98_STL)
+@@ -44,6 +52,7 @@
+ #endif
+
+ // This header provides backwards compatibility for C++98 STLs like stlport.
++namespace arrow_vendored_private {
+ namespace flatbuffers {
+
+ // Retrieve ::back() from a string in a way that is compatible with pre C++11
+@@ -303,5 +312,6 @@ inline void vector_emplace_back(std::vector<T> *vector, V
&&data) {
+ #endif // !FLATBUFFERS_CPP98_STL
+
+ } // namespace flatbuffers
++} // namespace arrow_vendored_private
+
+ #endif // FLATBUFFERS_STL_EMULATION_H_
+--
+2.25.1
+```
diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
index fccce42f68..a00d5b0fd2 100644
--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
+++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/base.h
@@ -1,6 +1,14 @@
#ifndef FLATBUFFERS_BASE_H_
#define FLATBUFFERS_BASE_H_
+// Move this vendored copy of flatbuffers to a private namespace,
+// but continue to access it through the "flatbuffers" alias.
+namespace arrow_vendored_private {
+namespace flatbuffers {
+}
+}
+namespace flatbuffers = arrow_vendored_private::flatbuffers;
+
// clang-format off
// If activate should be declared and included first.
@@ -144,10 +152,12 @@
#define FLATBUFFERS_VERSION_REVISION 0
#define FLATBUFFERS_STRING_EXPAND(X) #X
#define FLATBUFFERS_STRING(X) FLATBUFFERS_STRING_EXPAND(X)
+namespace arrow_vendored_private {
namespace flatbuffers {
// Returns version as string "MAJOR.MINOR.REVISION".
const char* FLATBUFFERS_VERSION();
}
+}
#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) {
/// @endcond
/// @file
+namespace arrow_vendored_private {
namespace flatbuffers {
/// @cond FLATBUFFERS_INTERNAL
@@ -388,4 +403,5 @@ inline size_t PaddingBytes(size_t buf_size, size_t
scalar_size) {
}
} // namespace flatbuffers
+} // namespace arrow_vendored_private
#endif // FLATBUFFERS_BASE_H_
diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
index c4dc5bcd03..2f7eb5fcf5 100644
--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
+++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/flatbuffers.h
@@ -23,6 +23,15 @@
# include <cmath>
#endif
+// Move this vendored copy of flatbuffers to a private namespace,
+// but continue to access it through the "flatbuffers" alias.
+namespace arrow_vendored_private {
+namespace flatbuffers {
+}
+}
+namespace flatbuffers = arrow_vendored_private::flatbuffers;
+
+namespace arrow_vendored_private {
namespace flatbuffers {
// Generic 'operator==' with conditional specialisations.
// T e - new value of a scalar field.
@@ -2777,6 +2786,7 @@ volatile __attribute__((weak)) const char
*flatbuffer_version_string =
}
/// @endcond
} // namespace flatbuffers
+} // namespace arrow_vendored_private
// clang-format on
diff --git a/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
b/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
index 8bae61bfd6..7e5a95233b 100644
--- a/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
+++ b/cpp/thirdparty/flatbuffers/include/flatbuffers/stl_emulation.h
@@ -25,6 +25,14 @@
#include <memory>
#include <limits>
+// Move this vendored copy of flatbuffers to a private namespace,
+// but continue to access it through the "flatbuffers" alias.
+namespace arrow_vendored_private {
+namespace flatbuffers {
+}
+}
+namespace flatbuffers = arrow_vendored_private::flatbuffers;
+
#if defined(_STLPORT_VERSION) && !defined(FLATBUFFERS_CPP98_STL)
#define FLATBUFFERS_CPP98_STL
#endif // defined(_STLPORT_VERSION) && !defined(FLATBUFFERS_CPP98_STL)
@@ -44,6 +52,7 @@
#endif
// This header provides backwards compatibility for C++98 STLs like stlport.
+namespace arrow_vendored_private {
namespace flatbuffers {
// Retrieve ::back() from a string in a way that is compatible with pre C++11
@@ -303,5 +312,6 @@ inline void vector_emplace_back(std::vector<T> *vector, V
&&data) {
#endif // !FLATBUFFERS_CPP98_STL
} // namespace flatbuffers
+} // namespace arrow_vendored_private
#endif // FLATBUFFERS_STL_EMULATION_H_
