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

lib: os: cbprintf: Add C++ support for static packaging #34697

Merged
merged 2 commits into from
May 5, 2021

Conversation

nordic-krch
Copy link
Contributor

Static packaging was using _Generic C11 keyword which is not supported by C++.
#34516 forced always runtime packaging in C++ which results in slower packaging (finally logging) from C++ files.

_Generic can be implemented in C++ using function overloading and template. Extended cbprintf_package test to be run for c++ (main.c content copied to test.inc).

@github-actions github-actions bot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test labels Apr 29, 2021
@nordic-krch nordic-krch force-pushed the cbprintf_package_cpp branch from 86af0f6 to 758175c Compare April 29, 2021 12:29
}

/* C++ version for long double detection. */
static inline bool z_cbprintf_cxx_is_longdouble(long double arg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this return bool when the equivalent for pchar returns int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is_pchar is used in FOREACH loop to calculate number of char pointers in the argument list. This one is used for assert. I can change that to int for consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that makes sense, since the C version appears to evaluate to 0 or 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zephyrbot zephyrbot added the area: Base OS Base OS Library (lib/os) label Apr 29, 2021
@zephyrbot zephyrbot requested a review from andyross April 29, 2021 13:48
Added C++ replacements for macros that are using _Generic
keyword.

Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Extended test suite to be run for C++

Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
@nordic-krch nordic-krch force-pushed the cbprintf_package_cpp branch from 758175c to 5c85454 Compare April 30, 2021 05:29
@pabigot pabigot self-requested a review April 30, 2021 11:39
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

I don't see any particular problems with this, but I also haven't looked closely nor have I tried any of this, so I'm reluctant to casually approve it. I would rather see this reviewed/approved by people who are invested in C++ Zephyr, such as @alexanderwachter.

@alexanderwachter
Copy link
Member

Ping @Thurnheer

@nashif nashif merged commit 61a2e8c into zephyrproject-rtos:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Base OS Base OS Library (lib/os) area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants