[PATCH 00/10] separate parsing of asm-names from attributes

[PATCH 00/10] separate parsing of asm-names from attributes

[Luc Van Oostenryck]

This series separate the parsing of asm-names from the parsing
of attributes. In itself this has not much value but this series
is part of a larger super-series aiming at rationalizing the
handling of attributes and modifiers.

Luc Van Oostenryck (10):
  use lookup_keyword() for qualifiers
  attribute: split handle_asm_name() from handle_attributes()
  attribute: fold parse_asm_declarator() into handle_asm_name()
  attribute: remove argument 'keywords' from handle_attributes()
  attribute: directly use attribute_specifier() to handle attributes
  attribute: factorize matching of '__attribute__'
  attribute: no need to lookup '__attribute__' in NS_KEYWORD
  testing for SYM_KEYWORD is unneeded for lookup_keyword()
  testing for sym->op is unneeded for lookup_keyword()
  keyword type is a bitmask and must be tested so

 parse.c | 103 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 54 insertions(+), 49 deletions(-)


base-commit: ab77399f33b800f0a1568e512e86df71dd70f566
-- 
2.28.0

[PATCH 01/10] use lookup_keyword() for qualifiers

[Luc Van Oostenryck]

When handling qualifiers, the corresponding symbol is looked-up with
lookup_symbol() then it's checked if the symbol's type is SYM_KEYWORD.

But, only if the identifier is a keyword (struct ident::keyword) can
the symbol be a SYM_KEYWORD. Thus, non-keyword can be filtered-out
early by using lookup_keyword().

