Skip to content

Commit d953676

Browse files
Hide visibility of non public symbols (rapidsai#15982)
Converts cudf over to a system of explicit markup of what symbols should be used by consumers. This is done by compiling with `-fvisibility=hidden` and explicit markup via `CUDF_EXPORT` of components we want usable. Due to issues with tests a portion of `include/` detail functions had to be marked as public API. More concernning are that the tests leverage functions from `cpp/` that are never part of the installed headers. That set of files can be found at rapidsai@16b3656 and we should discuss how we should restructure cudf to remove these. Authors: - Robert Maynard (https://github.com/robertmaynard) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: rapidsai#15982
1 parent 4aefcc7 commit d953676

File tree

321 files changed

+1326
-956
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

321 files changed

+1326
-956
lines changed

cpp/CMakeLists.txt

+4
Original file line numberDiff line numberDiff line change
@@ -711,8 +711,10 @@ set_target_properties(
711711
CXX_STANDARD_REQUIRED ON
712712
# For std:: support of __int128_t. Can be removed once using cuda::std
713713
CXX_EXTENSIONS ON
714+
CXX_VISIBILITY_PRESET hidden
714715
CUDA_STANDARD 17
715716
CUDA_STANDARD_REQUIRED ON
717+
CUDA_VISIBILITY_PRESET hidden
716718
POSITION_INDEPENDENT_CODE ON
717719
INTERFACE_POSITION_INDEPENDENT_CODE ON
718720
)
@@ -887,8 +889,10 @@ if(CUDF_BUILD_TESTUTIL)
887889
# set target compile options
888890
CXX_STANDARD 17
889891
CXX_STANDARD_REQUIRED ON
892+
CXX_VISIBILITY_PRESET hidden
890893
CUDA_STANDARD 17
891894
CUDA_STANDARD_REQUIRED ON
895+
CUDA_VISIBILITY_PRESET hidden
892896
POSITION_INDEPENDENT_CODE ON
893897
INTERFACE_POSITION_INDEPENDENT_CODE ON
894898
)

cpp/cmake/thirdparty/patches/cccl_override.json

+5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
"packages" : {
44
"CCCL" : {
55
"patches" : [
6+
{
7+
"file" : "${current_json_dir}/cccl_symbol_visibility.diff",
8+
"issue" : "Correct symbol visibility issues in libcudacxx [https://github.com/NVIDIA/cccl/pull/1832/]",
9+
"fixed_in" : "2.6"
10+
},
611
{
712
"file" : "${current_json_dir}/thrust_disable_64bit_dispatching.diff",
813
"issue" : "Remove 64bit dispatching as not needed by libcudf and results in compiling twice as many kernels [https://github.com/rapidsai/cudf/pull/11437]",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
diff --git a/libcudacxx/include/cuda/std/detail/libcxx/include/__config b/libcudacxx/include/cuda/std/detail/libcxx/include/__config
2+
index e7c62c031b..5db861853a 100644
3+
--- a/libcudacxx/include/cuda/std/detail/libcxx/include/__config
4+
+++ b/libcudacxx/include/cuda/std/detail/libcxx/include/__config
5+
@@ -1049,7 +1049,6 @@ typedef __char32_t char32_t;
6+
# define _LIBCUDACXX_EXPORTED_FROM_ABI __declspec(dllimport)
7+
# endif
8+
9+
-# define _LIBCUDACXX_TYPE_VIS _LIBCUDACXX_DLL_VIS
10+
# define _LIBCUDACXX_FUNC_VIS _LIBCUDACXX_DLL_VIS
11+
# define _LIBCUDACXX_EXCEPTION_ABI _LIBCUDACXX_DLL_VIS
12+
# define _LIBCUDACXX_HIDDEN
13+
@@ -1448,14 +1447,6 @@ __sanitizer_annotate_contiguous_container(const void*, const void*, const void*,
14+
# define _LIBCUDACXX_WEAK __attribute__((__weak__))
15+
# endif
16+
17+
-// Redefine some macros for internal use
18+
-# if defined(__cuda_std__)
19+
-# undef _LIBCUDACXX_FUNC_VIS
20+
-# define _LIBCUDACXX_FUNC_VIS _LIBCUDACXX_INLINE_VISIBILITY
21+
-# undef _LIBCUDACXX_TYPE_VIS
22+
-# define _LIBCUDACXX_TYPE_VIS
23+
-# endif // __cuda_std__
24+
-
25+
// Thread API
26+
# ifndef _LIBCUDACXX_HAS_THREAD_API_EXTERNAL
27+
# if defined(_CCCL_COMPILER_NVRTC) || defined(__EMSCRIPTEN__)

cpp/doxygen/developer_guide/DEVELOPER_GUIDE.md

+24-3
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,36 @@ header file in `cudf/cpp/include/cudf/`. For example, `cudf/cpp/include/cudf/cop
5252
contains the APIs for functions related to copying from one column to another. Note the `.hpp`
5353
file extension used to indicate a C++ header file.
5454

55-
Header files should use the `#pragma once` include guard.
55+
External/public libcudf C++ API header files need to mark all symbols inside of them with `CUDF_EXPORT`.
56+
This is done by placing the macro on the `namespace cudf` as seen below. Markup on namespace
57+
require them not to be nested, so the `cudf` namespace must be kept by itself.
58+
59+
```c++
60+
61+
#pragma once
62+
63+
namespace CUDF_EXPORT cudf {
64+
namespace lists {
65+
66+
...
67+
68+
69+
} // namespace lists
70+
} // namespace CUDF_EXPORT cudf
71+
72+
```
73+
5674
5775
The naming of external API headers should be consistent with the name of the folder that contains
5876
the source files that implement the API. For example, the implementation of the APIs found in
5977
`cudf/cpp/include/cudf/copying.hpp` are located in `cudf/src/copying`. Likewise, the unit tests for
6078
the APIs reside in `cudf/tests/copying/`.
6179
62-
Internal API headers containing `detail` namespace definitions that are used across translation
63-
units inside libcudf should be placed in `include/cudf/detail`.
80+
Internal API headers containing `detail` namespace definitions that are either used across translation
81+
units inside libcudf should be placed in `include/cudf/detail`. Just like the public C++ API headers, any
82+
internal C++ API header requires `CUDF_EXPORT` markup on the `cudf` namespace so that the functions can be tested.
83+
84+
All headers in cudf should use `#pragma once` for include guards.
6485
6586
## File extensions
6687

cpp/doxygen/developer_guide/DOCUMENTATION.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ Here is an example of a doxygen description comment for a namespace declaration.
363363
*
364364
* This is the top-level namespace which contains all cuDF functions and types.
365365
*/
366-
namespace cudf {
366+
namespace CUDF_EXPORT cudf {
367367

368368
A description comment should be included only once for each unique namespace declaration.
369369
Otherwise, if more than one description is found, doxygen aggregates the descriptions in an arbitrary order in the output pages.
@@ -385,7 +385,7 @@ The existing groups have been carefully structured and named, so new groups shou
385385

386386
When creating a new API, specify its group using the [\@ingroup](https://www.doxygen.nl/manual/commands.html#cmdingroup) tag and the group reference id from the [doxygen_groups.h](../include/doxygen_groups.h) file.
387387

388-
namespace cudf {
388+
namespace CUDF_EXPORT cudf {
389389

390390
/**
391391
* @brief ...
@@ -401,7 +401,7 @@ When creating a new API, specify its group using the [\@ingroup](https://www.dox
401401

402402
You can also use the \@addtogroup with a `@{ ... @}` pair to automatically include doxygen comment blocks as part of a group.
403403

404-
namespace cudf {
404+
namespace CUDF_EXPORT cudf {
405405
/**
406406
* @addtogroup transformation_fill
407407
* @{

cpp/include/cudf/aggregation.hpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#pragma once
1818

1919
#include <cudf/types.hpp>
20+
#include <cudf/utilities/export.hpp>
2021

2122
#include <functional>
2223
#include <memory>
@@ -31,7 +32,7 @@
3132
* individual function documentation to see what aggregations are supported.
3233
*/
3334

34-
namespace cudf {
35+
namespace CUDF_EXPORT cudf {
3536
/**
3637
* @addtogroup aggregation_factories
3738
* @{
@@ -770,4 +771,4 @@ template <typename Base>
770771
std::unique_ptr<Base> make_merge_tdigest_aggregation(int max_centroids = 1000);
771772

772773
/** @} */ // end of group
773-
} // namespace cudf
774+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/ast/detail/expression_parser.hpp

+4-7
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,8 @@
2929
#include <numeric>
3030
#include <optional>
3131

32-
namespace cudf {
33-
namespace ast {
34-
namespace detail {
32+
namespace CUDF_EXPORT cudf {
33+
namespace ast::detail {
3534

3635
/**
3736
* @brief Node data reference types.
@@ -328,8 +327,6 @@ class expression_parser {
328327
std::vector<generic_scalar_device_view> _literals;
329328
};
330329

331-
} // namespace detail
330+
} // namespace ast::detail
332331

333-
} // namespace ast
334-
335-
} // namespace cudf
332+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/ast/detail/expression_transformer.hpp

+7-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
/*
3-
* Copyright (c) 2023, NVIDIA CORPORATION.
3+
* Copyright (c) 2023-2024, NVIDIA CORPORATION.
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -18,7 +18,8 @@
1818

1919
#include <cudf/ast/expressions.hpp>
2020

21-
namespace cudf::ast::detail {
21+
namespace CUDF_EXPORT cudf {
22+
namespace ast::detail {
2223
/**
2324
* @brief Base "visitor" pattern class with the `expression` class for expression transformer.
2425
*
@@ -61,4 +62,7 @@ class expression_transformer {
6162

6263
virtual ~expression_transformer() {}
6364
};
64-
} // namespace cudf::ast::detail
65+
66+
} // namespace ast::detail
67+
68+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/ast/detail/operators.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include <utility>
3030
#include <vector>
3131

32-
namespace cudf {
32+
namespace CUDF_EXPORT cudf {
3333

3434
namespace ast {
3535

@@ -1233,4 +1233,4 @@ CUDF_HOST_DEVICE inline cudf::size_type ast_operator_arity(ast_operator op)
12331233

12341234
} // namespace ast
12351235

1236-
} // namespace cudf
1236+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/ast/expressions.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
#include <cstdint>
2525

26-
namespace cudf {
26+
namespace CUDF_EXPORT cudf {
2727
namespace ast {
2828
/**
2929
* @addtogroup expressions
@@ -555,4 +555,4 @@ class column_name_reference : public expression {
555555
/** @} */ // end of group
556556
} // namespace ast
557557

558-
} // namespace cudf
558+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/binaryop.hpp

+11-9
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@
1818

1919
#include <cudf/column/column.hpp>
2020
#include <cudf/scalar/scalar.hpp>
21+
#include <cudf/utilities/export.hpp>
2122

2223
#include <rmm/mr/device/per_device_resource.hpp>
2324
#include <rmm/resource_ref.hpp>
2425

2526
#include <memory>
2627

27-
namespace cudf {
28+
namespace CUDF_EXPORT cudf {
2829

2930
/**
3031
* @addtogroup transformation_binaryops
@@ -316,8 +317,13 @@ std::pair<rmm::device_buffer, size_type> scalar_col_valid_mask_and(
316317
rmm::cuda_stream_view stream = cudf::get_default_stream(),
317318
rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource());
318319

319-
namespace compiled {
320-
namespace detail {
320+
} // namespace binops
321+
322+
/** @} */ // end of group
323+
} // namespace CUDF_EXPORT cudf
324+
325+
namespace CUDF_EXPORT cudf {
326+
namespace binops::compiled::detail {
321327

322328
/**
323329
* @brief struct binary operation using `NaN` aware sorting physical element comparators
@@ -337,9 +343,5 @@ void apply_sorting_struct_binary_op(mutable_column_view& out,
337343
bool is_rhs_scalar,
338344
binary_operator op,
339345
rmm::cuda_stream_view stream);
340-
} // namespace detail
341-
} // namespace compiled
342-
} // namespace binops
343-
344-
/** @} */ // end of group
345-
} // namespace cudf
346+
} // namespace binops::compiled::detail
347+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/column/column.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
* @brief Class definition for cudf::column
3737
*/
3838

39-
namespace cudf {
39+
namespace CUDF_EXPORT cudf {
4040

4141
/**
4242
* @brief A container of nullable device data as a column of elements.
@@ -332,4 +332,4 @@ class column {
332332
};
333333

334334
/** @} */ // end of group
335-
} // namespace cudf
335+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/column/column_device_view.cuh

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
* @brief Column device view class definitions
4545
*/
4646

47-
namespace cudf {
47+
namespace CUDF_EXPORT cudf {
4848

4949
/**
5050
* @brief Indicates the presence of nulls at compile-time or runtime.
@@ -1527,4 +1527,4 @@ ColumnDeviceView* child_columns_to_device_array(ColumnViewIterator child_begin,
15271527
}
15281528

15291529
} // namespace detail
1530-
} // namespace cudf
1530+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/column/column_factories.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
#include <thrust/pair.h>
2929

30-
namespace cudf {
30+
namespace CUDF_EXPORT cudf {
3131
/**
3232
* @addtogroup column_factories
3333
* @{
@@ -571,4 +571,4 @@ std::unique_ptr<column> make_dictionary_from_scalar(
571571
rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource());
572572

573573
/** @} */ // end of group
574-
} // namespace cudf
574+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/column/column_view.hpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
* @file column_view.hpp
3232
* @brief column view class definitions
3333
*/
34-
35-
namespace cudf {
34+
namespace CUDF_EXPORT cudf {
3635
namespace detail {
3736
/**
3837
* @brief A non-owning, immutable view of device data as a column of elements,
@@ -296,6 +295,7 @@ class column_view_base {
296295
size_type null_count,
297296
size_type offset = 0);
298297
};
298+
299299
} // namespace detail
300300

301301
/**
@@ -797,5 +797,6 @@ std::size_t shallow_hash(column_view const& input);
797797
* @return If `lhs` and `rhs` have equivalent shallow state
798798
*/
799799
bool is_shallow_equivalent(column_view const& lhs, column_view const& rhs);
800+
800801
} // namespace detail
801-
} // namespace cudf
802+
} // namespace CUDF_EXPORT cudf

cpp/include/cudf/concatenate.hpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
#include <cudf/column/column_view.hpp>
1919
#include <cudf/table/table_view.hpp>
2020
#include <cudf/utilities/default_stream.hpp>
21+
#include <cudf/utilities/export.hpp>
2122
#include <cudf/utilities/span.hpp>
2223

2324
#include <rmm/mr/device/per_device_resource.hpp>
2425
#include <rmm/resource_ref.hpp>
2526

2627
#include <memory>
2728

28-
namespace cudf {
29+
namespace CUDF_EXPORT cudf {
2930
/**
3031
* @addtogroup copy_concatenate
3132
* @{
@@ -97,4 +98,4 @@ std::unique_ptr<table> concatenate(
9798
rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource());
9899

99100
/** @} */ // end of group
100-
} // namespace cudf
101+
} // namespace CUDF_EXPORT cudf

0 commit comments

Comments
 (0)