On 7 April 2012 00:04, Jason Merrill <ja...@redhat.com> wrote:
> On 04/06/2012 04:11 AM, Manuel López-Ibáńez wrote:
>> +++ gcc/testsuite/gcc.dg/torture/tls/tls.exp    (working copy)
>> @@ -48,10 +48,10 @@ dg-init
>>  torture-init
>>  set-torture-options $TLS_TORTURE_OPTIONS {{}} $LTO_TORTURE_OPTIONS
>>  # Main loop.
>>  gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
>> -        $DEFAULT_CFLAGS
>> +        "-fno-diagnostics-show-caret $DEFAULT_CFLAGS"
> I don't think this is needed since you're already adding it to
> TEST_ALWAYS_FLAGS.  The tests pass without this for me.

Good. I will try without, but the output does not show the flag two
times, which make me think that tls.exp is not using
TEST_ALWAYS_FLAGS. But maybe I was looking at the wrong place.

>> +  int max_width = 80, right_margin = 10;
> Let's not hardcode these numbers.  We should use getenv("COLUMNS") for the

Fine, I'll try that.

> width if it's set; otherwise I would lean toward unlimited width. And I'm
> not sure why we need a right margin at all.

The right margin is because:

* We don't want to do wrapping. Otherwise the caret looks strange. So
either we cut long lines or we leave the maximum width unlimited.
* If we cut long lines, and the line is too long and the caret is too
close to the right margin, it is better to cut the line at the
beginning to show the part pointed by the caret within the maximum
width, with some margin to the right. That is, we don't want to show:

 very_very_long_line with something w
We want to show:

 long_line with something wrong on it

>> +  s = expand_location(diagnostic->location);
>> +  line = location_get_source_line (s);
> Why not expand the location inside location_get_source_line?

Well, we have to expand outside anyway, because I need the column
info, so it seemed a waste to expand twice. Do you have a strong
preference for expanding twice? I don't care so much either way.



Reply via email to