Skip to content

Commit

Permalink
Move [[nodiscard]] to non-friend declaration (#339)
Browse files Browse the repository at this point in the history
`[[nodiscard]]` attribute does not have effect on `friend` declaration, and Clang compiler rejects it:
```
ucxx/cpp/include/ucxx/context.h:75:3: error: an attribute list cannot appear here
   75 |   [[nodiscard]] friend std::shared_ptr<Context> createContext(ConfigMap ucxConfig,
      |   ^~~~~~~~~~~~~
```

It also triggers `-Wattributes` in GCC:
```
ucxx/cpp/include/ucxx/context.h:75:49: warning: attribute ignored [-Wattributes]
   75 |   [[nodiscard]] friend std::shared_ptr<Context> createContext(ConfigMap ucxConfig,
      |                                                 ^~~~~~~~~~~~~
ucxx/cpp/include/ucxx/context.h:75:49: note: an attribute that appertains to a friend declaration that is not a definition is ignored
```

Move the `[[nodiscard]]` attribute to the non-friend declaration.

Authors:
  - Yuan Tong (https://github.com/tongyuantongyu)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: #339
  • Loading branch information
tongyuantongyu authored Nov 29, 2024
1 parent be0965b commit 16eaa57
Show file tree
Hide file tree
Showing 19 changed files with 81 additions and 83 deletions.
5 changes: 2 additions & 3 deletions cpp/include/ucxx/address.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ class Address : public Component {
*
* @returns The `shared_ptr<ucxx::Address>` object.
*/
[[nodiscard]] friend std::shared_ptr<Address> createAddressFromWorker(
std::shared_ptr<Worker> worker);
friend std::shared_ptr<Address> createAddressFromWorker(std::shared_ptr<Worker> worker);

/**
* @brief Constructor for `shared_ptr<ucxx::Address>` from string.
Expand All @@ -76,7 +75,7 @@ class Address : public Component {
*
* @returns The `shared_ptr<ucxx::Address>` object.
*/
[[nodiscard]] friend std::shared_ptr<Address> createAddressFromString(std::string addressString);
friend std::shared_ptr<Address> createAddressFromString(std::string addressString);

/**
* @brief Get the underlying `ucp_address_t*` handle.
Expand Down
70 changes: 35 additions & 35 deletions cpp/include/ucxx/constructors.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,87 +32,87 @@ class RequestTagMulti;
class Worker;

// Components
std::shared_ptr<Address> createAddressFromWorker(std::shared_ptr<Worker> worker);
[[nodiscard]] std::shared_ptr<Address> createAddressFromWorker(std::shared_ptr<Worker> worker);

std::shared_ptr<Address> createAddressFromString(std::string addressString);
[[nodiscard]] std::shared_ptr<Address> createAddressFromString(std::string addressString);

std::shared_ptr<Context> createContext(const ConfigMap ucxConfig, const uint64_t featureFlags);
[[nodiscard]] std::shared_ptr<Context> createContext(const ConfigMap ucxConfig,
const uint64_t featureFlags);

std::shared_ptr<Endpoint> createEndpointFromHostname(std::shared_ptr<Worker> worker,
std::string ipAddress,
uint16_t port,
bool endpointErrorHandling);
[[nodiscard]] std::shared_ptr<Endpoint> createEndpointFromHostname(std::shared_ptr<Worker> worker,
std::string ipAddress,
uint16_t port,
bool endpointErrorHandling);

std::shared_ptr<Endpoint> createEndpointFromConnRequest(std::shared_ptr<Listener> listener,
ucp_conn_request_h connRequest,
bool endpointErrorHandling);
[[nodiscard]] std::shared_ptr<Endpoint> createEndpointFromConnRequest(
std::shared_ptr<Listener> listener, ucp_conn_request_h connRequest, bool endpointErrorHandling);

std::shared_ptr<Endpoint> createEndpointFromWorkerAddress(std::shared_ptr<Worker> worker,
std::shared_ptr<Address> address,
bool endpointErrorHandling);
[[nodiscard]] std::shared_ptr<Endpoint> createEndpointFromWorkerAddress(
std::shared_ptr<Worker> worker, std::shared_ptr<Address> address, bool endpointErrorHandling);

std::shared_ptr<Listener> createListener(std::shared_ptr<Worker> worker,
uint16_t port,
ucp_listener_conn_callback_t callback,
void* callbackArgs);
[[nodiscard]] std::shared_ptr<Listener> createListener(std::shared_ptr<Worker> worker,
uint16_t port,
ucp_listener_conn_callback_t callback,
void* callbackArgs);

std::shared_ptr<Worker> createWorker(std::shared_ptr<Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);
[[nodiscard]] std::shared_ptr<Worker> createWorker(std::shared_ptr<Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);

std::shared_ptr<MemoryHandle> createMemoryHandle(
[[nodiscard]] std::shared_ptr<MemoryHandle> createMemoryHandle(
std::shared_ptr<Context> context,
const size_t size,
void* buffer = nullptr,
const ucs_memory_type_t memoryType = UCS_MEMORY_TYPE_HOST);

std::shared_ptr<RemoteKey> createRemoteKeyFromMemoryHandle(
[[nodiscard]] std::shared_ptr<RemoteKey> createRemoteKeyFromMemoryHandle(
std::shared_ptr<MemoryHandle> memoryHandle);

std::shared_ptr<RemoteKey> createRemoteKeyFromSerialized(std::shared_ptr<Endpoint> endpoint,
SerializedRemoteKey serializedRemoteKey);
[[nodiscard]] std::shared_ptr<RemoteKey> createRemoteKeyFromSerialized(
std::shared_ptr<Endpoint> endpoint, SerializedRemoteKey serializedRemoteKey);

// Transfers
std::shared_ptr<RequestAm> createRequestAm(
[[nodiscard]] std::shared_ptr<RequestAm> createRequestAm(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::AmSend, data::AmReceive> requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);

std::shared_ptr<RequestEndpointClose> createRequestEndpointClose(
[[nodiscard]] std::shared_ptr<RequestEndpointClose> createRequestEndpointClose(
std::shared_ptr<Endpoint> endpoint,
const data::EndpointClose requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);

std::shared_ptr<RequestFlush> createRequestFlush(std::shared_ptr<Component> endpointOrWorker,
const data::Flush requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);
[[nodiscard]] std::shared_ptr<RequestFlush> createRequestFlush(
std::shared_ptr<Component> endpointOrWorker,
const data::Flush requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);

std::shared_ptr<RequestStream> createRequestStream(
[[nodiscard]] std::shared_ptr<RequestStream> createRequestStream(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::StreamSend, data::StreamReceive> requestData,
const bool enablePythonFuture);

std::shared_ptr<RequestTag> createRequestTag(
[[nodiscard]] std::shared_ptr<RequestTag> createRequestTag(
std::shared_ptr<Component> endpointOrWorker,
const std::variant<data::TagSend, data::TagReceive> requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);

std::shared_ptr<RequestMem> createRequestMem(
[[nodiscard]] std::shared_ptr<RequestMem> createRequestMem(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::MemPut, data::MemGet> requestData,
const bool enablePythonFuture,
RequestCallbackUserFunction callbackFunction,
RequestCallbackUserData callbackData);

std::shared_ptr<RequestTagMulti> createRequestTagMulti(
[[nodiscard]] std::shared_ptr<RequestTagMulti> createRequestTagMulti(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::TagMultiSend, data::TagMultiReceive> requestData,
const bool enablePythonFuture);
Expand Down
3 changes: 1 addition & 2 deletions cpp/include/ucxx/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ class Context : public Component {
* @param[in] featureFlags feature flags to be used at UCP context construction time.
* @return The `shared_ptr<ucxx::Context>` object
*/
[[nodiscard]] friend std::shared_ptr<Context> createContext(ConfigMap ucxConfig,
const uint64_t featureFlags);
friend std::shared_ptr<Context> createContext(ConfigMap ucxConfig, const uint64_t featureFlags);

/**
* @brief `ucxx::Context` destructor
Expand Down
19 changes: 10 additions & 9 deletions cpp/include/ucxx/endpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,10 @@ class Endpoint : public Component {
*
* @returns The `shared_ptr<ucxx::Endpoint>` object
*/
[[nodiscard]] friend std::shared_ptr<Endpoint> createEndpointFromHostname(
std::shared_ptr<Worker> worker,
std::string ipAddress,
uint16_t port,
bool endpointErrorHandling);
friend std::shared_ptr<Endpoint> createEndpointFromHostname(std::shared_ptr<Worker> worker,
std::string ipAddress,
uint16_t port,
bool endpointErrorHandling);

/**
* @brief Constructor for `shared_ptr<ucxx::Endpoint>`.
Expand All @@ -185,8 +184,9 @@ class Endpoint : public Component {
*
* @returns The `shared_ptr<ucxx::Endpoint>` object
*/
[[nodiscard]] friend std::shared_ptr<Endpoint> createEndpointFromConnRequest(
std::shared_ptr<Listener> listener, ucp_conn_request_h connRequest, bool endpointErrorHandling);
friend std::shared_ptr<Endpoint> createEndpointFromConnRequest(std::shared_ptr<Listener> listener,
ucp_conn_request_h connRequest,
bool endpointErrorHandling);

/**
* @brief Constructor for `shared_ptr<ucxx::Endpoint>`.
Expand All @@ -207,8 +207,9 @@ class Endpoint : public Component {
*
* @returns The `shared_ptr<ucxx::Endpoint>` object
*/
[[nodiscard]] friend std::shared_ptr<Endpoint> createEndpointFromWorkerAddress(
std::shared_ptr<Worker> worker, std::shared_ptr<Address> address, bool endpointErrorHandling);
friend std::shared_ptr<Endpoint> createEndpointFromWorkerAddress(std::shared_ptr<Worker> worker,
std::shared_ptr<Address> address,
bool endpointErrorHandling);

/**
* @brief Get the underlying `ucp_ep_h` handle.
Expand Down
9 changes: 4 additions & 5 deletions cpp/include/ucxx/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,10 @@ class Listener : public Component {
*
* @returns The `shared_ptr<ucxx::Listener>` object.
*/
[[nodiscard]] friend std::shared_ptr<Listener> createListener(
std::shared_ptr<Worker> worker,
uint16_t port,
ucp_listener_conn_callback_t callback,
void* callbackArgs);
friend std::shared_ptr<Listener> createListener(std::shared_ptr<Worker> worker,
uint16_t port,
ucp_listener_conn_callback_t callback,
void* callbackArgs);

/**
* @brief Constructor for `shared_ptr<ucxx::Endpoint>`.
Expand Down
9 changes: 4 additions & 5 deletions cpp/include/ucxx/memory_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ class MemoryHandle : public Component {
*
* @returns The `shared_ptr<ucxx::MemoryHandle>` object
*/
[[nodiscard]] friend std::shared_ptr<MemoryHandle> createMemoryHandle(
std::shared_ptr<Context> context,
const size_t size,
void* buffer,
const ucs_memory_type_t memoryType);
friend std::shared_ptr<MemoryHandle> createMemoryHandle(std::shared_ptr<Context> context,
const size_t size,
void* buffer,
const ucs_memory_type_t memoryType);

~MemoryHandle();

Expand Down
4 changes: 2 additions & 2 deletions cpp/include/ucxx/remote_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class RemoteKey : public Component {
*
* @returns The `shared_ptr<ucxx::RemoteKey>` object
*/
[[nodiscard]] friend std::shared_ptr<RemoteKey> createRemoteKeyFromMemoryHandle(
friend std::shared_ptr<RemoteKey> createRemoteKeyFromMemoryHandle(
std::shared_ptr<MemoryHandle> memoryHandle);

/**
Expand Down Expand Up @@ -140,7 +140,7 @@ class RemoteKey : public Component {
*
* @returns The `shared_ptr<ucxx::RemoteKey>` object
*/
[[nodiscard]] friend std::shared_ptr<RemoteKey> createRemoteKeyFromSerialized(
friend std::shared_ptr<RemoteKey> createRemoteKeyFromSerialized(
std::shared_ptr<Endpoint> endpoint, SerializedRemoteKey serializedRemoteKey);

~RemoteKey();
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_am.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class RequestAm : public Request {
*
* @returns The `shared_ptr<ucxx::RequestAm>` object
*/
[[nodiscard]] friend std::shared_ptr<RequestAm> createRequestAm(
friend std::shared_ptr<RequestAm> createRequestAm(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::AmSend, data::AmReceive> requestData,
const bool enablePythonFuture,
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_endpoint_close.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class RequestEndpointClose : public Request {
*
* @returns The `shared_ptr<ucxx::RequestEndpointClose>` object.
*/
[[nodiscard]] friend std::shared_ptr<RequestEndpointClose> createRequestEndpointClose(
friend std::shared_ptr<RequestEndpointClose> createRequestEndpointClose(
std::shared_ptr<Endpoint> endpoint,
const data::EndpointClose requestData,
const bool enablePythonFuture,
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_flush.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class RequestFlush : public Request {
*
* @returns The `shared_ptr<ucxx::RequestFlush>` object
*/
[[nodiscard]] friend std::shared_ptr<RequestFlush> createRequestFlush(
friend std::shared_ptr<RequestFlush> createRequestFlush(
std::shared_ptr<Component> endpointOrWorker,
const data::Flush requestData,
const bool enablePythonFuture,
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class RequestMem : public Request {
*
* @returns The `shared_ptr<ucxx::RequestTag>` object
*/
[[nodiscard]] friend std::shared_ptr<RequestMem> createRequestMem(
friend std::shared_ptr<RequestMem> createRequestMem(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::MemPut, data::MemGet> requestData,
const bool enablePythonFuture,
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class RequestStream : public Request {
*
* @returns The `shared_ptr<ucxx::RequestStream>` object
*/
[[nodiscard]] friend std::shared_ptr<RequestStream> createRequestStream(
friend std::shared_ptr<RequestStream> createRequestStream(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::StreamSend, data::StreamReceive> requestData,
const bool enablePythonFuture);
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_tag.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class RequestTag : public Request {
*
* @returns The `shared_ptr<ucxx::RequestTag>` object
*/
[[nodiscard]] friend std::shared_ptr<RequestTag> createRequestTag(
friend std::shared_ptr<RequestTag> createRequestTag(
std::shared_ptr<Component> endpointOrWorker,
const std::variant<data::TagSend, data::TagReceive> requestData,
const bool enablePythonFuture,
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/ucxx/request_tag_multi.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ class RequestTagMulti : public Request {
*
* @returns Request to be subsequently checked for the completion and its state.
*/
[[nodiscard]] friend std::shared_ptr<RequestTagMulti> createRequestTagMulti(
friend std::shared_ptr<RequestTagMulti> createRequestTagMulti(
std::shared_ptr<Endpoint> endpoint,
const std::variant<data::TagMultiSend, data::TagMultiReceive> requestData,
const bool enablePythonFuture);
Expand Down
6 changes: 3 additions & 3 deletions cpp/include/ucxx/worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ class Worker : public Component {
* `ucxx::Request`, currently used only by `ucxx::python::Worker`.
* @returns The `shared_ptr<ucxx::Worker>` object
*/
[[nodiscard]] friend std::shared_ptr<Worker> createWorker(std::shared_ptr<Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);
friend std::shared_ptr<Worker> createWorker(std::shared_ptr<Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);

/**
* @brief `ucxx::Worker` destructor.
Expand Down
13 changes: 7 additions & 6 deletions cpp/python/include/ucxx/python/constructors.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ class Worker;

namespace python {

std::shared_ptr<::ucxx::Future> createFuture(std::shared_ptr<::ucxx::Notifier> notifier);
[[nodiscard]] std::shared_ptr<::ucxx::Future> createFuture(
std::shared_ptr<::ucxx::Notifier> notifier);

std::shared_ptr<::ucxx::Future> createFutureWithEventLoop(
[[nodiscard]] std::shared_ptr<::ucxx::Future> createFutureWithEventLoop(
PyObject* asyncioEventLoop, std::shared_ptr<::ucxx::Notifier> notifier);

std::shared_ptr<::ucxx::Notifier> createNotifier();
[[nodiscard]] std::shared_ptr<::ucxx::Notifier> createNotifier();

std::shared_ptr<::ucxx::Worker> createWorker(std::shared_ptr<ucxx::Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);
[[nodiscard]] std::shared_ptr<::ucxx::Worker> createWorker(std::shared_ptr<ucxx::Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);

} // namespace python

Expand Down
2 changes: 1 addition & 1 deletion cpp/python/include/ucxx/python/notifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class Notifier : public ::ucxx::Notifier {
*
* @returns The `shared_ptr<ucxx::python::Notifier>` object
*/
[[nodiscard]] friend std::shared_ptr<::ucxx::Notifier> createNotifier();
friend std::shared_ptr<::ucxx::Notifier> createNotifier();

/**
* @brief Virtual destructor.
Expand Down
5 changes: 2 additions & 3 deletions cpp/python/include/ucxx/python/python_future.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ class Future : public ::ucxx::Future {
*
* @returns The `shared_ptr<ucxx::python::Worker>` object
*/
[[nodiscard]] friend std::shared_ptr<::ucxx::Future> createFuture(
std::shared_ptr<::ucxx::Notifier> notifier);
friend std::shared_ptr<::ucxx::Future> createFuture(std::shared_ptr<::ucxx::Notifier> notifier);

/**
* @brief Constructor of `shared_ptr<ucxx::python::Future>`.
Expand All @@ -81,7 +80,7 @@ class Future : public ::ucxx::Future {
*
* @returns The `shared_ptr<ucxx::python::Worker>` object
*/
[[nodiscard]] friend std::shared_ptr<::ucxx::Future> createFutureWithEventLoop(
friend std::shared_ptr<::ucxx::Future> createFutureWithEventLoop(
PyObject* asyncioEventLoop, std::shared_ptr<::ucxx::Notifier> notifier);

/**
Expand Down
5 changes: 3 additions & 2 deletions cpp/python/include/ucxx/python/worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ class Worker : public ::ucxx::Worker {
*
* @returns The `shared_ptr<ucxx::python::Worker>` object
*/
[[nodiscard]] friend std::shared_ptr<::ucxx::Worker> createWorker(
std::shared_ptr<Context> context, const bool enableDelayedSubmission, const bool enableFuture);
friend std::shared_ptr<::ucxx::Worker> createWorker(std::shared_ptr<Context> context,
const bool enableDelayedSubmission,
const bool enableFuture);

/**
* @brief Populate the Python futures pool.
Expand Down

0 comments on commit 16eaa57

Please sign in to comment.