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] PR preprocessor/42014


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.
From dd02fb235774ca8087e4781fa05557ac78b3cc36 Mon Sep 17 00:00:00 2001
From: Krzesimir Nowak <qdlacz@gmail.com>
Date: Sat, 18 Oct 2014 13:17:59 +0200
Subject: [PATCH v2] 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.
---
 gcc/ChangeLog                  |  7 +++++++
 gcc/diagnostic.c               | 12 +++---------
 gcc/testsuite/ChangeLog        |  6 ++++++
 gcc/testsuite/gcc.dg/pr42014.i | 24 ++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr42014.i

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 667da04..2bb7a49 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+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.
+
 2014-10-13  Marat Zakirov  <m.zakirov@samsung.com>
 
 	* asan.c (instrument_derefs): BIT_FIELD_REF added.
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 881da0b..9a22d46 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -525,15 +525,9 @@ diagnostic_report_current_module (diagnostic_context *context, location_t where)
       if (! MAIN_FILE_P (map))
 	{
 	  map = INCLUDED_FROM (line_table, map);
-	  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));
-	  else
-	    pp_verbatim (context->printer,
-			 "In file included from %r%s:%d%R", "locus",
-			 LINEMAP_FILE (map), LAST_SOURCE_LINE (map));
+	  pp_verbatim (context->printer,
+		       "In file included from %r%s:%d%R", "locus",
+		       LINEMAP_FILE (map), LAST_SOURCE_LINE (map));
 	  while (! MAIN_FILE_P (map))
 	    {
 	      map = INCLUDED_FROM (line_table, map);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2134ada..d2936c4 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+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.
+
 2014-09-19  Marat Zakirov  <m.zakirov@samsung.com>
 
 	* c-c++-common/asan/bitfield-5.c: New test.
diff --git a/gcc/testsuite/gcc.dg/pr42014.i b/gcc/testsuite/gcc.dg/pr42014.i
new file mode 100644
index 0000000..b600de0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr42014.i
@@ -0,0 +1,24 @@
+/* PR preprocessor/42014   Inconsistent column number display for "In file included from" */
+/* { dg-do compile } */
+/* { dg-options "-fshow-column" } */
+# 1 "gcc/testsuite/gcc.dg/pr42014.c"
+# 1 "<built-in>"
+# 1 "<command-line>"
+# 1 "/usr/include/stdc-predef.h" 1 3 4
+# 1 "<command-line>" 2
+# 1 "gcc/testsuite/gcc.dg/pr42014.c"
+
+
+
+# 1 "gcc/testsuite/gcc.dg/pr42014-1.h" 1
+
+
+# 1 "gcc/testsuite/gcc.dg/pr42014-2.h" 1
+
+# 1 "gcc/testsuite/gcc.dg/pr42014-3.h" 1
+not_declared_yet();
+# 2 "gcc/testsuite/gcc.dg/pr42014-2.h" 2
+# 3 "gcc/testsuite/gcc.dg/pr42014-1.h" 2
+# 5 "gcc/testsuite/gcc.dg/pr42014.c" 2
+/* { dg-message "n file included from gcc/testsuite/gcc.dg/pr42014-2.h:2,\n *from gcc/testsuite/gcc.dg/pr42014-1.h:3,\n *from gcc/testsuite/gcc.dg/pr42014.c:4:" "include chain" { target *-*-* } 0 } */
+/* { dg-prune-output "warning" } */
-- 
1.9.3


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