2014-10-20 16:11 GMT+02:00 Manuel López-Ibáñez <lopeziba...@gmail.com>:
>> 2014-10-18 23:07 GMT+02:00 Krzesimir Nowak <qdl...@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 <qdl...@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 <qdl...@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 <qdl...@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 <qdl...@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 <qdl...@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 <qdl...@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.zaki...@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 <qdl...@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.zaki...@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
