Skip to content

Conversation

@finikorg
Copy link
Contributor

@finikorg finikorg commented Feb 8, 2019

Added tests for UTIL_LISTIFY and corrected test name.

@zephyrbot
Copy link

zephyrbot commented Feb 8, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #13174 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13174      +/-   ##
==========================================
+ Coverage   48.75%   48.76%   +0.01%     
==========================================
  Files         319      319              
  Lines       46871    46876       +5     
  Branches    10835    10840       +5     
==========================================
+ Hits        22850    22861      +11     
+ Misses      19394    19387       -7     
- Partials     4627     4628       +1
Impacted Files Coverage Δ
drivers/clock_control/nrf_power_clock.c 50% <0%> (-2.11%) ⬇️
boards/posix/nrf52_bsim/argparse.c 46.98% <0%> (+13.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acaae57...0ca9590. Read the comment docs.

Copy link
Contributor

@SebastianBoe SebastianBoe left a comment

Choose a reason for hiding this comment

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

An AFAICT equally[0] powerful mechanism is already present.

To reduce the amount of infrastructure, I believe it would be better to re-use what we have.

4dc9f86

[0] Actually, more flexible, as it doesn't require ';' to separate each element and is therefore more re-usable than LOOP.

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

Seems really clever. Random nitpikery.

@nordic-krch
Copy link
Contributor

nordic-krch commented Feb 11, 2019

there is UTIL_LISTIFY macro. It looks that it's doing the same. Though, i must say that name is intuitive since it narrows down the use case.

@finikorg
Copy link
Contributor Author

An AFAICT equally[0] powerful mechanism is already present.

To reduce the amount of infrastructure, I believe it would be better to re-use what we have.

4dc9f86

[0] Actually, more flexible, as it doesn't require ';' to separate each element and is therefore more re-usable than LOOP.

Thanks for notice, I will try to use UTIL_LISTIFY, could not understand what it does by description.

@SebastianBoe
Copy link
Contributor

could not understand what it does by description.

Please do post a PR if you can find a better description.

Trivial syntax fix.

Signed-off-by: Andrei Emeltchenko <[email protected]>
@finikorg
Copy link
Contributor Author

could not understand what it does by description.

Please do post a PR if you can find a better description.

I started to use UTIL_LISTIFY but it requires to add ; and I have warnings

@SebastianBoe
Copy link
Contributor

but it requires to add ;``

Yes it does, but this is an acceptable price to pay to not have to maintain two macros that nearly do the same thing.

LOOP()
LOOP_WITHOUT_SEMICOLON()

What kind of warnings?

@finikorg finikorg changed the title util: Add LOOP macro for loop with incremented arguments util: Add UTIL_LISTIFY test cases Feb 11, 2019
@finikorg
Copy link
Contributor Author

finikorg commented Feb 11, 2019

What kind of warnings?

Check warnings in this PR.

@SebastianBoe
Copy link
Contributor

SebastianBoe commented Feb 11, 2019

Check warnings in this PR.

Not sure how, "Details" doesn't give any more details.

Anyhow, these macro's often violate the static analyzer's rules, I believe a maintainer usually ignores the rule and merges it anyway.

EDIT: Maybe it is this: #13174 (comment)

@SebastianBoe
Copy link
Contributor

This error seems reasonable to me:

-:13: ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop

do { i += x; } while(0);

@finikorg
Copy link
Contributor Author

This error seems reasonable to me:

-:13: ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop

do { i += x; } while(0);

-:10: WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO: Single statement macros should not use a do {} while (0) loop
#10: FILE: tests/misc/util/src/main.c:63:
+#define INC(x, _) do { i += x; } while (0);

Add tests for UTIL_LISTIFY macro.

Signed-off-by: Andrei Emeltchenko <[email protected]>
@SebastianBoe
Copy link
Contributor

Can't catch a break ...

I don't know how to resolve the warnings.

@nashif
Copy link
Member

nashif commented Feb 11, 2019

@SebastianBoe

Not sure how, "Details" doesn't give any more details.

find first comment by zephyrbot, browse through the history of the comment.

@nashif nashif merged commit a4e950a into zephyrproject-rtos:master Feb 11, 2019
@finikorg finikorg deleted the macro branch February 13, 2019 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants