Re: [PATCH 1/2] namespace support

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:

Sami> Currently imports statements are stored in the nearest static
Sami> block. Also, these import statements only correspond to the
Sami> implicit importing of annonymous namespaces.

This looks pretty reasonable to me.
A few nits and such...

Sami> +/* This returns the using directives list associated to BLOCK, if
Sami> +   any.  Each BLOCK_NAMESPACE()->USING already contains all the namespaces
Sami> +   imported at that code point - even those from its parent blocks.  */

I think the caps on "USING" here are weird.
Maybe just:

    The result contains all the [...]

Sami> +/* using directives local to lexical context */

Comments should in general be sentences -- starting with a capital
letter and ending with a period and two spaces.
Here I would probably write:

  /* "using" directives local to lexical context.  */

... avoiding the capital since "using" is a keyword.

Sami> +EXTERN struct using_direct *using_directives;

Eww, buildsym is full of globals.  Not your problem :-), but yet
another thing that would be nice to clean up.

Sami> +    /* Pending using directives at the time we entered */

Likewise about the period and spaces.

Sami> -  using_list = cp_add_using (name, inner_length, outer_length,
Sami> -			     using_list);
Sami> +  using_directives = cp_add_using (name, inner_length, outer_length,
Sami> +      using_directives);

Looks like a formatting change; the old code was indented correctly.

Sami> +static void
Sami> +read_import_statement (struct die_info *die, struct dwarf2_cu *cu)

[...]

Sami> +  if (imported_name == NULL)
Sami> +    {
Sami> +      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */

It would be nice to understand this before the patch goes in.

Sami> +  /* FIXME: dwarf2_name (die); for the local name after import.  */

It would also be nice if we could not add new FIXMEs.
Or is that handled in a later patch?

Sami> +  using_directives = cp_add_using (imported_name, strlen (imported_name), 0,
Sami> +                                   using_directives);
Sami> +}

I didn't look -- but are there other buildsym globals which are
actively updated by the dwarf reader like this?  I'm wondering whether
this is something that should be hidden inside a call, like a new
wrapper for cp_add_using.

This is not a big deal.

Tom