Skip to content

include/macro.h: fix REVERSE_ARG/FOREACH_ARG for empty args in C++ - #20023

Open
xiaoxiang781216 wants to merge 1 commit into
apache:masterfrom
xiaoxiang781216:upstream-macro-cpp
Open

include/macro.h: fix REVERSE_ARG/FOREACH_ARG for empty args in C++#20023
xiaoxiang781216 wants to merge 1 commit into
apache:masterfrom
xiaoxiang781216:upstream-macro-cpp

Conversation

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

Summary

  • C++ strict mode (ISO C++11 and above) drops the GNU , ##__VA_ARGS__ comma elision extension, so GET_ARG_COUNT() returns 1 instead of 0 for the zero-argument case, which makes REVERSE_ARG() and FOREACH_ARG() misselect their dispatch entry with zero varargs.
  • Centralize the empty-argument handling in GET_ARG_COUNT (via __VA_OPT__ for C++) and make REVERSE_ARG and FOREACH_ARG dispatch through CONCATENATE(prefix, GET_ARG_COUNT(...)).
  • Remove the two duplicated 33-entry selector lists (REVERSE_ARG_ and FOREACH_ARG_) as a side benefit.

Impact

  • Only include/nuttx/macro.h is touched; no new configuration options.
  • The C path is unchanged (still uses , ##__VA_ARGS__); the C++ path now uses standard __VA_OPT__ (available since C++20, and as an extension in GNU C++ modes of GCC/Clang).
  • Users of these macros: include/nuttx/sched_note.h (NOTE_PRINTF_TYPES, NOTE_PRINTF_TAG), which is exercised by sched_note_printf() when CONFIG_DRIVERS_NOTE_STRIP_FORMAT is enabled.

Testing

Built and booted sim:nsh with this change:

NuttShell (NSH) NuttX-10.4.0
nsh> uname -a
NuttX 10.4.0 f64dedbe01e Aug 31 2026 17:10:36 sim sim
nsh> echo hello
hello
nsh> ps
  TID   PID  PPID PRI POLICY   TYPE    NPX STATE    EVENT     SIGMASK STACK COMMAND
    0     0     0   0 FIFO     Kthread   - Ready              0000000000000000 0069584 Idle_Task
    ...

Standalone macro test comparing the old and the new header (the NOTE_PRINTF_TAG pattern from sched_note.h):

$ g++ -std=c++20 test.cpp   # old header
tag0=268435456               # GET_ARG_COUNT() == 1, wrong
tag1=268435457

$ g++ -std=c++20 test.cpp   # new header
tag0=0                       # GET_ARG_COUNT() == 0, correct
tag1=268435457

The C behavior is identical before and after the change (gcc -std=gnu11 gives tag0=0 with both headers).

C++ strict mode drops the GNU ", ##__VA_ARGS__" comma elision, so
GET_ARG_COUNT() and REVERSE_ARG() misselect their dispatch entry with
zero varargs. Centralize the empty-argument handling in GET_ARG_COUNT
(via __VA_OPT__ for C++) and make REVERSE_ARG and FOREACH_ARG dispatch
through CONCATENATE(prefix, GET_ARG_COUNT(...)), removing the two
duplicated 33-entry selector lists.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@github-actions github-actions Bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Aug 31, 2026
@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@acassis

acassis commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

@xiaoxiang781216 and about this PR:
#19882

I just asked him to submit the loadelfdemo to let us have a way to test it and he close the PR without any explanation.

If they don't want to fix PRs, add documentation or test to validate the implementation, why are they submitting PRs?

It is not fair for those who are here (like you and me) trying to make NuttX better.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor Author

@xiaoxiang781216 and about this PR: #19882

I just asked him to submit the loadelfdemo to let us have a way to test it and he close the PR without any explanation.

If they don't want to fix PRs, add documentation or test to validate the implementation, why are they submitting PRs?

It is not fair for those who are here (like you and me) trying to make NuttX better.

the fix isn't good, and need more refine then upstream again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components issues Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants