Re: [PATCH 3/7] Emit macro expansion related diagnostics

[Jason Merrill <jason@redhat.com>]

On 08/04/2011 11:32 AM, Dodji Seketeli wrote:
> +++ b/gcc/diagnostic.c
> @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "input.h"
>  #include "intl.h"
>  #include "diagnostic.h"
> +#include "vec.h"

Do you still need this?

>  // Just discard errors pointing at header files
>  // { dg-prune-output "include" }
> +// { dg-prune-output "        from" }

These should be pruned by testsuite/lib/prune.exp.  I'm surprised they 
aren't already.

> +#define APPEND_LOC_TO_VEC(LOC)                                         \
> +  if (num_locs >=3D loc_vec_capacity)                                  \
> +    {                                                                  \
> +      loc_vec_capacity +=3D 4;                                         \
> +      loc_vec =3D XRESIZEVEC (loc_t, loc_vec, loc_vec_capacity);
> \
> +    }                                                                  \
> +  loc_vec[num_locs++] =3D LOC;

Why not use VEC since we're in gcc/ here?

> +/* Unwind the different macro expansions that lead to the token which
> +   location is WHERE and emit diagnostics showing the resulting
> +   unwound macro expansion stack.  If TOPMOST_EXP_POINT_MAP is
> +   non-null, *TOPMOST_EXP_POINT_MAP is set to the map of the expansion
> +   point of the top most macro of the stack.  This must be an ordinary
> +   map.  */

I find the use of "top" here confusing.  You mean the place in the 
source that first triggered the macro expansion, right?  Can we avoid 
talking about stacks here?

> +  /* Walk the map_vec and print the macro expansion stack, unless the
> +     topmost macro which expansion triggered this stack [assuming the
> +     stack grows downwards] was expanded inside a system header.  */

Abstract stacks are said to grow upwards; only the representation of a 
call stack on certain architectures can grow downwards.  The top of the 
stack is the most recently pushed element, which in the case of macro 
expansion would be the innermost macro expanded.

> +  while (unwind)
> +    {
...
> +      if (!linemap_macro_expansion_map_p (map))
> +       unwind =3D false;
> +    }

This seems like a job for do/while.

> +       diagnostic->location =3D resolved_def_loc;
> +       pp_base_set_prefix (context->printer,
> +                           diagnostic_build_prefix (context,
> +                                                    diagnostic));

Using pp_verbatim like in maybe_print_constexpr_context seems simpler to 
me, but if you prefer this approach that's fine.

Jason