Add C parser support for "restrict" and "_Atomic"

  A user noticed that "watch -location" would fail with a "restrict"
pointer.  The issue here is that if the DWARF mentions "restrict", gdb
will put this into the type name -- but then the C parser will not be
able to parse this type.

This patch adds support for "restrict" and "_Atomic" to the C parser.
It is done only for C and Objective C, not C++.

I wasn't sure if "restrict" should be marked FLAG_SHADOW to support
older dialects of C, where this was not a keyword.

gdb/ChangeLog
2020-02-29  Tom Tromey  <tom@tromey.com>

	* type-stack.h (enum type_pieces) <tp_atomic, tp_restrict>: New
	constants.
	* type-stack.c (type_stack::insert): Handle tp_atomic and
	tp_restrict.
	(type_stack::follow_type_instance_flags): Likewise.
	(type_stack::follow_types): Likewise.  Merge type-following code.
	* c-exp.y (RESTRICT, ATOMIC): New tokens.
	(space_identifier, cv_with_space_id)
	(const_or_volatile_or_space_identifier_noopt)
	(const_or_volatile_or_space_identifier): Remove.
	(single_qualifier, qualifier_seq_noopt, qualifier_seq): New
	rules.
	(ptr_operator, typebase): Update.
	(enum token_flag) <FLAG_C>: New constant.
	(ident_tokens): Add "restrict" and "_Atomic".
	(lex_one_token): Handle FLAG_C.

gdb/testsuite/ChangeLog
2020-02-29  Tom Tromey  <tom@tromey.com>

	* gdb.base/cvexpr.exp: Add test for _Atomic and restrict.
---
 gdb/ChangeLog                     | 19 ++++++++
 gdb/c-exp.y                       | 52 ++++++++++++++-------
 gdb/testsuite/ChangeLog           |  4 ++
 gdb/testsuite/gdb.base/cvexpr.exp |  4 ++
 gdb/type-stack.c                  | 76 +++++++++++++++----------------
 gdb/type-stack.h                  |  2 +
 6 files changed, 101 insertions(+), 56 deletions(-)
  

On Sun, 1 Mar 2020 at 06:01, Tom Tromey <tom@tromey.com> wrote:
>
> A user noticed that "watch -location" would fail with a "restrict"
> pointer.  The issue here is that if the DWARF mentions "restrict", gdb
> will put this into the type name -- but then the C parser will not be
> able to parse this type.
>
> This patch adds support for "restrict" and "_Atomic" to the C parser.
> It is done only for C and Objective C, not C++.

GCC has the keywords __restrict__ and __restrict that work in C++ too.
These are also affected, including the use in C++.

