pw_thread: Work around C++17 aggregate initialization issue for Options

Use `explicit` instead of a non-default constructor to prevent aggregate
initialization of the `pw::thread::Options` base class. This simplifies
derived classes since they can use the default constructor. Add negative
compilation tests for pw::thread::Options initialization.

Change-Id: I50cf9fbdfdc31b7ddf40c2dbe4182b37b4e61176
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/232019
Reviewed-by: Carlos Chinchilla <[email protected]>
Commit-Queue: Wyatt Hepler <[email protected]>
Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
Lint: Lint 🤖 <[email protected]>

pw_thread/BUILD.bazel

@@ -337,6 +337,15 @@ pw_cc_test(
     ],
 )
 
+pw_cc_test(
+    name = "options_test",
+    srcs = ["options_test.cc"],
+    deps = [
+        ":options",
+        "//pw_compilation_testing:negative_compilation_testing",
+    ],
+)
+
 pw_cc_test(
     name = "sleep_facade_test",
     srcs = [

pw_thread/BUILD.gn

@@ -178,6 +178,7 @@ pw_test_group("tests") {
     ":yield_facade_test",
     ":test_thread_context_facade_test",
     ":thread_snapshot_service_test",
+    ":options_test",
   ]
 }
 
@@ -253,6 +254,12 @@ pw_source_set("thread_facade_test") {
   ]
 }
 
+pw_test("options_test") {
+  sources = [ "options_test.cc" ]
+  deps = [ ":options" ]
+  negative_compilation_tests = true
+}
+
 pw_test("thread_info_test") {
   sources = [ "thread_info_test.cc" ]
   deps = [

pw_thread/docs.rst

@@ -252,13 +252,6 @@ 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

pw_thread/options_test.cc

@@ -0,0 +1,29 @@
+// Copyright 2024 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+#include "pw_thread/options.h"
+
+#include "pw_compilation_testing/negative_compilation.h"
+
+namespace {
+
+#if PW_NC_TEST(ThreadOptions_CannotConstruct)
+PW_NC_EXPECT("protected");
+[[maybe_unused]] pw::thread::Options options;
+#elif PW_NC_TEST(ThreadOptions_CannotConstructAsAggregate)
+PW_NC_EXPECT("protected");
+[[maybe_unused]] pw::thread::Options options{};
+#endif  // PW_NC_TEST
+
+}  // namespace

pw_thread/public/pw_thread/options.h

@@ -39,9 +39,9 @@ namespace pw::thread {
 /// starting a thread.
 class Options {
  protected:
-  // We can't use `= default` here, because it allows to create an Options
-  // instance in C++17 with `pw::thread::Options{}` syntax.
-  constexpr Options() {}
+  // Must be explicit to prevent direct instantiation in C++17 with
+  // `pw::thread::Options{}` syntax.
+  explicit constexpr Options() = default;
 };
 
 }  // namespace pw::thread

pw_thread_embos/public/pw_thread_embos/options.h

@@ -44,7 +44,7 @@ class Context;
 //
 class Options : public thread::Options {
  public:
-  constexpr Options() {}
+  constexpr Options() = default;
   constexpr Options(const Options&) = default;
   constexpr Options(Options&&) = default;
 

pw_thread_freertos/public/pw_thread_freertos/options.h

@@ -44,7 +44,7 @@ class Context;
 //
 class Options : public thread::Options {
  public:
-  constexpr Options() {}
+  constexpr Options() = default;
   constexpr Options(const Options&) = default;
   constexpr Options(Options&&) = default;
 

pw_thread_stl/public/pw_thread_stl/options.h

@@ -24,7 +24,7 @@ namespace pw::thread::stl {
 // threading APIs.
 class Options : public thread::Options {
  public:
-  constexpr Options() {}
+  constexpr Options() = default;
 };
 
 }  // namespace pw::thread::stl

pw_thread_threadx/public/pw_thread_threadx/options.h

@@ -50,7 +50,7 @@ class Context;
 //
 class Options : public thread::Options {
  public:
-  constexpr Options() {}
+  constexpr Options() = default;
   constexpr Options(const Options&) = default;
   constexpr Options(Options&&) = default;