Re: [PATCH] Debug support for global alias variable

[Tom Tromey <tom@tromey.com>]

>>>>> Natarajan, Kavitha via Gdb-patches <gdb-patches@sourceware.org> writes:

> When clang emits the dwarf information of global alias variable
> as DW_TAG_imported_declaration, gdb does not handle it. GDB reads
> this tag as C++/fortran imported declaration (type alias, namespace
> alias and fortran module). Added support to handle this tag as
> alias variable.

Thank you for the patch.

> +      if (((cu->per_cu->lang == language_c)
> +          || (cu->per_cu->lang == language_cplus))

This expression is over-parenthesized.
The inner parens can be removed.

However, I don't understand why this would necessarily be specific to C
or C++.  The old read_namespace_alias wasn't, so doesn't this check
possibly regress for some other language?

> +#define MAX_NESTED_IMPORTED_DECLARATIONS 100

There doesn't seem to be a need to move this.

>        /* If the compiler has nested DW_AT_imported_declaration DIEs,
> -        keep inspecting DIEs until we hit the underlying import.  */
> -#define MAX_NESTED_IMPORTED_DECLARATIONS 100
> +         keep inspecting DIEs until we hit the underlying import.  */

This is an unnecessary change to the indentation.
I think gdb uses tabs, so it's also incorrect.

>        for (num = 0; num  < MAX_NESTED_IMPORTED_DECLARATIONS; ++num)
>         {
>           attr = dwarf2_attr (d, DW_AT_import, cu);
>           if (attr == NULL)
> -           break;
> +           return false;

This change doesn't seem to be needed.

> +             /* This declaration is a global variable alias. Add
> +                a symbol for it whose type is same as aliased variable.  */
> +             type = die_type (d, imported_cu);
> +             struct symbol *sym = new_symbol (die, type, cu);
> +             attr = dwarf2_attr (d, DW_AT_location, imported_cu);
> +             sym->set_aclass_index (LOC_UNRESOLVED);
> +             if (attr != nullptr)
> +               {
> +                 var_decode_location (attr, sym, cu);
> +               }

This is over-braced.

> +             /* Reset the flag as it is not a namespace alias.  */
> +             cu->processing_has_namespace_info = false;

This variable is super weird.  Once it is set to true, it is never
changed back.  In fact, setting it to false here probably will introduce
bugs.

I wonder if processing_has_namespace_info can just be removed.  cu.h
makes it sound extremely obsolete:

  /* When true, the file that we're processing is known to have
     debugging info for C++ namespaces.  GCC 3.3.x did not produce
     this information, but later versions do.  */

Anyway, I think you should just remove this line and the comment
referring to it.

Tom