> The warning was attributed to the wrong lineno. Looks like
> discriminator does not work well when it coexists with macros.
I think the problem is with same_line_p. It's using expand_location to
test whether two locations refer to the same line, but expand_location
always unwinds the macro stack so that it's looking at the line number
of the macro expansion point. That means that every token in the macro
expansion will appear to be at the same line, so the loop over the
basic_block will replace all of the locations with the new_locus that
we just allocated for the new discriminator. Thus, it's clobbering the
locus for the instructions at line 27 in the macro definition with the
new locus we create to put a discriminator at line 26, and the warning
that should appear for line 27 now says line 26.
Other comments on the patch...
In expand_location_1:
+ if (min_discriminator_location != UNKNOWN_LOCATION
+ && loc >= min_discriminator_location
+ && loc < min_discriminator_location + next_discriminator_location)
The last comparison should just be "loc < next_discriminator_location".
Likewise in has_discriminator.
In assign_discriminator:
+ location_t new_locus = location_with_discriminator (locus,
discriminator);
+ gimple_stmt_iterator gsi;
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple stmt = gsi_stmt (gsi);
+ if (same_line_p (locus, gimple_location (stmt)))
+ gimple_set_location (stmt, block ?
+ COMBINE_LOCATION_DATA (line_table, new_locus, block) :
+ LOCATION_LOCUS (new_locus));
+ }
I'm not convinced the COMBINE_LOCATION_DATA is needed here, since
location_with_discriminator has already done that. And when block ==
NULL, calling LOCATION_LOCUS is effectively a no-op, isn't it? It
seems this could simply be:
gimple_set_location (stmt, new_locus);
Also, you need to add vec.o to OBJS-libcommon in Makefile.in, since
input.o now depends on it. Other binaries, like gcov, won't build
otherwise.
I'm still not happy with a straightforward port of the old patch,
though, since discriminator assignment might allocate locations that
overlap those used for macro definitions -- there's no checking to see
if we've run out of locations in that case. I guess this could be OK
for the google branch for now, but not for trunk. I'm looking at
alternatives using the adhoc mapping -- do you think it would work if
we extend the adhoc mapping to track both a "data" (i.e., the block
pointer) and a discriminator?
-cary
