Skip to content

Commit

Permalink
pw_thread: Add explicit public default ctor for stl::Options
Browse files Browse the repository at this point in the history
Without the explicit default public constructor c++20-based compiler
rejects `stl::Options{}` initialization and allows only `stl::Options()`
one.
See this example for the reference: https://godbolt.org/z/K5GWaGsTf.

This CL also prevents to create `thread::Options` using `{}` syntax
with explicitly defining the empty constructor instead of the default
one.
See this example for the reference: https://godbolt.org/z/EP7hTooo8.

Change-Id: I2749198c0f8134f57320d8f91515c45d252bd089
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/98101
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Dennis Kormalev <denk@google.com>
  • Loading branch information
dkormalev authored and CQ Bot Account committed Jun 20, 2022
1 parent 672047a commit 2bc2d9b
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 11 deletions.
7 changes: 7 additions & 0 deletions pw_thread/docs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,13 @@ Options must not contain any memory needed for a thread to run (TCB,
stack, etc.). The Options may be deleted or re-used immediately after
starting a thread.

Options subclass must contain non-default explicit constructor (parametrized or
not), e.g. ``constexpr Options() {}``. It is not enough to have them as
``= default`` ones, because C++17 considers subclasses like ``stl::Options`` as
aggregate classes if they have a default constructor and requires base class
constructor to be public (which is not the case for the ``thread::Options``) for
``Options{}`` syntax.

Please see the thread creation backend documentation for how their Options work.

Portable Thread Creation
Expand Down
4 changes: 3 additions & 1 deletion pw_thread/public/pw_thread/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ namespace pw::thread {
// starting a thread.
class Options {
protected:
constexpr Options() = default;
// We can't use `= default` here, because it allows to create an Options
// instance in C++17 with `pw::thread::Options{}` syntax.
constexpr Options() {}
};

// The class Thread can represent a single thread of execution. Threads allow
Expand Down
6 changes: 3 additions & 3 deletions pw_thread_embos/public/pw_thread_embos/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ namespace pw::thread::embos {
//
class Options : public thread::Options {
public:
constexpr Options() = default;
constexpr Options(const Options&) = default;
constexpr Options(Options&& other) = default;
constexpr Options() {}
constexpr Options(const Options&) {}
constexpr Options(Options&&) {}

// Sets the name for the embOS task, this is optional.
// Note that this will be deep copied into the context and may be truncated
Expand Down
6 changes: 3 additions & 3 deletions pw_thread_freertos/public/pw_thread_freertos/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ namespace pw::thread::freertos {
//
class Options : public thread::Options {
public:
constexpr Options() = default;
constexpr Options(const Options&) = default;
constexpr Options(Options&& other) = default;
constexpr Options() {}
constexpr Options(const Options&) {}
constexpr Options(Options&&) {}

// Sets the name for the FreeRTOS task, note that this will be truncated
// based on configMAX_TASK_NAME_LEN.
Expand Down
5 changes: 4 additions & 1 deletion pw_thread_stl/public/pw_thread_stl/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace pw::thread::stl {
// Instead, users are expected to start the thread and after dynamically adjust
// the thread's attributes using std::thread::native_handle based on the native
// threading APIs.
class Options : public thread::Options {};
class Options : public thread::Options {
public:
constexpr Options() {}
};

} // namespace pw::thread::stl
6 changes: 3 additions & 3 deletions pw_thread_threadx/public/pw_thread_threadx/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ namespace pw::thread::threadx {
//
class Options : public thread::Options {
public:
constexpr Options() = default;
constexpr Options(const Options&) = default;
constexpr Options(Options&& other) = default;
constexpr Options() {}
constexpr Options(const Options&) {}
constexpr Options(Options&&) {}

// Sets the name for the ThreadX thread, note that this will be deep copied
// into the context and may be truncated based on
Expand Down

0 comments on commit 2bc2d9b

Please sign in to comment.