[PATCH] PR preprocessor/42014

[Krzesimir Nowak]

2014-10-20 16:11 GMT+02:00 Manuel López-Ibáñez <lopezibanez@gmail.com>:
>> 2014-10-18 23:07 GMT+02:00 Krzesimir Nowak <qdlacz@gmail.com>:
>>> Hello.
>>>
>>> This is my first patch for GCC. I already started a paperwork for
>>> copyright assignment (sent an email to fsf-records at gnu org) -
>>> waiting for response.
>>>
>>> So, about this patch - it basically removes column printing from "In
>>> file included from ..." lines, as the column information always
>>> returned 0. Not sure if this is correct assumption - I tested only C
>>> and C++, so I don't know if other frontends (ada, go?) provide column
>>> information for include lines. Anyway, column information here is
>>> probably not useful.
>>>
>>> Or maybe it is, if GCC supports some language with include syntax like
>>> followish:
>>> #include <header_1.h>, <header_2.h>, <header_3.h>
>>>
>>> Maybe in this case printing column number has sense?
>>>
>>> I need help with testcase - I don't know how to implement it
>>> correctly. The output of compilation is something like this:
>>>
>>> In file included from .../pr42014-2.h:2,
>>>                  from .../pr42014-1.h:3,
>>>                  from .../pr42014.c:4:
>>> .../pr42014-3.h:1:7: error: 'foo' was not declared in this scope
>>>
>>> How to check the "from" lines? Is there some dg-foo (dg-grep?) command
>>> for it? dg-excess-errors is likely not suited for this purpose.
>>
>> I suppose I will have to add a preprocessed file and try using dg-message.
>
> Hi Krzesimir,
>
> I think you are overcomplicating it. The original reporter complained
> simply that there is an inconsistency between the first line and the
> next ones when -fshow-column is enabled (which is now the default but
> it wasn't some years ago). The following patch is sufficient to fix
> this inconsistency:
>
> Index: diagnostic.c
> ===================================================================
> --- diagnostic.c    (revision 216462)
> +++ diagnostic.c    (working copy)
> @@ -528,8 +528,8 @@
>        if (context->show_column)
>          pp_verbatim (context->printer,
>               "In file included from %r%s:%d:%d%R", "locus",
> -             LINEMAP_FILE (map),
> -             LAST_SOURCE_LINE (map), LAST_SOURCE_COLUMN (map));
> +             LINEMAP_FILE (map), LAST_SOURCE_LINE (map),
> +             LAST_SOURCE_COLUMN (map));
>        else
>          pp_verbatim (context->printer,
>               "In file included from %r%s:%d%R", "locus",
> @@ -537,9 +537,15 @@
>        while (! MAIN_FILE_P (map))
>          {
>            map = INCLUDED_FROM (line_table, map);
> -          pp_verbatim (context->printer,
> -               ",\n                 from %r%s:%d%R", "locus",
> -               LINEMAP_FILE (map), LAST_SOURCE_LINE (map));
> +          if (context->show_column)
> +        pp_verbatim (context->printer,
> +                 ",\n                 from %r%s:%d:%d%R", "locus",
> +                 LINEMAP_FILE (map), LAST_SOURCE_LINE (map),
> +                 LAST_SOURCE_COLUMN (map));
> +          else
> +        pp_verbatim (context->printer,
> +                 ",\n                 from %r%s:%d%R", "locus",
> +                 LINEMAP_FILE (map), LAST_SOURCE_LINE (map));
>          }
>        pp_verbatim (context->printer, ":");
>        pp_newline (context->printer);
>
> You can test this by simply building gcc and using -fshow-column vs.
> -fno-show-column. I think a testsuite testcase will be hard to build
> because DejaGNU. It doesn't seem worth the effort for such a minor
> issue. Given that you seem to have enough knowledge and ability to
> modify GCC and submit good patches, it would be better to spend your
> time on more important bugs.
>

Hello Manuel,

I already made another version of the patch. A simpler one, but
different than yours - I removed column printing altogether and added
the test. The rationale for removing column printing was, as Andrew
Pinski said [1], that column should be printed if we want it and if
this information is available.

And the test - it is ugly indeed. But well, works and I have learned a tiny bit.

Please see attached patch.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42014#c1



Fix PR preprocessor/42014

gcc/Changelog:

2014-10-20  Krzesimir Nowak  <qdlacz@gmail.com>

* diagnostic.c (diagnostic_report_current_module): Do not print
column, it is always zero. Also, it's quite pointless - it does
not give any useful information as there can be only one include
directive per line.

gcc/testsuite/ChangeLog:

2014-10-20  Krzesimir Nowak  <qdlacz@gmail.com>

PR preprocessor/42014
* gcc.dg/pr42014.i: New. Not in cpp subdirectory, because .i files
are not processed here.

> For example, this one needs to be analyzed, we don't even know how it happens:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52998
>
> Or this one, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45333
> which I think is just a matter of adding (or factoring out) some of
> the logic from maybe_unwind_expanded_macro_loc() and use it in various
> places in cp/error.c (print_instantiation_*).
>
> If you are not motivated by those, I can offer more suggestions...
>
> Cheers,
>
> Manuel.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-Fix-PR-preprocessor-42014.patch
Type: text/x-patch
Size: 3865 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20141020/98056153/attachment.bin>