Skip to content

preproc: varargscount: bugfix for tokens starting with '(' not being counted #543

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.

Sign up for GitHub

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
lgirdwood merged 2 commits into from
Nov 8, 2018
Merged

preproc: varargscount: bugfix for tokens starting with '(' not being counted #543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87dad4a
preproc: META_COUNT_VARARGS_BEFORE_COMPILE: bugfix for tokens startin…
mwierzbix Nov 6, 2018
f092d90
Merge branch 'master' into debugability-macro-metaprog-bugfix-varargc…
mwierzbix Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/include/sof/preproc-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@
* META_COUNT_VARAGS_BEFORE_COMPILE(A,B,C,D) evaluates to 4
*/
#define _META_PP_NARG_BEFORE_COMPILE_(...) \
META_DEC(_META_PP_ARG_N(__VA_ARGS__))
_META_PP_ARG_N(__VA_ARGS__)

#define _META_PP_ARG_N(\
_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, \
_11, _12, _13, _14, _15, _16, _17, _18, _19, _20, \
Expand Down Expand Up @@ -224,7 +225,7 @@
/* used by macro MAP, don't use on its own */
#define _META_MAP_BODY(arg_count, m, ...)\
META_IF_ELSE(META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__))(\
META_DEFER(2, _META_MAP)()\
_META_DEFER_2(_META_MAP)()\
(arg_count, m, __VA_ARGS__)\
)()

Expand All @@ -246,7 +247,7 @@
/* used by macro MAP_AGGREGATE, don't use on its own */
#define _META_MAP_AGGREGATE_BODY(arg_count, m, aggr, ...)\
META_IF_ELSE(META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__))(\
META_DEFER(2, _META_MAP_AGGREGATE)()\
_META_DEFER_2(_META_MAP_AGGREGATE)()\
(arg_count, m, aggr, __VA_ARGS__)\
)(aggr)

Expand Down
6 changes: 5 additions & 1 deletion src/include/sof/preproc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@
* META_COUNT_VARAGS_BEFORE_COMPILE(A,B,C,D) evaluates to 4
*/
#define META_COUNT_VARAGS_BEFORE_COMPILE(...)\
_META_PP_NARG_BEFORE_COMPILE_(_, ##__VA_ARGS__, _META_PP_RSEQ_N())
META_DEC(\
_META_PP_NARG_BEFORE_COMPILE_(\
_, ##__VA_ARGS__, _META_PP_RSEQ_N()\
)\
)

/* treat x as string while forcing x expansion beforehand */
#define META_QUOTE(x) _META_QUOTE(x)
Expand Down
32 changes: 17 additions & 15 deletions test/cmocka/src/debugability/macros.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,20 @@
static void test_debugability_macros_declare_log_entry(void **state)
{
const char *macro_result = CAPTURE(_DECLARE_LOG_ENTRY(
LOG_LEVEL_CRITICAL, "Message", TRACE_CLASS_DMA, 1));
LOG_LEVEL_CRITICAL,
"Message",
TRACE_CLASS_DMA,
1
));
const char *should_be_eq =
"__attribute__((section(\".static_log.\""
" \"LOG_LEVEL_CRITICAL\"))) "
"static const struct { uint32_t level; uint32_t component_id; "
"uint32_t params_num; uint32_t line_idx; uint32_t file_name_len; "
"const char file_name[sizeof(\"src/debugability/macros.c\")]; "
"uint32_t text_len; const char text[sizeof(\"Message\")]; } "
"log_entry = { 1";
"log_entry = { 1(6 << 24)152sizeof(\"src/debugability/macros.c\")"
"\"src/debugability/macros.c\"sizeof(\"Message\")\"Message\" }";
(void)state;

