genksyms: fix 6 shift/reduce conflicts and 5 reduce/reduce conflicts

The genksyms parser has ambiguities in its grammar, which are currently
suppressed by a workaround in scripts/genksyms/Makefile.

Building genksyms with W=1 generates the following warnings:

    YACC    scripts/genksyms/parse.tab.[ch]
  scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
  scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
  scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples

The comment in the parser describes the current problem:

    /* This wasn't really a typedef name but an identifier that
       shadows one.  */

Consider the following simple C code:

    typedef int foo;
    void my_func(foo foo) {}

In the function parameter list (foo foo), the first 'foo' is a type
specifier (typedef'ed as 'int'), while the second 'foo' is an identifier.

However, the lexer cannot distinguish between the two. Since 'foo' is
already typedef'ed, the lexer returns TYPE for both instances, instead
of returning IDENT for the second one.

To support shadowed identifiers, TYPE can be reduced to either a
simple_type_specifier or a direct_abstract_declarator, which creates
a grammatical ambiguity.

Without analyzing the grammar context, it is very difficult to resolve
this correctly.

This commit introduces a flag, dont_want_type_specifier, which allows
the parser to inform the lexer whether an identifier is expected. When
dont_want_type_specifier is true, the type lookup is suppressed, and
the lexer returns IDENT regardless of any preceding typedef.

After this commit, only 3 shift/reduce conflicts will remain.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Acked-by: Nicolas Schier <n.schier@avm.de>

diff --git a/scripts/genksyms/genksyms.h b/scripts/genksyms/genksyms.h
index 8c45ada59ece55ac55323453d74bd63782aedae4..0c355075f0e6701417c95eb43ec7b993e412db55 100644 (file)
--- a/scripts/genksyms/genksyms.h
+++ b/scripts/genksyms/genksyms.h
@@ -12,6 +12,7 @@
 #ifndef MODUTILS_GENKSYMS_H
 #define MODUTILS_GENKSYMS_H 1
 
+#include <stdbool.h>
 #include <stdio.h>
 
 #include <list_types.h>
@@ -66,6 +67,8 @@ struct string_list *copy_list_range(struct string_list *start,
 int yylex(void);
 int yyparse(void);
 
+extern bool dont_want_type_specifier;
+
 void error_with_pos(const char *, ...) __attribute__ ((format(printf, 1, 2)));
 
 /*----------------------------------------------------------------------*/

diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
index a4d7495eaf75ee26e1ef7c54ac2aab98199ff91d..e886133af5783fbf86190f682de7ae0d5d95eeab 100644 (file)
--- a/scripts/genksyms/lex.l
+++ b/scripts/genksyms/lex.l
@@ -12,6 +12,7 @@
 %{
 
 #include <limits.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
@@ -113,6 +114,12 @@ MC_TOKEN           ([~%^&*+=|<>/-]=)|(&&)|("||")|(->)|(<<)|(>>)
 /* The second stage lexer.  Here we incorporate knowledge of the state
    of the parser to tailor the tokens that are returned.  */
 
+/*
+ * The lexer cannot distinguish whether a typedef'ed string is a TYPE or an
+ * IDENT. We need a hint from the parser to handle this accurately.
+ */
+bool dont_want_type_specifier;
+
 int
 yylex(void)
 {
@@ -207,7 +214,7 @@ repeat:
                    goto repeat;
                  }
              }
-           if (!suppress_type_lookup)
+           if (!suppress_type_lookup && !dont_want_type_specifier)
              {
                if (find_symbol(yytext, SYM_TYPEDEF, 1))
                  token = TYPE;

diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
index 20cb3db7f149b8068917322738bbc9c7141fb257..dc575d467bbfe4a8139efefb45757d9a288a8204 100644 (file)
--- a/scripts/genksyms/parse.y
+++ b/scripts/genksyms/parse.y
@@ -12,6 +12,7 @@
 %{
 
 #include <assert.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include "genksyms.h"
@@ -148,6 +149,7 @@ simple_declaration:
                    current_name = NULL;
                  }
                  $$ = $3;
+                 dont_want_type_specifier = false;
                }
        ;
 
@@ -169,6 +171,7 @@ init_declarator_list:
                             is_typedef ? SYM_TYPEDEF : SYM_NORMAL, decl, is_extern);
                  current_name = NULL;
                  $$ = $1;
+                 dont_want_type_specifier = true;
                }
        | init_declarator_list ',' init_declarator
                { struct string_list *decl = *$3;
@@ -184,6 +187,7 @@ init_declarator_list:
                             is_typedef ? SYM_TYPEDEF : SYM_NORMAL, decl, is_extern);
                  current_name = NULL;
                  $$ = $3;
