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

init.h: restore designated initializers in SYS_INIT_NAMED() v2 #69486

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Feb 26, 2024

As seen in the PR #68125 and #69411 discussions, commit 19a33c7 ("init: adjust the SYS_INIT dev field init to play nice with older compilers") entirely threw away designated initializers in SYS_INIT_NAMED() to avoid compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two different languages and this is especially true with respect to designated initializers.

Designated initializers provide safer and more readable code, especially in their much stricter C++ version. So use an #ifdef to restore them in SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

@marc-hb marc-hb changed the title init.h: restore designated initializers in SYS_INIT_NAMED() init.h: restore designated initializers in SYS_INIT_NAMED() v2 Feb 26, 2024
@marc-hb
Copy link
Collaborator Author

marc-hb commented Feb 26, 2024

Alternative to #69411 recommended by @keith-packard . Only the #ifdef is different compared to #69411.


Some Github workflows are were timing out:

EDIT: the re-runs passed. All green now.

https://github.com/zephyrproject-rtos/zephyr/actions/runs/8054336494/job/21998974926?pr=69486

This is not specific to this PR, for instance this pure doxygen fix experienced a timeout too:
https://github.com/zephyrproject-rtos/zephyr/actions/runs/8053793552/job/21998901961

https://github.com/zephyrproject-rtos/zephyr/actions/workflows/twister.yaml

EDIT: just found this somewhere:

twister-build (3)
The self-hosted runner: zephyr-runner-linux-x64-4xlarge-deployment-g2tcl-knv45 lost communication with the server. Verify the machine is running and has a healthy network connection. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.

Reported on discord #ci

keith-packard
keith-packard previously approved these changes Feb 26, 2024
Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

Yeah, this should work great. Thanks for the discussion; I learned quite a bit!

cfriedt
cfriedt previously approved these changes Feb 29, 2024
@fabiobaltieri
Copy link
Member

@gmarull ping

Comment on lines 179 to 181
* This Zephyr test compares different C++ -std=... values:
*
* ./scripts/twister -v -b -p qemu_cortex_a53 -T tests/lib/cpp/
Copy link
Member

Choose a reason for hiding this comment

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

please drop CLI snippets from here, maybe just reference the test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

please drop CLI snippets from here, maybe just reference the test.

Done!

Small, pure comment change; @cfriedt can you please re-approve?

As seen in the PR zephyrproject-rtos#68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use an #ifdef to restore them in
SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb dismissed stale reviews from cfriedt and keith-packard via 8a7ef84 March 8, 2024 17:16
@marc-hb marc-hb force-pushed the init-design-init2 branch from 6131ef9 to 8a7ef84 Compare March 8, 2024 17:16
@dleach02 dleach02 merged commit c15f029 into zephyrproject-rtos:main Mar 12, 2024
21 checks passed
@marc-hb marc-hb deleted the init-design-init2 branch March 12, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C++ area: Device Model area: Toolchains Toolchains Enhancement Changes/Updates/Additions to existing features platform: Intel ADSP Intel Audio platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants