Inline functions that return static references must have default visibility

[wence-]

Description

In #833, we gave rmm::mr::detail::get_map default visibility. However, there are a number of other functions that return static references that should also have this visibility so that the static reference is unique across multiple DSOs.

• Closes #1651

Checklist

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

[wence-]

Technically this is "breaking", since it changes symbol visibility. ~But I think it's ok to be non-breaking.~ Set as breaking per discussion below.

[wence-]

doc build fails because breathe doesn't know how to deal with the definition RMM_EXPORT inline spdlog::logger& logger. I don't know enough doxygen to figure out how to elide the RMM_EXPORT

[harrism]

Why shouldn't we mark it as breaking? All that does is put it under the "breaking changes" section of the changelog. I think for future reference it should be labeled "breaking". Do you agree?

[harrism]

Hmm, looks like breathe doesn't like RMM_EXPORT:

/__w/rmm/rmm/python/rmm/docs/librmm_docs/logging.rst:4: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 18]

    inline RMM_EXPORT spdlog::logger & logger ()
    ------------------^
If the function has a return type:
  Error in declarator or parameters-and-qualifiers
  If pointer to member declarator:
    Invalid C++ declaration: Expected '::' in pointer to member (function). [error at 33]
      inline RMM_EXPORT spdlog::logger & logger ()
      ---------------------------------^
  If declarator-id:
    Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 33]
      inline RMM_EXPORT spdlog::logger & logger ()
      ---------------------------------^

[wence-]

This will be subsumed by #1654, but it's probably safe to belt-and-braces it if we can figure out that docs issue.

[wence-]

Why shouldn't we mark it as breaking? All that does is put it under the "breaking changes" section of the changelog. I think for future reference it should be labeled "breaking". Do you agree?

Yes, done.

[wence-]

botched the merge, will look tomorrow

[vyasr]

Can we close this (and #1651) now that #1654 is resolved?

[harrism]

We want these functions explicitly marked in case the namespace export ever changes. However my per device resource PR also has these changes...

[wence-]

After lots of fighting, I got doxygen working right. Unfortunately we have to reproduce the definitions of the macros in the doxyfile since it turns out that it's impossible to make EXCLUDE_PATTERNS work with wanting to have the doxygen preprocessor actually find the detail files that are included.

[wence-]

/merge