+                 dont_want_type_specifier = true;
                }
        ;
 
@@ -210,7 +214,7 @@ decl_specifier:
                  remove_node($1);
                  $$ = $1;
                }
-       | type_specifier
+       | type_specifier        { dont_want_type_specifier = true; $$ = $1; }
        | type_qualifier
        ;
 
@@ -307,15 +311,7 @@ direct_declarator:
                    current_name = (*$1)->string;
                    $$ = $1;
                  }
-               }
-       | TYPE
-               { if (current_name != NULL) {
-                   error_with_pos("unexpected second declaration name");
-                   YYERROR;
-                 } else {
-                   current_name = (*$1)->string;
-                   $$ = $1;
-                 }
+                 dont_want_type_specifier = false;
                }
        | direct_declarator '(' parameter_declaration_clause ')'
                { $$ = $4; }
@@ -335,8 +331,7 @@ nested_declarator:
        ;
 
 direct_nested_declarator:
-       IDENT
-       | TYPE
+       IDENT   { $$ = $1; dont_want_type_specifier = false; }
        | direct_nested_declarator '(' parameter_declaration_clause ')'
                { $$ = $4; }
        | direct_nested_declarator '(' error ')'
@@ -362,8 +357,9 @@ parameter_declaration_list_opt:
 
 parameter_declaration_list:
        parameter_declaration
+               { $$ = $1; dont_want_type_specifier = false; }
        | parameter_declaration_list ',' parameter_declaration
-               { $$ = $3; }
+               { $$ = $3; dont_want_type_specifier = false; }
        ;
 
 parameter_declaration:
@@ -375,6 +371,7 @@ abstract_declarator:
        ptr_operator abstract_declarator
                { $$ = $2 ? $2 : $1; }
        | direct_abstract_declarator
+               { $$ = $1; dont_want_type_specifier = false; }
        ;
 
 direct_abstract_declarator:
@@ -385,12 +382,6 @@ direct_abstract_declarator:
                  remove_node($1);
                  $$ = $1;
                }
-       /* This wasn't really a typedef name but an identifier that
-          shadows one.  */
-       | TYPE
-               { remove_node($1);
-                 $$ = $1;
-               }
        | direct_abstract_declarator '(' parameter_declaration_clause ')'
                { $$ = $4; }
        | direct_abstract_declarator '(' error ')'
@@ -440,9 +431,9 @@ member_specification:
 
 member_declaration:
        decl_specifier_seq_opt member_declarator_list_opt ';'
-               { $$ = $3; }
+               { $$ = $3; dont_want_type_specifier = false; }
        | error ';'
-               { $$ = $2; }
+               { $$ = $2; dont_want_type_specifier = false; }
        ;
 
 member_declarator_list_opt:
@@ -452,7 +443,9 @@ member_declarator_list_opt:
 
 member_declarator_list:
        member_declarator
-       | member_declarator_list ',' member_declarator  { $$ = $3; }
+               { $$ = $1; dont_want_type_specifier = true; }
+       | member_declarator_list ',' member_declarator
+               { $$ = $3; dont_want_type_specifier = true; }
        ;
 
 member_declarator: