From bd3f5955876ebf392ae782a84ef2ea5342a07546 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 19 Mar 2024 04:23:09 +0000 Subject: [PATCH 1/4] device.h: conditionally remove init braces for C++20 compatibility Conditionally remove braces for designated initializers of anonymous unions. This makes it compatible with C++20 while not breaking pre-C11 gcc. This does for device.h what commit c15f029a7108 ("init.h: restore designated initializers in SYS_INIT_NAMED()") did for init.h See https://docs.zephyrproject.org/latest/develop/languages/cpp/ and long discussion in #69411 for more obscure C/C++ compatibility details. Signed-off-by: Marc Herbert --- include/zephyr/device.h | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/include/zephyr/device.h b/include/zephyr/device.h index 2862c0befd38b4..e8bbc09a2990f2 100644 --- a/include/zephyr/device.h +++ b/include/zephyr/device.h @@ -1020,13 +1020,24 @@ device_get_dt_nodelabels(const struct device *dev) .state = (state_), \ .data = (data_), \ IF_ENABLED(CONFIG_DEVICE_DEPS, (.deps = (deps_),)) /**/ \ - IF_ENABLED(CONFIG_PM_DEVICE, ({ .pm_base = (pm_),},)) /**/ \ + IF_ENABLED(CONFIG_PM_DEVICE, Z_DEVICE_INIT_PM_BASE(pm_)) /**/ \ IF_ENABLED(CONFIG_DEVICE_DT_METADATA, \ (IF_ENABLED(DT_NODE_EXISTS(node_id_), \ (.dt_meta = &Z_DEVICE_DT_METADATA_NAME_GET( \ dev_id_),)))) \ } +/* + * Anonymous unions require C11. Some pre-C11 gcc versions have early support for anonymous + * unions but they require these braces when combined with C99 designated initializers. For + * more details see https://docs.zephyrproject.org/latest/develop/languages/cpp/ + */ +#if defined(__STDC_VERSION__) && (__STDC_VERSION__) < 201100 +# define Z_DEVICE_INIT_PM_BASE(pm_) ({ .pm_base = (pm_),},) +#else +# define Z_DEVICE_INIT_PM_BASE(pm_) (.pm_base = (pm_),) +#endif + /** * @brief Device section name (used for sorting purposes). * @@ -1098,10 +1109,7 @@ device_get_dt_nodelabels(const struct device *dev) Z_INIT_ENTRY_NAME(DEVICE_NAME_GET(dev_id)) = { \ .init_fn = {COND_CODE_1(Z_DEVICE_IS_MUTABLE(node_id), (.dev_rw), (.dev)) = \ (init_fn_)}, \ - { \ - COND_CODE_1(Z_DEVICE_IS_MUTABLE(node_id), (.dev_rw), (.dev)) = \ - &DEVICE_NAME_GET(dev_id), \ - }, \ + Z_DEVICE_INIT_ENTRY_DEV(node_id, dev_id), \ } #define Z_DEFER_DEVICE_INIT_ENTRY_DEFINE(node_id, dev_id, init_fn_) \ @@ -1110,12 +1118,23 @@ device_get_dt_nodelabels(const struct device *dev) Z_INIT_ENTRY_NAME(DEVICE_NAME_GET(dev_id)) = { \ .init_fn = {COND_CODE_1(Z_DEVICE_IS_MUTABLE(node_id), (.dev_rw), (.dev)) = \ (init_fn_)}, \ - { \ - COND_CODE_1(Z_DEVICE_IS_MUTABLE(node_id), (.dev_rw), (.dev)) = \ - &DEVICE_NAME_GET(dev_id), \ - }, \ + Z_DEVICE_INIT_ENTRY_DEV(node_id, dev_id), \ } +/* + * Anonymous unions require C11. Some pre-C11 gcc versions have early support for anonymous + * unions but they require these braces when combined with C99 designated initializers. For + * more details see https://docs.zephyrproject.org/latest/develop/languages/cpp/ + */ +#if defined(__STDC_VERSION__) && (__STDC_VERSION__) < 201100 +# define Z_DEVICE_INIT_ENTRY_DEV(node_id, dev_id) { Z_DEV_ENTRY_DEV(node_id, dev_id) } +#else +# define Z_DEVICE_INIT_ENTRY_DEV(node_id, dev_id) Z_DEV_ENTRY_DEV(node_id, dev_id) +#endif + +#define Z_DEV_ENTRY_DEV(node_id, dev_id) \ + COND_CODE_1(Z_DEVICE_IS_MUTABLE(node_id), (.dev_rw), (.dev)) = &DEVICE_NAME_GET(dev_id) + /** * @brief Define a @ref device and all other required objects. * From 7902ce8b18776938cb6140d4793c03e0c00f3cff Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 19 Mar 2024 22:07:23 +0000 Subject: [PATCH 2/4] tests: cpp: add some C++ coverage for device.h / Device Tree This provides C++ build test coverage for device.h (notably: Z_DEVICE_INIT() fixed in the previous commit) and for some other Device Tree macros. Signed-off-by: Marc Herbert --- tests/lib/cpp/cxx/app.overlay | 23 +++++++++++++++++++ .../lib/cpp/cxx/dts/bindings/test-device.yaml | 11 +++++++++ tests/lib/cpp/cxx/src/main.cpp | 11 +++++++++ 3 files changed, 45 insertions(+) create mode 100644 tests/lib/cpp/cxx/app.overlay create mode 100644 tests/lib/cpp/cxx/dts/bindings/test-device.yaml diff --git a/tests/lib/cpp/cxx/app.overlay b/tests/lib/cpp/cxx/app.overlay new file mode 100644 index 00000000000000..69b87b4c7ac752 --- /dev/null +++ b/tests/lib/cpp/cxx/app.overlay @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2024 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + * + * Application overlay for creating a fake device instance we can use to + * test. Subset of tests/kernel/device/app.overlay and other app.overlay + * files. + */ + +/ { + test_dev0_boot: dev0 { + compatible = "fake-cpp-driver"; + status = "okay"; + }; + + test_dev1_dfr: dev1 { + compatible = "fake-cpp-driver"; + status = "okay"; + + zephyr,deferred-init; + }; +}; diff --git a/tests/lib/cpp/cxx/dts/bindings/test-device.yaml b/tests/lib/cpp/cxx/dts/bindings/test-device.yaml new file mode 100644 index 00000000000000..154c9b54f82131 --- /dev/null +++ b/tests/lib/cpp/cxx/dts/bindings/test-device.yaml @@ -0,0 +1,11 @@ +# Copyright (c) 2024, Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +include: [base.yaml] + +description: | + When this file is missing _or_ the include above is missing, then + test coverage is _quietly_ reduced! Code that should fail turns + green. Nasty. + +compatible: "fake-cpp-driver" diff --git a/tests/lib/cpp/cxx/src/main.cpp b/tests/lib/cpp/cxx/src/main.cpp index b8b77c6c26a5a7..a8a49a3a254090 100644 --- a/tests/lib/cpp/cxx/src/main.cpp +++ b/tests/lib/cpp/cxx/src/main.cpp @@ -136,3 +136,14 @@ ZTEST(cxx_tests, test_new_delete) delete test_foo; } ZTEST_SUITE(cxx_tests, NULL, NULL, NULL, NULL, NULL); + +/* + * Unused macros are parsed but not actually compiled. So, even with all their NULL + * arguments these one-liners add significant C++ coverage. For instance this actually + * compiles some of the macros in zephyr/device.h in C++. + * + * DEVICE_DEFINE(dev_id, name, * init_fn, pm, data, config, level, prio, api) + */ +DEVICE_DT_DEFINE(DT_NODELABEL(test_dev0_boot), NULL, NULL, NULL, NULL, POST_KERNEL, 33, NULL); + +DEVICE_DT_DEFINE(DT_NODELABEL(test_dev1_dfr), NULL, NULL, NULL, NULL, POST_KERNEL, 33, NULL); From f9de7f8f927ab6b3c5e39e3b8f8a83ab85bb48ec Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 19 Mar 2024 04:35:39 +0000 Subject: [PATCH 3/4] pm: device.h: fix pm_device designated initializers order for C++ Fixes the following error appearing in the test coverage added by the next commit: In file included from zephyr/tests/lib/cpp/cxx/src/main.cpp:18: ``` zephyr/include/zephyr/pm/device.h:275:9: error: designator order for field 'pm_device_base::state' does not match declaration order in 'pm_device_base' 275 | } | ^ zephyr/include/zephyr/pm/device.h:315:17: note: in expansion of macro 'Z_PM_DEVICE_INIT' 315 | Z_PM_DEVICE_INIT(Z_PM_DEVICE_NAME(dev_id), node_id, ``` Note this failure is observed with any g++ -std=c++NN standard value - even before C++20. Signed-off-by: Marc Herbert --- include/zephyr/pm/device.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/zephyr/pm/device.h b/include/zephyr/pm/device.h index ce0e53d7446f05..5d3b4a96cea7bc 100644 --- a/include/zephyr/pm/device.h +++ b/include/zephyr/pm/device.h @@ -242,9 +242,9 @@ BUILD_ASSERT(offsetof(struct pm_device_isr, base) == 0); */ #define Z_PM_DEVICE_BASE_INIT(obj, node_id, pm_action_cb, _flags) \ { \ - .action_cb = pm_action_cb, \ - .state = PM_DEVICE_STATE_ACTIVE, \ .flags = ATOMIC_INIT(Z_PM_DEVICE_FLAGS(node_id) | (_flags)), \ + .state = PM_DEVICE_STATE_ACTIVE, \ + .action_cb = pm_action_cb, \ Z_PM_DEVICE_POWER_DOMAIN_INIT(node_id) \ } From efa14aeb18b1f3da713cf5226a0df609a8b43ba2 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 19 Mar 2024 19:04:52 +0000 Subject: [PATCH 4/4] tests: cpp: add C++ coverage for pm/device.h Note CONFIG_PM_* affects device.h too. Even if not compiled, the more code the pre-processor sees the better. Signed-off-by: Marc Herbert --- tests/lib/cpp/cxx/prj.conf | 14 ++++++++++++++ tests/lib/cpp/cxx/src/main.cpp | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/tests/lib/cpp/cxx/prj.conf b/tests/lib/cpp/cxx/prj.conf index 15ae9ce1ebd490..22fcfc3b59afa5 100644 --- a/tests/lib/cpp/cxx/prj.conf +++ b/tests/lib/cpp/cxx/prj.conf @@ -5,8 +5,22 @@ CONFIG_ZTEST_STACK_SIZE=2048 CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=128 CONFIG_CRC=y +# Enable optional features that are off by default to cover as much as +# possible of the same .h files. Ideally, kconfiglib.py would support +# "randconfig" because turning on some options can also reduce test +# coverage in some areas. But: 1. combinatorial explosion, 2. it does +# not. + # RTIO CONFIG_RTIO=y CONFIG_RTIO_SUBMIT_SEM=y CONFIG_RTIO_CONSUME_SEM=y CONFIG_RTIO_SYS_MEM_BLOCKS=y + +# Enable more features and C++ coverage in device.h, pm/device.h, pm.h... +CONFIG_PM=y +CONFIG_PM_DEVICE=y +CONFIG_PM_DEVICE_RUNTIME=y +CONFIG_DEVICE_DT_METADATA=y +# Deprecated but on by default! Silence the warning: +CONFIG_PM_DEVICE_RUNTIME_EXCLUSIVE=n diff --git a/tests/lib/cpp/cxx/src/main.cpp b/tests/lib/cpp/cxx/src/main.cpp index a8a49a3a254090..8b149290bab657 100644 --- a/tests/lib/cpp/cxx/src/main.cpp +++ b/tests/lib/cpp/cxx/src/main.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -147,3 +148,7 @@ ZTEST_SUITE(cxx_tests, NULL, NULL, NULL, NULL, NULL); DEVICE_DT_DEFINE(DT_NODELABEL(test_dev0_boot), NULL, NULL, NULL, NULL, POST_KERNEL, 33, NULL); DEVICE_DT_DEFINE(DT_NODELABEL(test_dev1_dfr), NULL, NULL, NULL, NULL, POST_KERNEL, 33, NULL); + +static int fake_pm_action(const struct device *dev, + enum pm_device_action pm_action) { return -1; } +PM_DEVICE_DT_DEFINE(DT_NODELABEL(test_dev0_boot), fake_pm_action);