Skip to content

Fix support for Pi Pico and C11 #50206

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

Merged
merged 1 commit into from
Jan 28, 2023
Merged

Conversation

ThadHouse
Copy link
Contributor

@ThadHouse ThadHouse commented Sep 14, 2022

Currently if C11 or higher is enabled, many files fail to compile because they include assert.h and config_autogen.h, which define conflicting copies of static_assert. Disable the one in the hal if the other is found.

Closes #54190

Signed-off-by: Thad House [email protected]

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Technically, static_assert is only available in C++98C++11 and above, and it is a keyword (i.e. not a macro) in C23, but encoding all these details here would be an overkill given that both C++98 and C23 are very niche use cases.

In the long term, we want to detect these compiler features during the configuration step in the build system and expose them to the code (e.g. COMPILER_HAS_STATIC_ASSERT)

Comment on lines 25 to 26
#ifndef __cplusplus
#ifndef static_assert
Copy link
Member

Choose a reason for hiding this comment

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

nested ifdefs should be avoided when possible.

Suggested change
#ifndef __cplusplus
#ifndef static_assert
#if !defined(__cplusplus) && !defined(static_assert)

@yonsch
Copy link
Collaborator

yonsch commented Nov 13, 2022

@ThadHouse Could you rebase and address the change request?

@klaasdannen
Copy link

@ThadHouse are you still working on this PR?

@ThadHouse
Copy link
Contributor Author

Sorry, I've been busy. I'll work on getting it updated this weekend.

@henrikbrixandersen henrikbrixandersen removed their request for review January 3, 2023 13:57
@yonsch
Copy link
Collaborator

yonsch commented Jan 3, 2023

@ThadHouse Ping?

Currently if C11 or higher is enabled, many files fail to compile because
static_assert is defined in multiple header files. Solve this by
disabling the one in the HAL if the other macro is found

Signed-off-by: Thad House <[email protected]>
@ThadHouse
Copy link
Contributor Author

Sorry its taken so long, I was swapped with work. Just pushed against the newest main.

@cfriedt
Copy link
Member

cfriedt commented Jan 28, 2023

It would be really great if there were some information on how to reproduce the problem. From the way it's phrased, it really sounds like a bug, and probably should have been filed as one with form fields filled-in.

https://github.com/zephyrproject-rtos/zephyr/issues/new?assignees=&labels=bug&template=bug_report.md&title=

@ThadHouse
Copy link
Contributor Author

Ok. Created one

#54190

@stephanosio stephanosio changed the title Add support for Pi Pico and C11 Fix support for Pi Pico and C11 Jan 28, 2023
@stephanosio stephanosio added the bug The issue is a bug, or the PR is fixing a bug label Jan 28, 2023
@stephanosio stephanosio added this to the v3.3.0 milestone Jan 28, 2023
@cfriedt cfriedt merged commit 92e518e into zephyrproject-rtos:main Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug manifest-hal_rpi_pico
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RP2040 cannot be compiled with C11 enabled
6 participants