Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide visibility of non public symbols #15982

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4f90a11
Remove usage of null related detail functions
robertmaynard Jul 10, 2024
14fdb58
Explicitly export the pre-compiled aggregation templates
robertmaynard Jun 10, 2024
6324f11
Remove unneeded default parameters
robertmaynard Jun 10, 2024
4f3bc20
All cpp/ sources include the respective header
robertmaynard Jun 11, 2024
5126cae
Ensure minimal set of cpp/ functions used by tests are marked public
robertmaynard Jun 11, 2024
19ca57a
nvtext public API is marked as public symbols
robertmaynard Jun 11, 2024
9f0ffd0
cudftestutil public API is marked as public symbols
robertmaynard Jun 11, 2024
e5f59d5
cudf public API is marked as public symbols
robertmaynard Jun 11, 2024
0cb912a
Ensure installed cudf detail API is marked as public symbols
robertmaynard Jun 11, 2024
c99a58c
Add needed visibility patches for CCCL
robertmaynard Jun 11, 2024
0017f41
CMake now builds cudf and test util with hidden vis
robertmaynard Jun 10, 2024
162eb78
Correct style issue found by CI
robertmaynard Jun 12, 2024
7ee6fd7
Mark more detail functions as public
robertmaynard Jul 9, 2024
13f247d
Search src files include respective header
robertmaynard Jul 10, 2024
2b301e3
Restore child_columns_to_device_array
robertmaynard Jul 10, 2024
ae631cf
Added chunked_parquet_reader constructor comments
robertmaynard Jul 10, 2024
26986ef
Correct style issues found by CI
robertmaynard Jul 10, 2024
bc79512
Add missing header so copy symbols get marked as public
robertmaynard Jul 11, 2024
28d34df
Correctly mark all detail functions as public
robertmaynard Jul 11, 2024
df8ba16
Correct style issues found by CI
robertmaynard Jul 11, 2024
ed88b30
Add first pass doc update
robertmaynard Jul 11, 2024
bc85b9f
Update java/src/main/native/src/TableJni.cpp
robertmaynard Jul 11, 2024
d2261b5
Debug java missing symbols
robertmaynard Jul 15, 2024
c127579
Ensure lists::detail::concatenate is public
robertmaynard Jul 15, 2024
97541e6
Ensure include headers are marked with CUDF_EXPORT
robertmaynard Jul 17, 2024
22a6f24
more doc updates
robertmaynard Jul 22, 2024
642713a
Merge branch 'branch-24.08' into fea/hide_visibility_of_non_public_sy…
robertmaynard Jul 22, 2024
cd9d68b
Expose prefect API via CUDF_EXPORT
robertmaynard Jul 23, 2024
9c9200c
Merge branch 'branch-24.08' into fea/hide_visibility_of_non_public_sy…
robertmaynard Jul 23, 2024
90ada43
Expose io::json::detail as they are required by tests
robertmaynard Jul 23, 2024
b012799
Merge branch 'branch-24.08' into fea/hide_visibility_of_non_public_sy…
robertmaynard Jul 24, 2024
3784f33
Merge branch 'branch-24.08' into fea/hide_visibility_of_non_public_sy…
robertmaynard Jul 24, 2024
d76ebb7
Merge branch 'branch-24.08' into fea/hide_visibility_of_non_public_sy…
bdice Jul 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 4 additions & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,10 @@ set_target_properties(
CXX_STANDARD_REQUIRED ON
# For std:: support of __int128_t. Can be removed once using cuda::std
CXX_EXTENSIONS ON
CXX_VISIBILITY_PRESET hidden
CUDA_STANDARD 17
CUDA_STANDARD_REQUIRED ON
CUDA_VISIBILITY_PRESET hidden
POSITION_INDEPENDENT_CODE ON
INTERFACE_POSITION_INDEPENDENT_CODE ON
)
Expand Down Expand Up @@ -887,8 +889,10 @@ if(CUDF_BUILD_TESTUTIL)
# set target compile options
CXX_STANDARD 17
CXX_STANDARD_REQUIRED ON
CXX_VISIBILITY_PRESET hidden
CUDA_STANDARD 17
CUDA_STANDARD_REQUIRED ON
CUDA_VISIBILITY_PRESET hidden
POSITION_INDEPENDENT_CODE ON
INTERFACE_POSITION_INDEPENDENT_CODE ON
)
Expand Down
5 changes: 5 additions & 0 deletions cpp/cmake/thirdparty/patches/cccl_override.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
"packages" : {
"CCCL" : {
"patches" : [
{
"file" : "${current_json_dir}/cccl_symbol_visibility.diff",
"issue" : "Correct symbol visibility issues in libcudacxx [https://github.com/NVIDIA/cccl/pull/1832/]",
"fixed_in" : "2.6"
},
{
"file" : "${current_json_dir}/thrust_disable_64bit_dispatching.diff",
"issue" : "Remove 64bit dispatching as not needed by libcudf and results in compiling twice as many kernels [https://github.com/rapidsai/cudf/pull/11437]",
Expand Down
27 changes: 27 additions & 0 deletions cpp/cmake/thirdparty/patches/cccl_symbol_visibility.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
diff --git a/libcudacxx/include/cuda/std/detail/libcxx/include/__config b/libcudacxx/include/cuda/std/detail/libcxx/include/__config
index e7c62c031b..5db861853a 100644
--- a/libcudacxx/include/cuda/std/detail/libcxx/include/__config
+++ b/libcudacxx/include/cuda/std/detail/libcxx/include/__config
@@ -1049,7 +1049,6 @@ typedef __char32_t char32_t;
# define _LIBCUDACXX_EXPORTED_FROM_ABI __declspec(dllimport)
# endif

-# define _LIBCUDACXX_TYPE_VIS _LIBCUDACXX_DLL_VIS
# define _LIBCUDACXX_FUNC_VIS _LIBCUDACXX_DLL_VIS
# define _LIBCUDACXX_EXCEPTION_ABI _LIBCUDACXX_DLL_VIS
# define _LIBCUDACXX_HIDDEN
@@ -1448,14 +1447,6 @@ __sanitizer_annotate_contiguous_container(const void*, const void*, const void*,
# define _LIBCUDACXX_WEAK __attribute__((__weak__))
# endif

-// Redefine some macros for internal use
-# if defined(__cuda_std__)
-# undef _LIBCUDACXX_FUNC_VIS
-# define _LIBCUDACXX_FUNC_VIS _LIBCUDACXX_INLINE_VISIBILITY
-# undef _LIBCUDACXX_TYPE_VIS
-# define _LIBCUDACXX_TYPE_VIS
-# endif // __cuda_std__
-
// Thread API
# ifndef _LIBCUDACXX_HAS_THREAD_API_EXTERNAL
# if defined(_CCCL_COMPILER_NVRTC) || defined(__EMSCRIPTEN__)
27 changes: 24 additions & 3 deletions cpp/doxygen/developer_guide/DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,36 @@ header file in `cudf/cpp/include/cudf/`. For example, `cudf/cpp/include/cudf/cop
contains the APIs for functions related to copying from one column to another. Note the `.hpp`
file extension used to indicate a C++ header file.

Header files should use the `#pragma once` include guard.
External/public libcudf C++ API header files need to mark all symbols inside of them with `CUDF_EXPORT`.
This is done by placing the macro on the `namespace cudf` as seen below. Markup on namespace
require them not to be nested, so the `cudf` namespace must be kept by itself.

```c++

#pragma once

namespace CUDF_EXPORT cudf {
namespace lists {

...


} // namespace lists
} // namespace CUDF_EXPORT cudf

```


The naming of external API headers should be consistent with the name of the folder that contains
the source files that implement the API. For example, the implementation of the APIs found in
`cudf/cpp/include/cudf/copying.hpp` are located in `cudf/src/copying`. Likewise, the unit tests for
the APIs reside in `cudf/tests/copying/`.

Internal API headers containing `detail` namespace definitions that are used across translation
units inside libcudf should be placed in `include/cudf/detail`.
Internal API headers containing `detail` namespace definitions that are either used across translation
units inside libcudf should be placed in `include/cudf/detail`. Just like the public C++ API headers, any
internal C++ API header requires `CUDF_EXPORT` markup on the `cudf` namespace so that the functions can be tested.

All headers in cudf should use `#pragma once` for include guards.

## File extensions

Expand Down
6 changes: 3 additions & 3 deletions cpp/doxygen/developer_guide/DOCUMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ Here is an example of a doxygen description comment for a namespace declaration.
*
* This is the top-level namespace which contains all cuDF functions and types.
*/
namespace cudf {
namespace CUDF_EXPORT cudf {

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

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.

namespace cudf {
namespace CUDF_EXPORT cudf {

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

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

namespace cudf {
namespace CUDF_EXPORT cudf {
/**
* @addtogroup transformation_fill
* @{
Expand Down
5 changes: 3 additions & 2 deletions cpp/include/cudf/aggregation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#pragma once

#include <cudf/types.hpp>
#include <cudf/utilities/export.hpp>

#include <functional>
#include <memory>
Expand All @@ -31,7 +32,7 @@
* individual function documentation to see what aggregations are supported.
*/

namespace cudf {
namespace CUDF_EXPORT cudf {
/**
* @addtogroup aggregation_factories
* @{
Expand Down Expand Up @@ -770,4 +771,4 @@ template <typename Base>
std::unique_ptr<Base> make_merge_tdigest_aggregation(int max_centroids = 1000);

/** @} */ // end of group
} // namespace cudf
} // namespace CUDF_EXPORT cudf
11 changes: 4 additions & 7 deletions cpp/include/cudf/ast/detail/expression_parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@
#include <numeric>
#include <optional>

namespace cudf {
namespace ast {
namespace detail {
namespace CUDF_EXPORT cudf {
namespace ast::detail {

/**
* @brief Node data reference types.
Expand Down Expand Up @@ -328,8 +327,6 @@ class expression_parser {
std::vector<generic_scalar_device_view> _literals;
};

} // namespace detail
} // namespace ast::detail

} // namespace ast

} // namespace cudf
} // namespace CUDF_EXPORT cudf
10 changes: 7 additions & 3 deletions cpp/include/cudf/ast/detail/expression_transformer.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

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

#include <cudf/ast/expressions.hpp>

namespace cudf::ast::detail {
namespace CUDF_EXPORT cudf {
namespace ast::detail {
/**
* @brief Base "visitor" pattern class with the `expression` class for expression transformer.
*
Expand Down Expand Up @@ -61,4 +62,7 @@ class expression_transformer {

virtual ~expression_transformer() {}
};
} // namespace cudf::ast::detail

} // namespace ast::detail

} // namespace CUDF_EXPORT cudf
4 changes: 2 additions & 2 deletions cpp/include/cudf/ast/detail/operators.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#include <utility>
#include <vector>

namespace cudf {
namespace CUDF_EXPORT cudf {

namespace ast {

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

} // namespace ast

} // namespace cudf
} // namespace CUDF_EXPORT cudf
4 changes: 2 additions & 2 deletions cpp/include/cudf/ast/expressions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

#include <cstdint>

namespace cudf {
namespace CUDF_EXPORT cudf {
namespace ast {
/**
* @addtogroup expressions
Expand Down Expand Up @@ -555,4 +555,4 @@ class column_name_reference : public expression {
/** @} */ // end of group
} // namespace ast

} // namespace cudf
} // namespace CUDF_EXPORT cudf
20 changes: 11 additions & 9 deletions cpp/include/cudf/binaryop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@

#include <cudf/column/column.hpp>
#include <cudf/scalar/scalar.hpp>
#include <cudf/utilities/export.hpp>

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

#include <memory>

namespace cudf {
namespace CUDF_EXPORT cudf {

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

namespace compiled {
namespace detail {
} // namespace binops

/** @} */ // end of group
} // namespace CUDF_EXPORT cudf

namespace CUDF_EXPORT cudf {
namespace binops::compiled::detail {

/**
* @brief struct binary operation using `NaN` aware sorting physical element comparators
Expand All @@ -337,9 +343,5 @@ void apply_sorting_struct_binary_op(mutable_column_view& out,
bool is_rhs_scalar,
binary_operator op,
rmm::cuda_stream_view stream);
} // namespace detail
} // namespace compiled
} // namespace binops

/** @} */ // end of group
} // namespace cudf
} // namespace binops::compiled::detail
} // namespace CUDF_EXPORT cudf
4 changes: 2 additions & 2 deletions cpp/include/cudf/column/column.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* @brief Class definition for cudf::column
*/

namespace cudf {
namespace CUDF_EXPORT cudf {

/**
* @brief A container of nullable device data as a column of elements.
Expand Down Expand Up @@ -332,4 +332,4 @@ class column {
};

/** @} */ // end of group
} // namespace cudf
} // namespace CUDF_EXPORT cudf
4 changes: 2 additions & 2 deletions cpp/include/cudf/column/column_device_view.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* @brief Column device view class definitions
*/

namespace cudf {
namespace CUDF_EXPORT cudf {

/**
* @brief Indicates the presence of nulls at compile-time or runtime.
Expand Down Expand Up @@ -1527,4 +1527,4 @@ ColumnDeviceView* child_columns_to_device_array(ColumnViewIterator child_begin,
}

} // namespace detail
} // namespace cudf
} // namespace CUDF_EXPORT cudf
4 changes: 2 additions & 2 deletions cpp/include/cudf/column/column_factories.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

#include <thrust/pair.h>

namespace cudf {
namespace CUDF_EXPORT cudf {
/**
* @addtogroup column_factories
* @{
Expand Down Expand Up @@ -571,4 +571,4 @@ std::unique_ptr<column> make_dictionary_from_scalar(
rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource());

/** @} */ // end of group
} // namespace cudf
} // namespace CUDF_EXPORT cudf
7 changes: 4 additions & 3 deletions cpp/include/cudf/column/column_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@
* @file column_view.hpp
* @brief column view class definitions
*/

namespace cudf {
namespace CUDF_EXPORT cudf {
namespace detail {
/**
* @brief A non-owning, immutable view of device data as a column of elements,
Expand Down Expand Up @@ -296,6 +295,7 @@ class column_view_base {
size_type null_count,
size_type offset = 0);
};

} // namespace detail

/**
Expand Down Expand Up @@ -797,5 +797,6 @@ std::size_t shallow_hash(column_view const& input);
* @return If `lhs` and `rhs` have equivalent shallow state
*/
bool is_shallow_equivalent(column_view const& lhs, column_view const& rhs);

} // namespace detail
} // namespace cudf
} // namespace CUDF_EXPORT cudf
5 changes: 3 additions & 2 deletions cpp/include/cudf/concatenate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@
#include <cudf/column/column_view.hpp>
#include <cudf/table/table_view.hpp>
#include <cudf/utilities/default_stream.hpp>
#include <cudf/utilities/export.hpp>
#include <cudf/utilities/span.hpp>

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

#include <memory>

namespace cudf {
namespace CUDF_EXPORT cudf {
/**
* @addtogroup copy_concatenate
* @{
Expand Down Expand Up @@ -97,4 +98,4 @@ std::unique_ptr<table> concatenate(
rmm::device_async_resource_ref mr = rmm::mr::get_current_device_resource());

/** @} */ // end of group
} // namespace cudf
} // namespace CUDF_EXPORT cudf
Loading
Loading