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

Explicitly mark RMM headers with RMM_EXPORT #1654

Conversation

robertmaynard
Copy link
Contributor

Description

Fixes #1652

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@robertmaynard robertmaynard requested a review from a team as a code owner August 20, 2024 17:54
@github-actions github-actions bot added the cpp Pertains to C++ code label Aug 20, 2024
@robertmaynard robertmaynard added bug Something isn't working non-breaking Non-breaking change labels Aug 20, 2024
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks! Changes make sense to me based on our offline conversations.

@harrism harrism changed the title Eplicitly mark RMM headers with RMM_EXPORT Explicitly mark RMM headers with RMM_EXPORT Aug 21, 2024
include/rmm/aligned.hpp Outdated Show resolved Hide resolved
@@ -81,7 +81,8 @@
* @endcode
*/

namespace rmm::mr {
namespace RMM_EXPORT rmm {
Copy link
Contributor

Choose a reason for hiding this comment

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

This now means that the existing RMM_EXPORT on get_map in this file could be removed, IIUC. However, I suspect we should keep it (see also #1653) so that if we ever decide to tighten up the export list we don't lose that correctness.

@robertmaynard robertmaynard force-pushed the bug/explicitly_mark_exception_types_with_export_vis branch from 406a1eb to 8df2207 Compare August 21, 2024 13:21
@robertmaynard robertmaynard force-pushed the bug/explicitly_mark_exception_types_with_export_vis branch from eba260d to 870632a Compare August 22, 2024 12:44
doxygen/Doxyfile Outdated Show resolved Hide resolved
@robertmaynard
Copy link
Contributor Author

/merge

@wence-
Copy link
Contributor

wence- commented Aug 22, 2024

Thanks @robertmaynard!

@rapids-bot rapids-bot bot merged commit 1555d6c into rapidsai:branch-24.10 Aug 22, 2024
50 checks passed
@robertmaynard robertmaynard deleted the bug/explicitly_mark_exception_types_with_export_vis branch August 26, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp Pertains to C++ code non-breaking Non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] Cannot catch RMM exceptions thrown across DSO boundaries
4 participants