You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pitrou (via GitHub)" <gi...@apache.org> on 2025/07/09 14:13:23 UTC
[PR] GH-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
pitrou opened a new pull request, #47042:
URL: https://github.com/apache/arrow/pull/47042
### Rationale for this change
The warning option `-Wmissing-declarations` allows finding private functions that erroneously have global linkage (because they are neither static nor in the anonymous namespace).
We only care about this for the public Arrow libraries, not for tests or utilities where it's harmless to have private functions that nevertheless have global linkage (and changing that would require a lot of pointless code churn).
### Are these changes tested?
Yes, on builds using gcc.
### Are there any user-facing changes?
No.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3083336185
@github-actions crossbow submit -g cpp
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3079101801
@github-actions crossbow submit *cpp-minimal*
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3083345022
Revision: 674a2ab6fe430a396d8858b8c8a687360d21b3e3
Submitted crossbow builds: [ursacomputing/crossbow @ actions-e1167ec300](https://github.com/ursacomputing/crossbow/branches/all?query=actions-e1167ec300)
|Task|Status|
|----|------|
|example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476613/job/46164892100)|
|example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476263/job/46164890958)|
|example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476600/job/46164892240)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476726/job/46164892726)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476442/job/46164891412)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476569/job/46164891864)|
|test-cuda-cpp-ubuntu-22.04-cuda-11.7.1|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476664/job/46164892411)|
|test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476843/job/46164893074)|
|test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476776/job/46164892926)|
|test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476965/job/46164893750)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476560/job/46164891810)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476653/job/46164892365)|
|test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476701/job/46164892554)|
|test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476385/job/46164891272)|
|test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476607/job/46164891993)|
|test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476421/job/46164891309)|
|test-ubuntu-24.04-cpp-bundled-offline|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476759/job/46164892849)|
|test-ubuntu-24.04-cpp-gcc-13-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476840/job/46164893084)|
|test-ubuntu-24.04-cpp-gcc-14|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476833/job/46164893005)|
|test-ubuntu-24.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476430/job/46164891374)|
|test-ubuntu-24.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/16341476727/job/46164892712)|
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3079187261
cc @kou and @lidavidm for the CMake and Flight changes, respectively.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2212678718
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -159,6 +159,9 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ${ARROW_POSITION_INDEPENDENT_CODE})
set(UNKNOWN_COMPILER_MESSAGE
"Unknown compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}")
+# Compiler flags used when building Arrow libraries (but not tests, utilities, etc.)
+set(LIBARROW_CXX_FLAGS)
Review Comment:
Looks good. Thanks!
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3078916052
Revision: 1b731774ae60b579673131524d8fcddd90f3dc26
Submitted crossbow builds: [ursacomputing/crossbow @ actions-b5c4cb9745](https://github.com/ursacomputing/crossbow/branches/all?query=actions-b5c4cb9745)
|Task|Status|
|----|------|
|example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559335/job/46103979148)|
|example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559779/job/46103980656)|
|example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559679/job/46103980298)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559318/job/46103979019)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559434/job/46103979596)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559848/job/46103981113)|
|test-cuda-cpp-ubuntu-22.04-cuda-11.7.1|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559499/job/46103979634)|
|test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/16322560030/job/46103981502)|
|test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559673/job/46103980314)|
|test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559663/job/46103980318)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559236/job/46103978659)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559529/job/46103979646)|
|test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559398/job/46103979249)|
|test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559640/job/46103980280)|
|test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559964/job/46103981193)|
|test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559231/job/46103978649)|
|test-ubuntu-24.04-cpp-bundled-offline|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559685/job/46103980330)|
|test-ubuntu-24.04-cpp-gcc-13-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559655/job/46103980264)|
|test-ubuntu-24.04-cpp-gcc-14|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559317/job/46103978947)|
|test-ubuntu-24.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559392/job/46103979284)|
|test-ubuntu-24.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/16322559257/job/46103978921)|
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2211684533
##########
cpp/src/arrow/flight/test_util.h:
##########
@@ -143,6 +143,9 @@ class ARROW_FLIGHT_EXPORT NumberingStream : public FlightDataStream {
ARROW_FLIGHT_EXPORT
std::shared_ptr<Schema> ExampleIntSchema();
+ARROW_FLIGHT_EXPORT
Review Comment:
The declaration for `ExampleFloatSchema()` is added but there's no corresponding implementation visible in the diff. Ensure that the implementation exists and is properly exported.
##########
cpp/src/arrow/filesystem/filesystem_library.h:
##########
@@ -26,7 +26,9 @@ extern "C" {
// _declspec(dllexport)/[[gnu::visibility("default")]] even when
// this header is #included by a non-arrow source, as in a third
// party filesystem implementation.
-ARROW_FORCE_EXPORT void* arrow_filesystem_get_registry() {
+ARROW_FORCE_EXPORT void* arrow_filesystem_get_registry();
Review Comment:
[nitpick] The function declaration is separated from its definition on line 31. This pattern of declaration followed immediately by definition in a header is unusual. Consider whether both the declaration and definition need to be in the header, or if one should be moved to a .cc file.
##########
cpp/src/arrow/extension/tensor_internal.h:
##########
@@ -25,8 +25,7 @@
namespace arrow::internal {
-ARROW_EXPORT
-Status IsPermutationValid(const std::vector<int64_t>& permutation) {
+inline Status IsPermutationValid(const std::vector<int64_t>& permutation) {
Review Comment:
[nitpick] The function `IsPermutationValid` was changed from `ARROW_EXPORT` to `inline` but remains in a header file. If this function is only used internally within a single compilation unit, consider moving it to the corresponding .cc file instead of making it inline in the header.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2211681432
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -159,6 +159,9 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ${ARROW_POSITION_INDEPENDENT_CODE})
set(UNKNOWN_COMPILER_MESSAGE
"Unknown compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}")
+# Compiler flags used when building Arrow libraries (but not tests, utilities, etc.)
+set(LIBARROW_CXX_FLAGS)
Review Comment:
How about renaming this to `ARROW_LIBRARY_ONLY_CXX_FLAGS` or something?
* We want to use `ARROW_` prefix for our variables
* I feel that `LIBARROW` means only for `libarrow.so` but this is for not only `libarrow.so` but also other libraries such as `libparquet.so`
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #47042:
URL: https://github.com/apache/arrow/pull/47042
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2212498346
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -159,6 +159,9 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ${ARROW_POSITION_INDEPENDENT_CODE})
set(UNKNOWN_COMPILER_MESSAGE
"Unknown compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}")
+# Compiler flags used when building Arrow libraries (but not tests, utilities, etc.)
+set(LIBARROW_CXX_FLAGS)
Review Comment:
+1 on this.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2212527011
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -159,6 +159,9 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ${ARROW_POSITION_INDEPENDENT_CODE})
set(UNKNOWN_COMPILER_MESSAGE
"Unknown compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}")
+# Compiler flags used when building Arrow libraries (but not tests, utilities, etc.)
+set(LIBARROW_CXX_FLAGS)
Review Comment:
Ok, I've made it `ARROW_LIBRARIES_ONLY_CXX_FLAGS`.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3052838275
:warning: GitHub issue #46272 **has been automatically assigned in GitHub** to PR creator.
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3079112078
Revision: b326f54ace4de55a4b98a3340ae6ce02ca239eea
Submitted crossbow builds: [ursacomputing/crossbow @ actions-0d601307e7](https://github.com/ursacomputing/crossbow/branches/all?query=actions-0d601307e7)
|Task|Status|
|----|------|
|example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/16323477236/job/46107275096)|
|example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/16323477699/job/46107276732)|
|test-ubuntu-24.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/16323477244/job/46107275146)|
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3080002710
This produces a very small reduction in code size:
* before:
```console
$ size /build/build-release/relwithdebinfo/*.so
text data bss dec hex filename
1641597 26416 832 1668845 1976ed /build/build-release/relwithdebinfo/libarrow_acero.so
14278498 115464 46544 14440506 dc583a /build/build-release/relwithdebinfo/libarrow_compute.so
201302 8168 3016 212486 33e06 /build/build-release/relwithdebinfo/libarrow_cuda.so
1611037 35128 4256 1650421 192ef5 /build/build-release/relwithdebinfo/libarrow_dataset.so
14373351 293720 2181025 16848096 10114e0 /build/build-release/relwithdebinfo/libarrow.so
1476139 30588 1640 1508367 17040f /build/build-release/relwithdebinfo/libarrow_testing.so
3952862 149024 5008 4106894 3eaa8e /build/build-release/relwithdebinfo/libparquet.so
```
* after:
```console
$ size /build/build-release/relwithdebinfo/*.so
text data bss dec hex filename
1641317 26416 1088 1668821 1976d5 /build/build-release/relwithdebinfo/libarrow_acero.so
14276349 115456 44632 14436437 dc4855 /build/build-release/relwithdebinfo/libarrow_compute.so
201302 8168 3016 212486 33e06 /build/build-release/relwithdebinfo/libarrow_cuda.so
1607787 35088 3384 1646259 191eb3 /build/build-release/relwithdebinfo/libarrow_dataset.so
14314903 292456 2183377 16790736 10034d0 /build/build-release/relwithdebinfo/libarrow.so
1474566 30588 3176 1508330 1703ea /build/build-release/relwithdebinfo/libarrow_testing.so
3945688 148992 4016 4098696 3e8a88 /build/build-release/relwithdebinfo/libparquet.so
```
--
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-46272: [C++] Build Arrow libraries with `-Wmissing-definitions` on gcc [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47042:
URL: https://github.com/apache/arrow/pull/47042#issuecomment-3078901574
@github-actions crossbow submit -g cpp
--
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