So change the call to lookup_symbol() by a call to lookup_keyword().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index b07237eee0bf..19520eaebf38 100644
--- a/parse.c
+++ b/parse.c
@@ -1618,7 +1618,7 @@ struct symbol *ctype_integer(int size, int want_unsigned)
 static struct token *handle_qualifiers(struct token *t, struct decl_state *ctx)
 {
 	while (token_type(t) == TOKEN_IDENT) {
-		struct symbol *s = lookup_symbol(t->ident, NS_TYPEDEF);
+		struct symbol *s = lookup_keyword(t->ident, NS_TYPEDEF);
 		if (!s)
 			break;
 		if (s->type != SYM_KEYWORD)
-- 
2.28.0

[PATCH 02/10] attribute: split handle_asm_name() from handle_attributes()

[Luc Van Oostenryck]

handle_attributes() handles attributes but also the asm names.
These asm names must occur before the attributes and only once
while the attributes may occur multiple time. Also, these asm
names are not allowed everywhere attributes, only in declarations.

It's maybe handy to process both in the same function but it's
also slightly confusing. So, move the handling of the asm names
in a separate function.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/parse.c b/parse.c
index 19520eaebf38..cf897e5d2804 100644
--- a/parse.c
+++ b/parse.c
@@ -1742,6 +1742,20 @@ static struct token *parameter_type_list(struct token *, struct symbol *);
 static struct token *identifier_list(struct token *, struct symbol *);
 static struct token *declarator(struct token *token, struct decl_state *ctx);
 
+static struct token *handle_asm_name(struct token *token, struct decl_state *ctx)
+{
+	struct symbol *keyword;
+
+	if (token_type(token) != TOKEN_IDENT)
+		return token;
+	keyword = lookup_keyword(token->ident, NS_KEYWORD);
+	if (!keyword || keyword->type != SYM_KEYWORD)
+		return token;
+	if (!(keyword->op->type & KW_ASM))
+		return token;
+	return keyword->op->declarator(token->next, ctx);
+}
+
 static struct token *skip_attribute(struct token *token)
 {
 	token = token->next;
@@ -1798,7 +1812,6 @@ static struct token *handle_attributes(struct token *token, struct decl_state *c
 		if (!(keyword->op->type & keywords))
 			break;
 		token = keyword->op->declarator(token->next, ctx);
-		keywords &= KW_ATTRIBUTE;
 	}
 	return token;
 }
@@ -3018,7 +3031,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 
 	saved = ctx.ctype;
 	token = declarator(token, &ctx);
-	token = handle_attributes(token, &ctx, KW_ATTRIBUTE | KW_ASM);
+	token = handle_asm_name(token, &ctx);
+	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
 	apply_modifiers(token->pos, &ctx);
 
 	decl->ctype = ctx.ctype;
@@ -3142,7 +3156,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 		ctx.ctype = saved;
 		token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
 		token = declarator(token, &ctx);
-		token = handle_attributes(token, &ctx, KW_ATTRIBUTE | KW_ASM);
+		token = handle_asm_name(token, &ctx);
+		token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
 		apply_modifiers(token->pos, &ctx);
 		decl->ctype = ctx.ctype;
 		decl->ctype.modifiers |= mod;
-- 
2.28.0

[PATCH 03/10] attribute: fold parse_asm_declarator() into handle_asm_name()

[Luc Van Oostenryck]

An asm name is not really a declarator, it must only be placed
*after* a declarator and is directly handled by handle_asm_name().
It's thus not needed and possibly confusing to treat it like a
generic declarator.

So, fold parse_asm_declarator() into handle_asm_name() and remove it.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/parse.c b/parse.c
index cf897e5d2804..73ec579cfa7f 100644
--- a/parse.c
+++ b/parse.c
@@ -54,7 +54,7 @@ static struct token *handle_attributes(struct token *token, struct decl_state *c
 typedef struct token *declarator_t(struct token *, struct decl_state *);
 static declarator_t
 	struct_specifier, union_specifier, enum_specifier,
-	attribute_specifier, typeof_specifier, parse_asm_declarator,
+	attribute_specifier, typeof_specifier,
 	typedef_specifier, inline_specifier, auto_specifier,
 	register_specifier, static_specifier, extern_specifier,
 	thread_specifier, const_qualifier, volatile_qualifier;
@@ -363,7 +363,6 @@ static struct symbol_op range_op = {
 
 static struct symbol_op asm_op = {
 	.type = KW_ASM,
-	.declarator = parse_asm_declarator,
 	.statement = parse_asm_statement,
 	.toplevel = toplevel_asm_declaration,
 };
@@ -1744,6 +1743,7 @@ static struct token *declarator(struct token *token, struct decl_state *ctx);
 
 static struct token *handle_asm_name(struct token *token, struct decl_state *ctx)
 {
+	struct expression *expr;
 	struct symbol *keyword;
 
 	if (token_type(token) != TOKEN_IDENT)
@@ -1753,7 +1753,12 @@ static struct token *handle_asm_name(struct token *token, struct decl_state *ctx
 		return token;
 	if (!(keyword->op->type & KW_ASM))
 		return token;
-	return keyword->op->declarator(token->next, ctx);
+
+	token = token->next;
+	token = expect(token, '(', "after asm");
+	token = string_expression(token, &expr, "asm name");
+	token = expect(token, ')', "after asm");
+	return token;
 }
 
 static struct token *skip_attribute(struct token *token)
@@ -2181,15 +2186,6 @@ static struct token *parse_asm_statement(struct token *token, struct statement *
 	return expect(token, ';', "at end of asm-statement");
 }
 
-static struct token *parse_asm_declarator(struct token *token, struct decl_state *ctx)
-{
-	struct expression *expr;
-	token = expect(token, '(', "after asm");
-	token = string_expression(token, &expr, "inline asm");
-	token = expect(token, ')', "after asm");
-	return token;
-}
-
 static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
 {
 	struct expression *cond = NULL, *message = NULL;
-- 
2.28.0

[PATCH 04/10] attribute: remove argument 'keywords' from handle_attributes()

[Luc Van Oostenryck]

Now that the asm names are handled in handle_asm(), the 'keywords'
argument of handle_attributes() is no more needed since it always
must be 'KW_ATTRIBUTE'.

So, remove this argument.

Note: this is preparation work to later make the distinction between
      function/variable/type/label/... attributes.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/parse.c b/parse.c
index 73ec579cfa7f..ec675f20ef01 100644
--- a/parse.c
+++ b/parse.c
@@ -49,7 +49,7 @@ struct symbol_list *function_computed_target_list;
 struct statement_list *function_computed_goto_list;
 
 static struct token *statement(struct token *token, struct statement **tree);
-static struct token *handle_attributes(struct token *token, struct decl_state *ctx, unsigned int keywords);
+static struct token *handle_attributes(struct token *token, struct decl_state *ctx);
 
 typedef struct token *declarator_t(struct token *, struct decl_state *);
 static declarator_t
@@ -756,7 +756,7 @@ static struct token *struct_union_enum_specifier(enum type type,
 	struct symbol *sym;
 	struct position *repos;
 
-	token = handle_attributes(token, ctx, KW_ATTRIBUTE);
+	token = handle_attributes(token, ctx);
 	if (token_type(token) == TOKEN_IDENT) {
 		sym = lookup_symbol(token->ident, NS_STRUCT);
 		if (!sym ||
@@ -956,7 +956,7 @@ static struct token *parse_enum_declaration(struct token *token, struct symbol *
 		struct symbol *sym;
 
 		// FIXME: only 'deprecated' should be accepted
-		next = handle_attributes(next, &ctx, KW_ATTRIBUTE);
+		next = handle_attributes(next, &ctx);
 
 		if (match_op(next, '=')) {
 			next = constant_expression(next->next, &expr);
@@ -1805,7 +1805,7 @@ static struct token *skip_attributes(struct token *token)
 	return token;
 }
 
-static struct token *handle_attributes(struct token *token, struct decl_state *ctx, unsigned int keywords)
+static struct token *handle_attributes(struct token *token, struct decl_state *ctx)
 {
 	struct symbol *keyword;
 	for (;;) {
@@ -1814,7 +1814,7 @@ static struct token *handle_attributes(struct token *token, struct decl_state *c
 		keyword = lookup_keyword(token->ident, NS_KEYWORD | NS_TYPEDEF);
 		if (!keyword || keyword->type != SYM_KEYWORD)
 			break;
-		if (!(keyword->op->type & keywords))
+		if (!(keyword->op->type & KW_ATTRIBUTE))
 			break;
 		token = keyword->op->declarator(token->next, ctx);
 	}
@@ -1903,8 +1903,7 @@ static struct token *direct_declarator(struct token *token, struct decl_state *c
 	    is_nested(token, &next, ctx->prefer_abstract)) {
 		struct symbol *base_type = ctype->base_type;
 		if (token->next != next)
-			next = handle_attributes(token->next, ctx,
-						  KW_ATTRIBUTE);
+			next = handle_attributes(token->next, ctx);
 		token = declarator(next, ctx);
 		token = expect(token, ')', "in nested declarator");
 		while (ctype->base_type != base_type)
@@ -2027,7 +2026,7 @@ static struct token *declaration_list(struct token *token, struct symbol_list **
 		if (match_op(token, ':'))
 			token = handle_bitfield(token, &ctx);
 
-		token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+		token = handle_attributes(token, &ctx);
 		apply_modifiers(token->pos, &ctx);
 
 		decl->ctype = ctx.ctype;
@@ -2067,7 +2066,7 @@ static struct token *parameter_declaration(struct token *token, struct symbol *s
 	token = declaration_specifiers(token, &ctx);
 	ctx.ident = &sym->ident;
 	token = declarator(token, &ctx);
-	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+	token = handle_attributes(token, &ctx);
 	apply_modifiers(token->pos, &ctx);
 	sym->ctype = ctx.ctype;
 	sym->ctype.modifiers |= decl_modifiers(&ctx);
@@ -2567,7 +2566,7 @@ static struct token *handle_label_attributes(struct token *token, struct symbol
 {
 	struct decl_state ctx = { };
 
-	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+	token = handle_attributes(token, &ctx);
 	label->label_modifiers = ctx.ctype.modifiers;
 	return token;
 }
@@ -3028,7 +3027,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 	saved = ctx.ctype;
 	token = declarator(token, &ctx);
 	token = handle_asm_name(token, &ctx);
-	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+	token = handle_attributes(token, &ctx);
 	apply_modifiers(token->pos, &ctx);
 
 	decl->ctype = ctx.ctype;
@@ -3150,10 +3149,10 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 		ident = NULL;
 		decl = alloc_symbol(token->pos, SYM_NODE);
 		ctx.ctype = saved;
-		token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+		token = handle_attributes(token, &ctx);
 		token = declarator(token, &ctx);
 		token = handle_asm_name(token, &ctx);
-		token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+		token = handle_attributes(token, &ctx);
 		apply_modifiers(token->pos, &ctx);
 		decl->ctype = ctx.ctype;
 		decl->ctype.modifiers |= mod;
-- 
2.28.0

[PATCH 05/10] attribute: directly use attribute_specifier() to handle attributes

[Luc Van Oostenryck]

handle_attributes() used to also handle asm names and so used
the declarator method associated to the taken. But now, asm names
are handled in a separated function.

So, directly use attribute_specifier() to handle the attributes
instead of using it via the declarator method.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index ec675f20ef01..14b1746d38c5 100644
--- a/parse.c
+++ b/parse.c
@@ -1816,7 +1816,7 @@ static struct token *handle_attributes(struct token *token, struct decl_state *c
 			break;
 		if (!(keyword->op->type & KW_ATTRIBUTE))
 			break;
-		token = keyword->op->declarator(token->next, ctx);
+		token = attribute_specifier(token->next, ctx);
 	}
 	return token;
 }
-- 
2.28.0

[PATCH 06/10] attribute: factorize matching of '__attribute__'

[Luc Van Oostenryck]

Matching the keyword '__attribute__' (or '__attribute') needs
several tests and this matching is needed to handle attributes
or to skip them.

So, create an helper, match_attribute(), and use it in the loops
to handle and to skip attributes.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/parse.c b/parse.c
index 14b1746d38c5..681d98e49641 100644
--- a/parse.c
+++ b/parse.c
@@ -1761,6 +1761,20 @@ static struct token *handle_asm_name(struct token *token, struct decl_state *ctx
 	return token;
 }
 
+///
+// test if @token is '__attribute__' (or one of its variant)
+static bool match_attribute(struct token *token)
+{
+	struct symbol *sym;
+
+	if (token_type(token) != TOKEN_IDENT)
+		return false;
+	sym = lookup_keyword(token->ident, NS_KEYWORD | NS_TYPEDEF);
+	if (!sym || sym->type != SYM_KEYWORD)
+		return false;
+	return sym->op->type & KW_ATTRIBUTE;
+}
+
 static struct token *skip_attribute(struct token *token)
 {
 	token = token->next;
@@ -1782,15 +1796,7 @@ static struct token *skip_attribute(struct token *token)
 
 static struct token *skip_attributes(struct token *token)
 {
-	struct symbol *keyword;
-	for (;;) {
-		if (token_type(token) != TOKEN_IDENT)
-			break;
-		keyword = lookup_keyword(token->ident, NS_KEYWORD | NS_TYPEDEF);
-		if (!keyword || keyword->type != SYM_KEYWORD)
-			break;
-		if (!(keyword->op->type & KW_ATTRIBUTE))
-			break;
+	while (match_attribute(token)) {
 		token = expect(token->next, '(', "after attribute");
 		token = expect(token, '(', "after attribute");
 		while (token_type(token) == TOKEN_IDENT) {
@@ -1807,17 +1813,8 @@ static struct token *skip_attributes(struct token *token)
 
 static struct token *handle_attributes(struct token *token, struct decl_state *ctx)
 {
-	struct symbol *keyword;
-	for (;;) {
-		if (token_type(token) != TOKEN_IDENT)
-			break;
-		keyword = lookup_keyword(token->ident, NS_KEYWORD | NS_TYPEDEF);
-		if (!keyword || keyword->type != SYM_KEYWORD)
-			break;
-		if (!(keyword->op->type & KW_ATTRIBUTE))
-			break;
+	while (match_attribute(token))
 		token = attribute_specifier(token->next, ctx);
-	}
 	return token;
 }
 
-- 
2.28.0

[PATCH 07/10] attribute: no need to lookup '__attribute__' in NS_KEYWORD

[Luc Van Oostenryck]

Since '__attribute__' is in NS_TYPEDEF, it's not useful to
look it up also in NS_KEYWORD.

So, remove NS_KEYWORD from the mask while looking up '__attribute__'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index 681d98e49641..f2fdbc9b5a7b 100644
--- a/parse.c
+++ b/parse.c
@@ -1769,7 +1769,7 @@ static bool match_attribute(struct token *token)
 
 	if (token_type(token) != TOKEN_IDENT)
 		return false;
-	sym = lookup_keyword(token->ident, NS_KEYWORD | NS_TYPEDEF);
+	sym = lookup_keyword(token->ident, NS_TYPEDEF);
 	if (!sym || sym->type != SYM_KEYWORD)
 		return false;
 	return sym->op->type & KW_ATTRIBUTE;
-- 
2.28.0

[PATCH 08/10] testing for SYM_KEYWORD is unneeded for lookup_keyword()

[Luc Van Oostenryck]

All symbols returned by lookup_keyword() are of type SYM_KEYWORD,
because either:
   1) it's in NS_KEYWORD (and all symbol in NS_KEYWORD are SYM_KEYWORD)
   2) it's in NS_TYPEDEF and all *keywords* in NS_TYPEDEF are reserved
      and so can't be user defined and so must be SYM_KEYWORD.
It's thus unneeded to test it.

So, remove the unneeded test.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/parse.c b/parse.c
index f2fdbc9b5a7b..2b7ef2ae23c4 100644
--- a/parse.c
+++ b/parse.c
@@ -1620,8 +1620,6 @@ static struct token *handle_qualifiers(struct token *t, struct decl_state *ctx)
 		struct symbol *s = lookup_keyword(t->ident, NS_TYPEDEF);
 		if (!s)
 			break;
-		if (s->type != SYM_KEYWORD)
-			break;
 		if (!(s->op->type & (KW_ATTRIBUTE | KW_QUALIFIER)))
 			break;
 		t = t->next;
@@ -1749,7 +1747,7 @@ static struct token *handle_asm_name(struct token *token, struct decl_state *ctx
 	if (token_type(token) != TOKEN_IDENT)
 		return token;
 	keyword = lookup_keyword(token->ident, NS_KEYWORD);
-	if (!keyword || keyword->type != SYM_KEYWORD)
+	if (!keyword)
 		return token;
 	if (!(keyword->op->type & KW_ASM))
 		return token;
@@ -1770,7 +1768,7 @@ static bool match_attribute(struct token *token)
 	if (token_type(token) != TOKEN_IDENT)
 		return false;
 	sym = lookup_keyword(token->ident, NS_TYPEDEF);
-	if (!sym || sym->type != SYM_KEYWORD)
+	if (!sym)
 		return false;
 	return sym->op->type & KW_ATTRIBUTE;
 }
-- 
2.28.0

[PATCH 09/10] testing for sym->op is unneeded for lookup_keyword()

[Luc Van Oostenryck]

All symbols returned by lookup_keyword() are of type SYM_KEYWORD,
because either:
   1) it's in NS_KEYWORD (and all symbol in NS_KEYWORD are SYM_KEYWORD)
   2) it's in NS_TYPEDEF and all *keywords* in NS_TYPEDEF are reserved
      and so can't be user defined and so must be SYM_KEYWORD.
Thus, they all have a symbol_op associated to them and it's
unneeded to test it.

So, remove the unneeded test.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index 2b7ef2ae23c4..d378f1255fee 100644
--- a/parse.c
+++ b/parse.c
@@ -2160,7 +2160,7 @@ static struct token *parse_asm_statement(struct token *token, struct statement *
 	stmt->type = STMT_ASM;
 	while (token_type(token) == TOKEN_IDENT) {
 		struct symbol *s = lookup_keyword(token->ident, NS_TYPEDEF);
-		if (s && s->op  && s->op->asm_modifier)
+		if (s && s->op->asm_modifier)
 			s->op->asm_modifier(token, &mods);
 		else if (token->ident == &goto_ident)
 			asm_modifier(token, &mods, MOD_ASM_GOTO);
-- 
2.28.0

[PATCH 10/10] keyword type is a bitmask and must be tested so

[Luc Van Oostenryck]

The keyword's type is a bitmask because depending on the context
the same keyword can be of different type (for example 'const'
as qualifier and the attribute 'const' , a variant of 'pure').

Thus, it's an error to test this type for equality, instead it's
a specific (set of) bit(s) that must be tested.

So, change a test ' x == KW_...' into 'x & KW_...'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index d378f1255fee..e1a5cce4e46b 100644
--- a/parse.c
+++ b/parse.c
@@ -1287,7 +1287,7 @@ static struct token *attribute_mode(struct token *token, struct symbol *attr, st
 	token = expect(token, '(', "after mode attribute");
 	if (token_type(token) == TOKEN_IDENT) {
 		struct symbol *mode = lookup_keyword(token->ident, NS_KEYWORD);
-		if (mode && mode->op->type == KW_MODE)
+		if (mode && mode->op->type & KW_MODE)
 			ctx->mode = mode->op;
 		else
 			sparse_error(token->pos, "unknown mode attribute %s", show_ident(token->ident));
-- 
2.28.0