| Message ID | 20201226175129.9621-9-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | support __packed struct | expand |
On 26/12/2020 17:51, Luc Van Oostenryck wrote: > apply_ctype()'s argument order is: src, dst (so the reading > direction) but the assignment/memcpy() order is much more used: > dst = src; > memcpy(dst, src, n); > than the order here is confusing. > > So, change its argument order to comply with the memcpy()/ > assignement order and stop the confusion. Hmm, how about: """ apply_ctype()'s argument order is 'src' then 'dst', which reads as copying 'src' to 'dst'. However, assignment, and many library functions (eg. memcpy()), use the opposite order for the source and destination of a copy operation. So, change the argument order of apply_ctype() to mimic the order of memcpy()/assignment, to hopefully reduce any potential confusion. """ Heh, well that is probably not much better! ;-) However, what about the (er...) position of the 'pos' argument? Should it move to the end? ATB, Ramsay Jones > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > parse.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/parse.c b/parse.c > index 402214212d77..f106444f75d8 100644 > --- a/parse.c > +++ b/parse.c > @@ -1043,7 +1043,7 @@ static struct token *enum_specifier(struct token *token, struct symbol *sym, str > return ret; > } > > -static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst); > +static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src); > > static struct token *typeof_specifier(struct token *token, struct symbol *sym, struct decl_state *ctx) > { > @@ -1056,7 +1056,7 @@ static struct token *typeof_specifier(struct token *token, struct symbol *sym, s > struct symbol *sym; > token = typename(token->next, &sym, NULL); > ctx->ctype.base_type = sym->ctype.base_type; > - apply_ctype(token->pos, &sym->ctype, &ctx->ctype); > + apply_ctype(token->pos, &ctx->ctype, &sym->ctype); > } else { > struct symbol *typeof_sym = alloc_symbol(token->pos, SYM_TYPEOF); > token = parse_expression(token->next, &typeof_sym->initializer); > @@ -1427,7 +1427,7 @@ static struct token *generic_qualifier(struct token *next, struct symbol *sym, s > return next; > } > > -static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst) > +static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src) > { > unsigned long mod = src->modifiers; > > @@ -1529,7 +1529,7 @@ static struct token *declaration_specifiers(struct token *token, struct decl_sta > break; > seen |= Set_S | Set_T; > ctx->ctype.base_type = s->ctype.base_type; > - apply_ctype(token->pos, &s->ctype, &ctx->ctype); > + apply_ctype(token->pos, &ctx->ctype, &s->ctype); > token = token->next; > continue; > } >
On Mon, Dec 28, 2020 at 04:47:01PM +0000, Ramsay Jones wrote: > > > On 26/12/2020 17:51, Luc Van Oostenryck wrote: > > apply_ctype()'s argument order is: src, dst (so the reading > > direction) but the assignment/memcpy() order is much more used: > > dst = src; > > memcpy(dst, src, n); > > than the order here is confusing. > > > > So, change its argument order to comply with the memcpy()/ > > assignement order and stop the confusion. > > Hmm, how about: > > """ > apply_ctype()'s argument order is 'src' then 'dst', which reads as > copying 'src' to 'dst'. However, assignment, and many library functions > (eg. memcpy()), use the opposite order for the source and destination > of a copy operation. > > So, change the argument order of apply_ctype() to mimic the order of > memcpy()/assignment, to hopefully reduce any potential confusion. > """ > > Heh, well that is probably not much better! ;-) It's better but I'll try to reformulate it to better express that: *) the 'reading direction' (left to right) vs. the 'assignment direction' (right to left) is a question of API choice *) the left-to-right direction confuses *me* endlessly, but it's just me. -- Luc
diff --git a/parse.c b/parse.c index 402214212d77..f106444f75d8 100644 --- a/parse.c +++ b/parse.c @@ -1043,7 +1043,7 @@ static struct token *enum_specifier(struct token *token, struct symbol *sym, str return ret; } -static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst); +static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src); static struct token *typeof_specifier(struct token *token, struct symbol *sym, struct decl_state *ctx) { @@ -1056,7 +1056,7 @@ static struct token *typeof_specifier(struct token *token, struct symbol *sym, s struct symbol *sym; token = typename(token->next, &sym, NULL); ctx->ctype.base_type = sym->ctype.base_type; - apply_ctype(token->pos, &sym->ctype, &ctx->ctype); + apply_ctype(token->pos, &ctx->ctype, &sym->ctype); } else { struct symbol *typeof_sym = alloc_symbol(token->pos, SYM_TYPEOF); token = parse_expression(token->next, &typeof_sym->initializer); @@ -1427,7 +1427,7 @@ static struct token *generic_qualifier(struct token *next, struct symbol *sym, s return next; } -static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst) +static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src) { unsigned long mod = src->modifiers; @@ -1529,7 +1529,7 @@ static struct token *declaration_specifiers(struct token *token, struct decl_sta break; seen |= Set_S | Set_T; ctx->ctype.base_type = s->ctype.base_type; - apply_ctype(token->pos, &s->ctype, &ctx->ctype); + apply_ctype(token->pos, &ctx->ctype, &s->ctype); token = token->next; continue; }
apply_ctype()'s argument order is: src, dst (so the reading direction) but the assignment/memcpy() order is much more used: dst = src; memcpy(dst, src, n); than the order here is confusing. So, change its argument order to comply with the memcpy()/ assignement order and stop the confusion. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- parse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)