Skip to content

Fix symbol export#3006

Merged
rapids-bot[bot] merged 5 commits intorapidsai:mainfrom
vyasr:fix/symbol_export
May 8, 2026
Merged

Fix symbol export#3006
rapids-bot[bot] merged 5 commits intorapidsai:mainfrom
vyasr:fix/symbol_export

Conversation

@vyasr
Copy link
Copy Markdown
Contributor

@vyasr vyasr commented Apr 29, 2026

This PR adds symbol visibility controls to raft to avoid exporting weak symbols that it shouldn't.

Contributes to https://github.com/rapidsai/build-infra/issues/53

@vyasr vyasr self-assigned this Apr 29, 2026
@vyasr vyasr requested review from a team as code owners April 29, 2026 17:11
@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 29, 2026
Comment thread cpp/include/raft/core/detail/macros.hpp
Copy link
Copy Markdown
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect we need all installed RAFT headers to have the markup of RAFT_EXPORT otherwise you can't pass a raft handle from a library that use raft into raft itself.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 30, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

#include <raft/util/vectorized.cuh>

namespace raft::detail {
namespace RAFT_EXPORT raft {
Copy link
Copy Markdown
Contributor

@csadorf csadorf Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the raft::detail namespace is not considered part of the public API. I think we should roll this out in stages though and maybe drop that in a follow-up to minimize discruption.

vyasr added 2 commits April 30, 2026 15:15
Remove the separate _FUNCTION macro variants since RAFT_EXPORT can be
used directly on function declarations. Update raft_runtime headers to
use RAFT_EXPORT instead of the removed RAFT_EXPORT_FUNCTION.
@vyasr vyasr force-pushed the fix/symbol_export branch from b30e484 to 0942b42 Compare April 30, 2026 22:15
@vyasr vyasr requested a review from robertmaynard May 1, 2026 04:29
@vyasr
Copy link
Copy Markdown
Contributor Author

vyasr commented May 8, 2026

/merge

@vyasr vyasr dismissed robertmaynard’s stale review May 8, 2026 20:27

Addressed and stale

@rapids-bot rapids-bot Bot merged commit da7efb0 into rapidsai:main May 8, 2026
293 of 301 checks passed
@vyasr vyasr deleted the fix/symbol_export branch May 8, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants