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.

Sign up for GitHub

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

Don't ignore GCC-specific warning under Clang #1557

Merged
merged 1 commit into from
May 14, 2024
Merged

Don't ignore GCC-specific warning under Clang #1557

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions include/rmm/mr/device/pool_memory_resource.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,19 @@ struct maybe_remove_property<PoolResource,
Upstream,
Property,
cuda::std::enable_if_t<!cuda::has_property<Upstream, Property>>> {
#ifdef __GNUC__ // GCC warns about compatibility issues with pre ISO C++ code
#if defined(__GNUC__) && !defined(__clang__) // GCC warns about compatibility
Copy link
Member

Choose a reason for hiding this comment

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

Just thought of this: Under what circumstances are __GNUC__ and __clang__ both defined? They must be or we wouldn't need this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • __GNUC__ means "compatibility with GNU C". This is defined by both GCC and Clang.
  • __clang__ means Clang. I'd guess that this is also defined by clang derivatives such as HIPCC and DPC++. But it's not defined by GCC.
  • AFAIK GCC doesn't have a similar macro. I.e. there is no __gcc__ macro.

Under what circumstances are __GNUC__ and __clang__ both defined?

When you run a compilation with clang this is the case.

The "-Wnon-template-friend" exists in GCC, but not in Clang. With the way things are now we're essentially saying: If you're a GNU C compatible compiler, but you're not Clang (or a Clang derivative where we can expect warning flags to be reasonably similar to clang), then add -Wnon-template-friend to the ignorelist.

Before this patch Clang triggered warnings like warning: ignoring unknown warning which spammed the logs.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the informative answer!

// issues with pre ISO C++ code
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wnon-template-friend"
#endif // __GNUC__
#endif // __GNUC__ and not __clang__
/**
* @brief Explicit removal of the friend function so we do not pretend to provide device
* accessible memory
*/
friend void get_property(const PoolResource&, Property) = delete;
#ifdef __GNUC__
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif // __GNUC__
#endif // __GNUC__ and not __clang__
};
} // namespace detail

Expand Down
Loading