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

Add nd_type_p macro #5091

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Add nd_type_p macro #5091

merged 3 commits into from
Dec 3, 2021

Conversation

S-H-GAMELINKS
Copy link
Contributor

MRI has these code using nd_type macro to check NODE type.

static NODE *
rest_arg_append(struct parser_params *p, NODE *args, NODE *rest_arg, const YYLTYPE *loc)
{
    NODE *n1;
    if ((nd_type(rest_arg) == NODE_LIST) && (n1 = splat_array(args)) != 0) {
	return list_concat(n1, rest_arg);
    }
    return arg_concat(p, args, rest_arg, loc);
}

However, conditions will be long and sometimes difficult to read, I think.

So, I think better to introduce nd_type_p macro like this.

#define nd_type_p(n, t) (nd_type((n)) == (t))

I think that conditions can be more shorter to using nd_type_p macro.
How about it?

static NODE *
rest_arg_append(struct parser_params *p, NODE *args, NODE *rest_arg, const YYLTYPE *loc)
{
    NODE *n1;
    if ((nd_type_p(rest_arg, NODE_LIST)) && (n1 = splat_array(args)) != 0) {
	return list_concat(n1, rest_arg);
    }
    return arg_concat(p, args, rest_arg, loc);
}

parse.y Outdated Show resolved Hide resolved
parse.y Outdated Show resolved Hide resolved
node.c Outdated Show resolved Hide resolved
ast.c Outdated Show resolved Hide resolved
@nobu
Copy link
Member

nobu commented Nov 8, 2021

Another nd_type() == NODE_... is in a comment at compile.c:1784.

@S-H-GAMELINKS
Copy link
Contributor Author

@nobu
Thank you for your review.
I fixed code that you review pointed out in this pull request.

@S-H-GAMELINKS
Copy link
Contributor Author

Rebased to latest master(a84dc9d) branch.

@nobu nobu merged commit ec7f14d into ruby:master Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants