This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Caret diagnostics


On 7 April 2012 00:04, Jason Merrill <jason@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.

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]