Re: [PATCH v2] gdb: search local symbols before global symbols in completion

=D0=BF=D0=BD, 3 =D0=B0=D0=B2=D0=B3. 2026=E2=80=AF=D0=B3. =D0=B2 22:26, Oleg=
 Tolmatcev <[email protected]>:
>
> When symbol completion is performed from a selected frame, search the
> local blocks before scanning minimal symbols and global/static symbols.
> This makes completion prefer names that are visible in the current
> context, including local variables.
>
> Add a regression test that checks completion prefers a local symbol over
> a global symbol with the same name.
> ---
>  gdb/symtab.c                          | 72 +++++++++++++--------------
>  gdb/testsuite/gdb.base/break.c        | 21 ++++++++
>  gdb/testsuite/gdb.base/completion.exp |  6 +++
>  3 files changed, 63 insertions(+), 36 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 5d5076f2e77..85665bebdf4 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5931,6 +5931,42 @@ default_collect_symbol_completion_matches_break_on
>
>    lookup_name_info lookup_name (sym_text, name_match_type, true);
>
> +  /* Search upwards from currently selected frame (so that we can
> +     complete on local vars).  Also catch fields of types defined in
> +     this places which match our text string.  Only complete on types
> +     visible from current context.  */
> +
> +  b =3D get_selected_block ();
> +  surrounding_static_block =3D b =3D=3D nullptr ? nullptr : b->static_bl=
ock ();
> +  surrounding_global_block =3D b =3D=3D nullptr ? nullptr : b->global_bl=
ock ();
> +  if (surrounding_static_block !=3D NULL)
> +    while (b !=3D surrounding_static_block)
> +      {
> +       QUIT;
> +
> +       for (struct symbol *sym : block_iterator_range (b))
> +         {
> +           if (code =3D=3D TYPE_CODE_UNDEF)
> +             {
> +               completion_list_add_symbol (tracker, sym, lookup_name,
> +                                           sym_text, word);
> +               completion_list_add_fields (tracker, sym, lookup_name,
> +                                           sym_text, word);
> +             }
> +           else if (sym->domain () =3D=3D STRUCT_DOMAIN
> +                    && sym->type ()->code () =3D=3D code)
> +             completion_list_add_symbol (tracker, sym, lookup_name,
> +                                         sym_text, word);
> +         }
> +
> +       /* Stop when we encounter an enclosing function.  Do not stop for
> +          non-inlined functions - the locals of the enclosing function
> +          are in scope for a nested function.  */
> +       if (b->function () !=3D NULL && b->inlined_p ())
> +         break;
> +       b =3D b->superblock ();
> +      }
> +
>    /* At this point scan through the misc symbol vectors and add each
>       symbol you find to the list.  Eventually we want to ignore
>       anything that isn't a text symbol (everything else will be
> @@ -5974,42 +6010,6 @@ default_collect_symbol_completion_matches_break_on
>          SEARCH_ALL_DOMAINS);
>      }
>
> -  /* Search upwards from currently selected frame (so that we can
> -     complete on local vars).  Also catch fields of types defined in
> -     this places which match our text string.  Only complete on types
> -     visible from current context.  */
> -
> -  b =3D get_selected_block ();
> -  surrounding_static_block =3D b =3D=3D nullptr ? nullptr : b->static_bl=
ock ();
> -  surrounding_global_block =3D b =3D=3D nullptr ? nullptr : b->global_bl=
ock ();
> -  if (surrounding_static_block !=3D NULL)
> -    while (b !=3D surrounding_static_block)
> -      {
> -       QUIT;
> -
> -       for (struct symbol *sym : block_iterator_range (b))
> -         {
> -           if (code =3D=3D TYPE_CODE_UNDEF)
> -             {
> -               completion_list_add_symbol (tracker, sym, lookup_name,
> -                                           sym_text, word);
> -               completion_list_add_fields (tracker, sym, lookup_name,
> -                                           sym_text, word);
> -             }
> -           else if (sym->domain () =3D=3D STRUCT_DOMAIN
> -                    && sym->type ()->code () =3D=3D code)
> -             completion_list_add_symbol (tracker, sym, lookup_name,
> -                                         sym_text, word);
> -         }
> -
> -       /* Stop when we encounter an enclosing function.  Do not stop for
> -          non-inlined functions - the locals of the enclosing function
> -          are in scope for a nested function.  */
> -       if (b->function () !=3D NULL && b->inlined_p ())
> -         break;
> -       b =3D b->superblock ();
> -      }
> -
>    /* Add fields from the file's types; symbols will be added below.  */
>
>    if (code =3D=3D TYPE_CODE_UNDEF)
> diff --git a/gdb/testsuite/gdb.base/break.c b/gdb/testsuite/gdb.base/brea=
k.c
> index 26085d40ca2..c9b74c6a567 100644
> --- a/gdb/testsuite/gdb.base/break.c
> +++ b/gdb/testsuite/gdb.base/break.c
> @@ -23,6 +23,26 @@ extern int marker2 (int a);
>  extern void marker3 (char *a, char *b);
>  extern void marker4 (long d);
>
> +struct completion_global_struct
> +{
> +  int global_field;
> +};
> +
> +struct completion_local_struct
> +{
> +  int local_field;
> +};
> +
> +struct completion_global_struct completion_var;
> +
> +static void
> +completion_local_over_global (void)
> +{
> +  struct completion_local_struct completion_var =3D { 23 };
> +
> +  completion_var.local_field++; /* local-completion-breakpoint */
> +}
> +
>  /* We're used by a test that requires malloc, so make sure it is in
>     the executable.  */
>  void *need_malloc ()
> @@ -50,6 +70,7 @@ main (int argc, char **argv, char **envp)
>      marker2 (43); /* set breakpoint 20 here */
>      marker3 ("stack", "trace"); /* set breakpoint 21 here */
>      marker4 (177601976L);
> +    completion_local_over_global ();
>      /* We're used by a test that requires malloc, so make sure it is
>         in the executable.  */
>      (void)malloc (1);
> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.ba=
se/completion.exp
> index 44cf649d5b6..9848f21d951 100644
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -87,6 +87,12 @@ gdb_test "complete set gnutarget aut" "set gnutarget a=
uto"
>
>  gdb_test "complete set cp-abi aut" "set cp-abi auto"
>
> +# Check that symbol completion in a selected frame prefers a local
> +# symbol over a global symbol with the same name.
> +gdb_breakpoint [gdb_get_line_number "local-completion-breakpoint"]
> +gdb_continue_to_breakpoint "local-completion-breakpoint"
> +gdb_test "complete p completion_var.l" "p completion_var.local_field"
> +
>  # Test that completion of commands 'target FOO' works well.
>  set targets [list "core" "tfile" "exec"]
>
> --
> 2.55.0.windows.1
>

For some reason this patch caused regressions on Arm. I need to investigate=
.
The previous one, without the test didn't cause regressions.

Oleg