Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

device.h: C++20 compatibility #70403

Merged
merged 4 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions include/zephyr/device.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
Expand Down Expand Up @@ -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_) \
Expand All @@ -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) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd to find this macro defined after it was "used" above. Not strictly required, of course.

Copy link
Collaborator Author

@marc-hb marc-hb Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This places the macro closer to the two locations where it's used.

It's also a lower-level implementation detail, so I think it it reads better last - that's subjective of course.

EDIT: I wrote too fast, I was referring to Z_DEVICE_INIT_ENTRY_DEV(). Then I wanted to preserve the "top-down" order for the next macro.

I think it's "odd" only if you're tainted by C compilation which requires "forward" declarations. Maybe dating all the way back to when compilation was performed in a single pass? Funny how the pre-processor is in some respect more advanced that C itself... https://stackoverflow.com/questions/3136686/is-the-c99-preprocessor-turing-complete

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.
*
Expand Down
4 changes: 2 additions & 2 deletions include/zephyr/pm/device.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, \
ceolin marked this conversation as resolved.
Show resolved Hide resolved
Z_PM_DEVICE_POWER_DOMAIN_INIT(node_id) \
}

Expand Down
23 changes: 23 additions & 0 deletions tests/lib/cpp/cxx/app.overlay
Original file line number Diff line number Diff line change
@@ -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;
};
};
11 changes: 11 additions & 0 deletions tests/lib/cpp/cxx/dts/bindings/test-device.yaml
Original file line number Diff line number Diff line change
@@ -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"
14 changes: 14 additions & 0 deletions tests/lib/cpp/cxx/prj.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 16 additions & 0 deletions tests/lib/cpp/cxx/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include <zephyr/init.h>
#include <zephyr/device.h>
#include <zephyr/pm/device.h>
#include <zephyr/kernel.h>
#include <zephyr/net/buf.h>
#include <zephyr/sys/crc.h>
Expand Down Expand Up @@ -136,3 +137,18 @@ 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);

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);
Loading