You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ap...@apache.org on 2022/08/08 07:39:26 UTC
[arrow] branch master updated: ARROW-17280: [C++] Move vendored flatbuffers to private namespace (#13775)
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 <ko...@users.noreply.github.com>
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 <co...@deephaven.io>
Signed-off-by: Antoine Pitrou <pi...@free.fr>
---
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_