>
> I wasn't sure if "restrict" should be marked FLAG_SHADOW to support
> older dialects of C, where this was not a keyword.
>
> gdb/ChangeLog
> 2020-02-29  Tom Tromey  <tom@tromey.com>
>
>         * type-stack.h (enum type_pieces) <tp_atomic, tp_restrict>: New
>         constants.
>         * type-stack.c (type_stack::insert): Handle tp_atomic and
>         tp_restrict.
>         (type_stack::follow_type_instance_flags): Likewise.
>         (type_stack::follow_types): Likewise.  Merge type-following code.
>         * c-exp.y (RESTRICT, ATOMIC): New tokens.
>         (space_identifier, cv_with_space_id)
>         (const_or_volatile_or_space_identifier_noopt)
>         (const_or_volatile_or_space_identifier): Remove.
>         (single_qualifier, qualifier_seq_noopt, qualifier_seq): New
>         rules.
>         (ptr_operator, typebase): Update.
>         (enum token_flag) <FLAG_C>: New constant.
>         (ident_tokens): Add "restrict" and "_Atomic".
>         (lex_one_token): Handle FLAG_C.
>
> gdb/testsuite/ChangeLog
> 2020-02-29  Tom Tromey  <tom@tromey.com>
>
>         * gdb.base/cvexpr.exp: Add test for _Atomic and restrict.
> ---
>  gdb/ChangeLog                     | 19 ++++++++
>  gdb/c-exp.y                       | 52 ++++++++++++++-------
>  gdb/testsuite/ChangeLog           |  4 ++
>  gdb/testsuite/gdb.base/cvexpr.exp |  4 ++
>  gdb/type-stack.c                  | 76 +++++++++++++++----------------
>  gdb/type-stack.h                  |  2 +
>  6 files changed, 101 insertions(+), 56 deletions(-)
>
> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
> index 3403a857a83..1b35ef2e60a 100644
> --- a/gdb/c-exp.y
> +++ b/gdb/c-exp.y
> @@ -237,6 +237,7 @@ static void c_print_token (FILE *file, int type, YYSTYPE value);
>  /* Special type cases, put in to allow the parser to distinguish different
>     legal basetypes.  */
>  %token SIGNED_KEYWORD LONG SHORT INT_KEYWORD CONST_KEYWORD VOLATILE_KEYWORD DOUBLE_KEYWORD
> +%token RESTRICT ATOMIC
>
>  %token <sval> DOLLAR_VARIABLE
>
> @@ -1169,36 +1170,43 @@ variable:       name_not_typename
>                         }
>         ;
>
> -space_identifier : '@' NAME
> -               {
> -                 cpstate->type_stack.insert (pstate,
> -                                             copy_name ($2.stoken).c_str ());
> -               }
> -       ;
> -
>  const_or_volatile: const_or_volatile_noopt
>         |
>         ;
>
> -cv_with_space_id : const_or_volatile space_identifier const_or_volatile
> +single_qualifier:
> +               CONST_KEYWORD
> +                       { cpstate->type_stack.insert (tp_const); }
> +       |       VOLATILE_KEYWORD
> +                       { cpstate->type_stack.insert (tp_volatile); }
> +       |       ATOMIC
> +                       { cpstate->type_stack.insert (tp_atomic); }
> +       |       RESTRICT
> +                       { cpstate->type_stack.insert (tp_restrict); }
> +       |       '@' NAME
> +               {
> +                 cpstate->type_stack.insert (pstate,
> +                                             copy_name ($2.stoken).c_str ());
> +               }
>         ;
>
> -const_or_volatile_or_space_identifier_noopt: cv_with_space_id
> -       | const_or_volatile_noopt
> +qualifier_seq_noopt:
> +               single_qualifier
> +       |       qualifier_seq single_qualifier
>         ;
>
> -const_or_volatile_or_space_identifier:
> -               const_or_volatile_or_space_identifier_noopt
> +qualifier_seq:
> +               qualifier_seq_noopt
>         |
>         ;
>
>  ptr_operator:
>                 ptr_operator '*'
>                         { cpstate->type_stack.insert (tp_pointer); }
> -               const_or_volatile_or_space_identifier
> +               qualifier_seq
>         |       '*'
>                         { cpstate->type_stack.insert (tp_pointer); }
> -               const_or_volatile_or_space_identifier
> +               qualifier_seq
>         |       '&'
>                         { cpstate->type_stack.insert (tp_reference); }
>         |       '&' ptr_operator
> @@ -1472,9 +1480,9 @@ typebase
>                             (copy_name($2).c_str (), $4,
>                              pstate->expression_context_block);
>                         }
> -       | const_or_volatile_or_space_identifier_noopt typebase
> +       |       qualifier_seq_noopt typebase
>                         { $$ = cpstate->type_stack.follow_types ($2); }
> -       | typebase const_or_volatile_or_space_identifier_noopt
> +       |       typebase qualifier_seq_noopt
>                         { $$ = cpstate->type_stack.follow_types ($1); }
>         ;
>
> @@ -2345,6 +2353,10 @@ enum token_flag
>
>    FLAG_CXX = 1,
>
> +  /* If this bit is set, the token is C-only.  */
> +
> +  FLAG_C = 1,
> +
>    /* If this bit is set, the token is conditional: if there is a
>       symbol of the same name, then the token is a symbol; otherwise,
>       the token is a keyword.  */
> @@ -2416,6 +2428,8 @@ static const struct token ident_tokens[] =
>      {"union", UNION, OP_NULL, 0},
>      {"short", SHORT, OP_NULL, 0},
>      {"const", CONST_KEYWORD, OP_NULL, 0},
> +    {"restrict", RESTRICT, OP_NULL, 0},
> +    {"_Atomic", ATOMIC, OP_NULL, 0},
>      {"enum", ENUM, OP_NULL, 0},
>      {"long", LONG, OP_NULL, 0},
>      {"true", TRUEKEYWORD, OP_NULL, FLAG_CXX},
> @@ -2550,6 +2564,7 @@ lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
>         if ((tokentab3[i].flags & FLAG_CXX) != 0
>             && par_state->language ()->la_language != language_cplus)
>           break;
> +       gdb_assert ((tokentab3[i].flags & FLAG_C) == 0);
>
>         pstate->lexptr += 3;
>         yylval.opcode = tokentab3[i].opcode;
> @@ -2563,6 +2578,7 @@ lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
>         if ((tokentab2[i].flags & FLAG_CXX) != 0
>             && par_state->language ()->la_language != language_cplus)
>           break;
> +       gdb_assert ((tokentab3[i].flags & FLAG_C) == 0);
>
>         pstate->lexptr += 2;
>         yylval.opcode = tokentab2[i].opcode;
> @@ -2857,6 +2873,10 @@ lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
>         if ((ident_tokens[i].flags & FLAG_CXX) != 0
>             && par_state->language ()->la_language != language_cplus)
>           break;
> +       if ((ident_tokens[i].flags & FLAG_C) != 0
> +           && par_state->language ()->la_language != language_c
> +           && par_state->language ()->la_language != language_objc)
> +         break;
>
>         if ((ident_tokens[i].flags & FLAG_SHADOW) != 0)
>           {
> diff --git a/gdb/testsuite/gdb.base/cvexpr.exp b/gdb/testsuite/gdb.base/cvexpr.exp
> index 92a073a774e..cf5d2c4a553 100644
> --- a/gdb/testsuite/gdb.base/cvexpr.exp
> +++ b/gdb/testsuite/gdb.base/cvexpr.exp
> @@ -509,3 +509,7 @@ foreach testspec $specs {
>         do_test $prefix $opts
>      }
>  }
> +
> +# These tests don't rely on the debug format.
> +gdb_test "ptype _Atomic int" "type = _Atomic int"
> +gdb_test "ptype int * restrict" "type = int \\* restrict"
> diff --git a/gdb/type-stack.c b/gdb/type-stack.c
> index ab7e0261cad..73b7d5a8dfc 100644
> --- a/gdb/type-stack.c
> +++ b/gdb/type-stack.c
> @@ -33,12 +33,14 @@ type_stack::insert (enum type_pieces tp)
>
>    gdb_assert (tp == tp_pointer || tp == tp_reference
>               || tp == tp_rvalue_reference || tp == tp_const
> -             || tp == tp_volatile);
> +             || tp == tp_volatile || tp == tp_restrict
> +             || tp == tp_atomic);
>
>    /* If there is anything on the stack (we know it will be a
>       tp_pointer), insert the qualifier above it.  Otherwise, simply
>       push this on the top of the stack.  */
> -  if (!m_elements.empty () && (tp == tp_const || tp == tp_volatile))
> +  if (!m_elements.empty () && (tp == tp_const || tp == tp_volatile
> +                              || tp == tp_restrict))
>      slot = 1;
>    else
>      slot = 0;
> @@ -88,6 +90,12 @@ type_stack::follow_type_instance_flags ()
>        case tp_volatile:
>         flags |= TYPE_INSTANCE_FLAG_VOLATILE;
>         break;
> +      case tp_atomic:
> +       flags |= TYPE_INSTANCE_FLAG_ATOMIC;
> +       break;
> +      case tp_restrict:
> +       flags |= TYPE_INSTANCE_FLAG_RESTRICT;
> +       break;
>        default:
>         gdb_assert_not_reached ("unrecognized tp_ value in follow_types");
>        }
> @@ -102,6 +110,8 @@ type_stack::follow_types (struct type *follow_type)
>    int make_const = 0;
>    int make_volatile = 0;
>    int make_addr_space = 0;
> +  bool make_restrict = false;
> +  bool make_atomic = false;
>    int array_size;
>
>    while (!done)
> @@ -109,19 +119,7 @@ type_stack::follow_types (struct type *follow_type)
>        {
>        case tp_end:
>         done = 1;
> -       if (make_const)
> -         follow_type = make_cv_type (make_const,
> -                                     TYPE_VOLATILE (follow_type),
> -                                     follow_type, 0);
> -       if (make_volatile)
> -         follow_type = make_cv_type (TYPE_CONST (follow_type),
> -                                     make_volatile,
> -                                     follow_type, 0);
> -       if (make_addr_space)
> -         follow_type = make_type_with_address_space (follow_type,
> -                                                     make_addr_space);
> -       make_const = make_volatile = 0;
> -       make_addr_space = 0;
> +       goto process_qualifiers;
>         break;
>        case tp_const:
>         make_const = 1;
> @@ -132,41 +130,39 @@ type_stack::follow_types (struct type *follow_type)
>        case tp_space_identifier:
>         make_addr_space = pop_int ();
>         break;
> +      case tp_atomic:
> +       make_atomic = true;
> +       break;
> +      case tp_restrict:
> +       make_restrict = true;
> +       break;
>        case tp_pointer:
>         follow_type = lookup_pointer_type (follow_type);
> +       goto process_qualifiers;
> +      case tp_reference:
> +       follow_type = lookup_lvalue_reference_type (follow_type);
> +       goto process_qualifiers;
> +      case tp_rvalue_reference:
> +       follow_type = lookup_rvalue_reference_type (follow_type);
> +      process_qualifiers:
>         if (make_const)
> -         follow_type = make_cv_type (make_const,
> -                                     TYPE_VOLATILE (follow_type),
> +         follow_type = make_cv_type (make_const,
> +                                     TYPE_VOLATILE (follow_type),
>                                       follow_type, 0);
>         if (make_volatile)
> -         follow_type = make_cv_type (TYPE_CONST (follow_type),
> -                                     make_volatile,
> +         follow_type = make_cv_type (TYPE_CONST (follow_type),
> +                                     make_volatile,
>                                       follow_type, 0);
>         if (make_addr_space)
> -         follow_type = make_type_with_address_space (follow_type,
> +         follow_type = make_type_with_address_space (follow_type,
>                                                       make_addr_space);
> +       if (make_restrict)
> +         follow_type = make_restrict_type (follow_type);
> +       if (make_atomic)
> +         follow_type = make_atomic_type (follow_type);
>         make_const = make_volatile = 0;
>         make_addr_space = 0;
> -       break;
> -      case tp_reference:
> -        follow_type = lookup_lvalue_reference_type (follow_type);
> -        goto process_reference;
> -       case tp_rvalue_reference:
> -        follow_type = lookup_rvalue_reference_type (follow_type);
> -       process_reference:
> -        if (make_const)
> -          follow_type = make_cv_type (make_const,
> -                                      TYPE_VOLATILE (follow_type),
> -                                      follow_type, 0);
> -        if (make_volatile)
> -          follow_type = make_cv_type (TYPE_CONST (follow_type),
> -                                      make_volatile,
> -                                      follow_type, 0);
> -        if (make_addr_space)
> -          follow_type = make_type_with_address_space (follow_type,
> -                                                      make_addr_space);
> -       make_const = make_volatile = 0;
> -       make_addr_space = 0;
> +       make_restrict = make_atomic = false;
>         break;
>        case tp_array:
>         array_size = pop_int ();
> diff --git a/gdb/type-stack.h b/gdb/type-stack.h
> index ee004d1be8d..8060f2fea78 100644
> --- a/gdb/type-stack.h
> +++ b/gdb/type-stack.h
> @@ -40,6 +40,8 @@ enum type_pieces
>      tp_const,
>      tp_volatile,
>      tp_space_identifier,
> +    tp_atomic,
> +    tp_restrict,
>      tp_type_stack,
>      tp_kind
>    };
> --
> 2.17.2
>
  

>>>>> "Ruslan" == Ruslan Kabatsayev <b7.10110111@gmail.com> writes:

Ruslan> On Sun, 1 Mar 2020 at 06:01, Tom Tromey <tom@tromey.com> wrote:
>> 
>> A user noticed that "watch -location" would fail with a "restrict"
>> pointer.  The issue here is that if the DWARF mentions "restrict", gdb
>> will put this into the type name -- but then the C parser will not be
>> able to parse this type.
>> 
>> This patch adds support for "restrict" and "_Atomic" to the C parser.
>> It is done only for C and Objective C, not C++.

Ruslan> GCC has the keywords __restrict__ and __restrict that work in C++ too.
Ruslan> These are also affected, including the use in C++.

Thanks, I'll add those.

>> I wasn't sure if "restrict" should be marked FLAG_SHADOW to support
>> older dialects of C, where this was not a keyword.

I'll have to check how C++ prints a restrict type.
Maybe gdb will need another tweak here.

>> +    {"restrict", RESTRICT, OP_NULL, 0},

I forgot to actually use FLAG_C here, oops.

Tom