assert_string_equal(macro_result, should_be_eq);
Expand All @@ -78,32 +83,29 @@ static char *get_should_be(const int param_count)
{
char *result = malloc(sizeof(char) * 1024);
char *paramlist = get_param_list(param_count);
char *maybe_space = " ";
char *maybe_comma = ",";

if (!param_count)
maybe_space = "";
else
if (param_count)
maybe_comma = "";

/* which format: 0 1 2 3 4 5 6 7 8 9 A*/
sprintf(result, "%s%d%s%d%s%s%d%s%s%s%s",
/* which format: 0 1 2 3 4 5 6 7 8 9*/
sprintf(result, "%s%d%s%d%s%d%s%s%s%s",
/*0*/"{ __attribute__((unused)) typedef char assertion_failed_"
META_QUOTE(BASE_LOG_ASSERT_FAIL_MSG)
"[(",
/*1*/_TRACE_EVENT_MAX_ARGUMENT_COUNT,
/*2*/" >= ",
/*3*/param_count,
/*4*/maybe_space,
/*5*/") ? 1 : -1]; log_func log_function = (log_func)& _trace_event",
/*6*/param_count,
/*7*/"; log_function(&log_entry",
/*8*/maybe_comma,
/*9*/paramlist,
/*A*/");}"
/*4*/") ? 1 : -1]; log_func log_function = (log_func)& _trace_event",
/*5*/param_count,
/*6*/"; log_function(&log_entry",
/*7*/maybe_comma,
/*8*/paramlist,
/*9*/");}"
);
if (paramlist)
free(paramlist);
// TODO: maybe remove all whitespace chars; they're not important here
Copy link
Contributor Author

@mwierzbix mwierzbix Nov 6, 2018

Choose a reason for hiding this comment

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

I am thinking about removing all whitespaces for this check in the future, as it really doesn't matter whether preprocessor gives us
uint32_t param0 , uint32_t... or
uint32_t param0,uint32_t...
as the compiler doesn't care.

Again, I'd rather not do this in bugfix PR.

return result;
}

Expand Down
38 changes: 26 additions & 12 deletions test/cmocka/src/lib/preproc/varargs_count.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,35 +51,49 @@ static void META_CONCAT_SEQ_DELIM_(prefix, test_func, postfix)(void **state)\
#define A 1
#define B 2
#define C 3
#define PARENTHESIS_PRE() (1+3)/2
#define PARENTHESIS_POST() 4/(3-1)

#define META_NAME META_COUNT_VARAGS_BEFORE_COMPILE
TEST_HERE_DECLARE(META_NAME, 0, 0, )
TEST_HERE_DECLARE(META_NAME, 1, 1, A )
TEST_HERE_DECLARE(META_NAME, 3, 3, A, B, C)
TEST_HERE_DECLARE(PP_NARG , 0, 0, )
TEST_HERE_DECLARE(PP_NARG , 1, 1, A )
TEST_HERE_DECLARE(PP_NARG , 3, 3, A, B, C)
#define TEST_HERE_DECLARE_GROUP(func_name)\
TEST_HERE_DECLARE(func_name, 0, 0, )\
TEST_HERE_DECLARE(func_name, 1, 1, A )\
TEST_HERE_DECLARE(func_name, 3, 3, A, B, C)\
TEST_HERE_DECLARE(func_name, with_parenthesis_pre, 1,\
Copy link
Contributor Author

@mwierzbix mwierzbix Nov 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

New unit tests:

#define PARENTHESIS_PRE()  (1+3)/2
#define PARENTHESIS_POST() 4/(3-1)
...
TEST_HERE_DECLARE(func_name, with_parenthesis_pre,  1, PARENTHESIS_PRE())
TEST_HERE_DECLARE(func_name, with_parenthesis_post, 1,PARENTHESIS_POST())

PARENTHESIS_PRE())\
TEST_HERE_DECLARE(func_name, with_parenthesis_post, 1,\
PARENTHESIS_POST())

TEST_HERE_DECLARE_GROUP(META_NAME)
TEST_HERE_DECLARE_GROUP(PP_NARG)

#undef TEST_HERE_DECLARE_GROUP
#undef PARENTHESIS_POST
#undef PARENTHESIS_PRE
#undef C
#undef B
#undef A
#undef TEST_FUNC

#define TEST_HERE_USE_GROUP(func_name)\
TEST_HERE_USE(func_name, 0),\
TEST_HERE_USE(func_name, 1),\
TEST_HERE_USE(func_name, 3),\
TEST_HERE_USE(func_name, with_parenthesis_pre),\
TEST_HERE_USE(func_name, with_parenthesis_post),

int main(void)
{
const struct CMUnitTest tests[] = {
TEST_HERE_USE(META_NAME, 0),
TEST_HERE_USE(META_NAME, 1),
TEST_HERE_USE(META_NAME, 3),
TEST_HERE_USE(PP_NARG , 0),
TEST_HERE_USE(PP_NARG , 1),
TEST_HERE_USE(PP_NARG , 3),
TEST_HERE_USE_GROUP(META_NAME)
TEST_HERE_USE_GROUP(PP_NARG)
};

cmocka_set_message_output(CM_OUTPUT_TAP);

return cmocka_run_group_tests(tests, NULL, NULL);
}

#undef TEST_HERE_USE_GROUP
#undef META_NAME
#undef TEST_PREFIX