| Message ID | 20200214113320.GA31578@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) | expand |
On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote: > With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM, > see the next patch. > > Note that MOD_TOPLEVEL can be set even if struct/union/enum type is > private and bind_symbol() is not called. I don't like that very much. For example: why this is needed for struct/union/enum and not other types? Should it be possible to use the function toplevel() or add and helper for it in scope.c? > IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check" > doesn't show any difference. Yes, it's true and it shouldn't make any difference but still I would prefer to not mix symbols and types more than they already are. -- Luc
On 02/17, Luc Van Oostenryck wrote: > > On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote: > > With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM, > > see the next patch. > > > > Note that MOD_TOPLEVEL can be set even if struct/union/enum type is > > private and bind_symbol() is not called. > > I don't like that very much. For example: why this is needed for > struct/union/enum and not other types? Do you mean builtin types like int_ctype? OK, I agree, this is slightly inconsistent. > Should it be possible to use the function toplevel() or add and > helper for it in scope.c? Well, toplevel() won't work if SYM_STRUCT/etc is anonymous, in this case bind_symbol() is not called and thus sym->scope = NULL. Consider struct { int m; } x; void func(void) { struct { int m; } x; } we want to report the 2nd struct definition as "local" 1:8 def s :x 1:14 def m :x.m int 1:19 def v x struct :x 3:6 def f func void ( ... ) 5:16 func def . s :x 5:22 func def . m :x.m int 5:27 func def . v x struct :x so that this spam can be filtered out, but base->scope is NULL in both cases. > > IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check" > > doesn't show any difference. > > Yes, it's true and it shouldn't make any difference but still I > would prefer to not mix symbols and types more than they already are. OK, will you agree with one-liner below? This should make toplevel() work. Oleg. --- a/parse.c +++ b/parse.c @@ -772,6 +772,7 @@ static struct token *struct_union_enum_specifier(enum type type, } sym = alloc_symbol(token->pos, type); + sym->scope = block_scope; token = parse(token->next, sym); ctx->ctype.base_type = sym; token = expect(token, '}', "at end of specifier");
On Tue, Feb 18, 2020 at 11:38:38AM +0100, Oleg Nesterov wrote: > On 02/17, Luc Van Oostenryck wrote: > > > > On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote: > > > With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM, > > > see the next patch. > > > > > > Note that MOD_TOPLEVEL can be set even if struct/union/enum type is > > > private and bind_symbol() is not called. > > > > I don't like that very much. For example: why this is needed for > > struct/union/enum and not other types? > > Do you mean builtin types like int_ctype? OK, I agree, this is slightly > inconsistent. I was thinking of the other constructed types: arrays & pointers. > > Should it be possible to use the function toplevel() or add and > > helper for it in scope.c? > > Well, toplevel() won't work if SYM_STRUCT/etc is anonymous, in this > case bind_symbol() is not called and thus sym->scope = NULL. > > Consider > > struct { int m; } x; > > void func(void) > { > struct { int m; } x; > > } > > we want to report the 2nd struct definition as "local" > > 1:8 def s :x > 1:14 def m :x.m int > 1:19 def v x struct :x > 3:6 def f func void ( ... ) > 5:16 func def . s :x > 5:22 func def . m :x.m int > 5:27 func def . v x struct :x > > so that this spam can be filtered out, but base->scope is NULL in both > cases. OK, I see. > > > IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check" > > > doesn't show any difference. > > > > Yes, it's true and it shouldn't make any difference but still I > > would prefer to not mix symbols and types more than they already are. > > OK, will you agree with one-liner below? This should make toplevel() work. This will still be inconsistent with the other types but I can live with this. If you could just add a comment explaining why it is needed and using an helper instead of directly using 'block_scope' (like set_[current_]scope() or something). -- Luc
diff --git a/parse.c b/parse.c index a08165a..4492586 100644 --- a/parse.c +++ b/parse.c @@ -741,6 +741,8 @@ static struct token *struct_union_enum_specifier(enum type type, // symbol being redefined. sym = alloc_symbol(token->pos, type); bind_symbol(sym, token->ident, NS_STRUCT); + if (toplevel(sym->scope)) + sym->ctype.modifiers |= MOD_TOPLEVEL; } if (sym->type != type) error_die(token->pos, "invalid tag applied to %s", show_typename (sym)); @@ -772,6 +774,8 @@ static struct token *struct_union_enum_specifier(enum type type, } sym = alloc_symbol(token->pos, type); + if (toplevel(block_scope)) + sym->ctype.modifiers |= MOD_TOPLEVEL; token = parse(token->next, sym); ctx->ctype.base_type = sym; token = expect(token, '}', "at end of specifier");
With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM, see the next patch. Note that MOD_TOPLEVEL can be set even if struct/union/enum type is private and bind_symbol() is not called. IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check" doesn't show any difference. Signed-off-by: Oleg Nesterov <oleg@redhat.com> --- parse.c | 4 ++++ 1 file changed, 4 insertions(+)