Skip to content

Commit

Permalink
Added some [[nodiscard]], coro lifetime attributes
Browse files Browse the repository at this point in the history
- Added [[nodiscard]] to Coroutine, Error, Result, etc.
- static_ASYNC, virtual_ASYNC macros no longer necessary
- Added coro_return_type_ etc. but they aren't implemented in the
  version of Clang in Xcode 16
  • Loading branch information
snej committed Aug 4, 2024
1 parent 5bc44d3 commit e644245
Show file tree
Hide file tree
Showing 27 changed files with 76 additions and 57 deletions.
4 changes: 2 additions & 2 deletions include/crouton/CoCondition.hh
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ namespace crouton {
Blocker supports only one waiting coroutine. If you need more, use a CoCondition. */
template <typename T>
class Blocker : public BlockerBase {
class [[nodiscard]] Blocker : public BlockerBase {
public:
T await_resume() noexcept {
assert(_state == Ready);
Expand All @@ -116,7 +116,7 @@ namespace crouton {


template <>
class Blocker<void> : public BlockerBase {
class [[nodiscard]] Blocker<void> : public BlockerBase {
public:
void notify() {BlockerBase::notify();}
};
Expand Down
4 changes: 2 additions & 2 deletions include/crouton/Coroutine.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace crouton {
/** Base class for the public object returned by a coroutine function.
Most of the implementation is usually in the associated CoroutineImpl subclass (IMPL). */
template <class IMPL>
class Coroutine {
class [[nodiscard]] coro_return_type_ Coroutine {
public:
// movable, but not copyable.
Coroutine(Coroutine&& c) noexcept :_handle(c._handle) {c._handle = {};}
Expand Down Expand Up @@ -94,7 +94,7 @@ namespace crouton {



class CoroutineImplBase {
class coro_wrapper_ CoroutineImplBase {
public:
CoroutineImplBase() = default;
~CoroutineImplBase();
Expand Down
8 changes: 2 additions & 6 deletions include/crouton/CroutonFwd.hh
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,8 @@
To be #include'd in header files instead of Crouton.hh, when possible. */


/// Macros for declaring a function that returns a Future, e.g. `ASYNC<void> close();`
/// It's surprisingly easy to forget to await the Future, especially `Future<void>`,
/// hence the `[[nodiscard]]` annotation.
#define ASYNC [[nodiscard("Future must be AWAITed or returned")]] ::crouton::Future
#define staticASYNC [[nodiscard("Future must be AWAITed or returned")]] static ::crouton::Future
#define virtualASYNC [[nodiscard("Future must be AWAITed or returned")]] virtual ::crouton::Future
/// Marker macro for declaring a function that returns a Future, e.g. `ASYNC<void> close();`
#define ASYNC ::crouton::Future


namespace crouton {
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/Error.hh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace crouton {

/** Error holds an error code as a type-erased enum value, of any type that matches `ErrorDomain`.
There's also a default empty state. */
class Error {
class [[nodiscard("check for error")]] Error {
public:
/// The default no-error value. Available as the constant `noerror`.
constexpr Error() = default;
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/EventLoop.hh
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ namespace crouton {
static void after(double delaySecs, std::function<void()> fn);

/// Static method that returns a Future that completes after the given delay.
staticASYNC<void> sleep(double delaySecs);
static ASYNC<void> sleep(double delaySecs);

private:
friend class EventLoop;
Expand Down
3 changes: 2 additions & 1 deletion include/crouton/Future.hh
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ namespace crouton {
A regular function that gets a Future can call `then()` to register a callback. */
template <typename T>
class Future : public Coroutine<FutureImpl<T>>, public ISelectable {
class [[nodiscard("Future must be AWAITed or returned")]]
Future : public Coroutine<FutureImpl<T>>, public ISelectable {
public:
using nonvoidT = std::conditional_t<std::is_void_v<T>, std::byte, T>;

Expand Down
2 changes: 1 addition & 1 deletion include/crouton/Generator.hh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace crouton {
A Generator can also be iterated synchronously via its begin/end methods,
or with a `for(:)` loop.*/
template <typename T>
class Generator : public Coroutine<GeneratorImpl<T>>, public ISeries<T> {
class [[nodiscard]] Generator : public Coroutine<GeneratorImpl<T>>, public ISeries<T> {
public:
Generator(Generator&&) noexcept = default;
Generator& operator=(Generator&&) = default;
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/PubSub.hh
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ namespace crouton::ps {

/// Abstract method that handles an item received from the Publisher.
/// @warning You must override either this method or `run`.
virtualASYNC<void> handle(T) {return CroutonError::Unimplemented;}
virtual ASYNC<void> handle(T) {return CroutonError::Unimplemented;}

/// Handles the final Error/noerror item from the publisher.
/// Default implementation sets the `error` property; make sure to call through.
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/Result.hh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace crouton {
(A `Result<void>` has no explicit value, but still distinguishes between empty/nonempty.)
It's commonly used as a return type, and in the implementation of `Future<T>`. */
template <typename T>
class Result {
class [[nodiscard("check Result for error")]] Result {
public:
using TT = std::conditional_t<std::is_void_v<T>, std::monostate, T>;

Expand Down
6 changes: 3 additions & 3 deletions include/crouton/Scheduler.hh
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ namespace crouton {

//==== Awaitable:

class SchedAwaiter {
class SchedAwaiter {
public:
explicit SchedAwaiter(Scheduler* sched) :_sched(sched) { }
bool await_ready() noexcept {return _sched->isCurrent();}
Expand Down Expand Up @@ -138,12 +138,12 @@ namespace crouton {
void resumed(coro_handle h);

/// Returns the coroutine that should be resumed, or else `dflt`.
coro_handle nextOr(coro_handle dflt);
[[nodiscard]] coro_handle nextOr(coro_handle dflt);

/// Adds a coroutine handle to the suspension set.
/// To make it runnable again, call the returned Suspension's `wakeUp` method
/// from any thread.
Suspension suspend(coro_handle h);
[[nodiscard]] Suspension suspend(coro_handle h);

/// Tells the Scheduler a coroutine is about to be destroyed, so it can manage it
/// correctly if it's in the suspended set.
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/io/AddrInfo.hh
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace crouton::io {
#endif

/// Does a DNS lookup of the given hostname, returning an AddrInfo or an error.
staticASYNC<AddrInfo> lookup(string hostname, uint16_t port =0);
static ASYNC<AddrInfo> lookup(string hostname, uint16_t port =0);

/// Returns the primary address, which may be either IPv4 or IPv6.
RawAddress const& primaryAddress() const;
Expand Down
2 changes: 1 addition & 1 deletion include/crouton/io/FileStream.hh
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace crouton::io {

ASYNC<ConstBytes> readNoCopy(size_t maxLen = 65536) override;
ASYNC<ConstBytes> peekNoCopy() override;
virtualASYNC<string> readAll() override;
virtual ASYNC<string> readAll() override;

ASYNC<void> write(ConstBytes) override;
ASYNC<void> write(const ConstBytes buffers[], size_t nBuffers) override;
Expand Down
6 changes: 3 additions & 3 deletions include/crouton/io/ISocket.hh
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,18 @@ namespace crouton::io {
virtual void keepAlive(unsigned intervalSecs) {_binding->keepAlive = intervalSecs;}

/// Opens the socket to the bound address. Resolves once opened.
virtualASYNC<void> open() =0;
virtual ASYNC<void> open() =0;

/// Equivalent to bind + open.
virtualASYNC<void> connect(string const& address, uint16_t port);
virtual ASYNC<void> connect(string const& address, uint16_t port);

/// True if the socket is open/connected.
virtual bool isOpen() const =0;

/// The socket's data stream.
virtual std::shared_ptr<IStream> stream() =0;

virtualASYNC<void> close() =0;
virtual ASYNC<void> close() =0;

virtual ~ISocket() = default;

Expand Down
18 changes: 9 additions & 9 deletions include/crouton/io/IStream.hh
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ namespace crouton::io {
virtual bool isOpen() const =0;

/// Opens the stream, if it wasn't already open; resolves when it's open.
virtualASYNC<void> open() =0;
virtual ASYNC<void> open() =0;

/// Closes the stream; resolves when it's closed.
virtualASYNC<void> close() =0;
virtual ASYNC<void> close() =0;

/// Closes the write side, but not the read side. (Like a socket's `shutdown`.)
virtualASYNC<void> closeWrite() =0;
virtual ASYNC<void> closeWrite() =0;

//---- Reading:

Expand All @@ -53,7 +53,7 @@ namespace crouton::io {
/// @warning The returned buffer belongs to the stream, and is only valid until the next
/// read or close call.
/// @note This is an abstract method that subclasses must implement.
virtualASYNC<ConstBytes> readNoCopy(size_t maxLen = 65536) =0;
virtual ASYNC<ConstBytes> readNoCopy(size_t maxLen = 65536) =0;

/// Returns the next available unread bytes, always at least 1 byte except at EOF.
/// Unlike `readNoCopy`, the returned bytes are _not consumed_ -- they will
Expand All @@ -62,11 +62,11 @@ namespace crouton::io {
/// @warning The returned buffer belongs to the stream, and is only valid until the next
/// read or close call.
/// @note This is an abstract method that subclasses must implement.
virtualASYNC<ConstBytes> peekNoCopy() =0;
virtual ASYNC<ConstBytes> peekNoCopy() =0;

/// Reads `len` bytes, copying into memory starting at `dst` (which must remain valid.)
/// Will always read the full number of bytes unless it hits EOF.
virtualASYNC<size_t> read(MutableBytes buf);
virtual ASYNC<size_t> read(MutableBytes buf);

/// Reads `len` bytes, returning them as a string.
/// Will always read the full number of bytes unless it hits EOF.
Expand All @@ -81,7 +81,7 @@ namespace crouton::io {
ASYNC<string> readUntil(string end, size_t maxLen = SIZE_MAX);

/// Reads until EOF.
virtualASYNC<string> readAll();
virtual ASYNC<string> readAll();

/// Returns a `Generator` that produces chunks of data read from the stream.
/// This is a wrapper around `readNoCopy()` that makes `IStream` satisfy the concept
Expand All @@ -94,7 +94,7 @@ namespace crouton::io {
/// Writes all the bytes.
/// @warning The memory must remain valid until this call completes.
/// @note This is the abstract write method that subclasses must implement.
virtualASYNC<void> write(ConstBytes) =0;
virtual ASYNC<void> write(ConstBytes) =0;

/// Writes data, fully. The string is copied, so the caller doesn't need to keep it.
ASYNC<void> write(string);
Expand All @@ -104,7 +104,7 @@ namespace crouton::io {
/// @note The default implementation makes `nBuffers` calls to `write(ConstBytes)`.
/// A subclass that natively supports multi-buffer write ("writev") may override
/// this method as an optimization.
virtualASYNC<void> write(const ConstBytes buffers[], size_t nBuffers);
virtual ASYNC<void> write(const ConstBytes buffers[], size_t nBuffers);

ASYNC<void> write(std::initializer_list<ConstBytes> buffers);
};
Expand Down
4 changes: 2 additions & 2 deletions include/crouton/io/Stream.hh
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace crouton::io {
/// Closes the write stream, leaving the read stream open until the peer closes it.
ASYNC<void> closeWrite() override;

/// Closes the stream entirely. (Called by the destructor.)
virtual Future<void> close() override;
/// Closes the stream entirely.
virtual ASYNC<void> close() override;

//---- READING

Expand Down
8 changes: 4 additions & 4 deletions include/crouton/io/apple/NWConnection.hh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ namespace crouton::io::apple {
void useTLS(bool tls) {_useTLS = tls;}

/// Opens the socket to the bound address. Resolves once opened.
virtualASYNC<void> open() override;
virtual ASYNC<void> open() override;

bool isOpen() const override {return _isOpen;}

Expand All @@ -60,12 +60,12 @@ namespace crouton::io::apple {
~NWConnection();

private:
virtualASYNC<ConstBytes> readNoCopy(size_t maxLen = 65536) override;
virtualASYNC<ConstBytes> peekNoCopy() override;
virtual ASYNC<ConstBytes> readNoCopy(size_t maxLen = 65536) override;
virtual ASYNC<ConstBytes> peekNoCopy() override;
ASYNC<void> write(ConstBytes b) override;
using IStream::write;

virtualASYNC<ConstBytes> _readNoCopy(size_t maxLen, bool peek);
virtual ASYNC<ConstBytes> _readNoCopy(size_t maxLen, bool peek);
ASYNC<void> _writeOrShutdown(ConstBytes, bool shutdown);
void _close();
void clearReadBuf();
Expand Down
24 changes: 24 additions & 0 deletions include/crouton/util/Base.hh
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,30 @@
#endif


// These will be helpful for detecting bugs, but AppleClang doesn't support them yet as of Xcode 16.
// <https://clang.llvm.org/docs/AttributeReference.html#id488>
#if __has_attribute(coro_return_type)
# define coro_return_type_ [[clang::coro_return_type]]
#else
# define coro_return_type_
#endif
#if __has_attribute(coro_wrapper)
# define coro_wrapper_ [[clang::coro_wrapper]]
#else
# define coro_wrapper_
#endif
#if __has_attribute(coro_lifetimebound)
# define coro_lifetimebound_ [[clang::coro_lifetimebound]]
#else
# define coro_lifetimebound_
#endif
#if __has_attribute(coro_disable_lifetimebound)
# define coro_disable_lifetimebound_ [[clang::coro_disable_lifetimebound]]
#else
# define coro_disable_lifetimebound_
#endif


// Synonyms for coroutine primitives. Optional, but they're more visible in the code.
#define AWAIT co_await
#define YIELD co_yield
Expand Down
4 changes: 2 additions & 2 deletions src/io/blip/Connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ namespace crouton::io::blip {
else
_io.closeSend();
auto& j = _outputTask->join();
AWAIT j;
(void) AWAIT j;
LBLIP->debug("Connection now sending WebSocket CLOSE...");
AWAIT _socket->send(ws::Message{code, message});
auto& j2 = _inputTask->join();
AWAIT j2;
(void) AWAIT j2;
AWAIT _socket->close();
RETURN noerror;
}
Expand Down
2 changes: 1 addition & 1 deletion src/io/uv/Stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace crouton::io {
Stream::Stream() = default;

Stream::~Stream() {
close();
_close();
}


Expand Down
2 changes: 1 addition & 1 deletion src/support/Logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ namespace crouton {
assert_failed_hook = [](const char* message) {
Log->critical(message);
};
Log->info("---------- Welcome to Crouton ----------");
//Log->info("---------- Welcome to Crouton ----------");
});
}

Expand Down
4 changes: 1 addition & 3 deletions tests/ESP32/main/test_esp32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ static void testCodec() {
}


staticASYNC<void> testBLIP() {
static ASYNC<void> testBLIP() {
// Send HTTP request:
auto ws = make_unique<ws::ClientWebSocket>("ws://work.local:4985/travel-sample/_blipsync");
ws->setHeader("Sec-WebSocket-Protocol", "BLIP_3+CBMobile_2");
Expand Down Expand Up @@ -235,5 +235,3 @@ static void initialize() {
esp_log_level_set("example_common", ESP_LOG_WARN);
ESP_ERROR_CHECK(example_connect());
}


4 changes: 2 additions & 2 deletions tests/demo_blipclient.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ using namespace crouton::mini;
/* NOTE to newbies: this file uses some simple/optional macros that are used everywhere in Crouton
to highlight suspend points and use of asynchronous code:
staticASYNC --> [[nodiscard]] static Future
static ASYNC --> [[nodiscard]] static Future
AWAIT --> co_await
RETURN --> co_return
*/


staticASYNC<int> run() {
static ASYNC<int> run() {
// Read flags:
auto args = MainArgs();
string protocol;
Expand Down
4 changes: 2 additions & 2 deletions tests/demo_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ using namespace crouton::io;
/* NOTE to newbies: this file uses some simple/optional macros that are used everywhere in Crouton
to highlight suspend points and use of asynchronous code:
staticASYNC --> [[nodiscard]] static Future
static ASYNC --> [[nodiscard]] static Future
AWAIT --> co_await
RETURN --> co_return
*/


staticASYNC<int> run() {
static ASYNC<int> run() {
// Read flags:
auto args = MainArgs();
bool includeHeaders = false;
Expand Down
Loading

0 comments on commit e644245

Please sign in to comment.