* [PATCH] PR preprocessor/42014
@ 2014-10-18 21:07 Krzesimir Nowak
2014-10-18 21:35 ` [PATCH] Fix " Krzesimir Nowak
2014-10-20 8:09 ` [PATCH] " Krzesimir Nowak
0 siblings, 2 replies; 6+ messages in thread
From: Krzesimir Nowak @ 2014-10-18 21:07 UTC (permalink / raw)
To: gcc-patches; +Cc: Krzesimir Nowak
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.
Also, do I need to run make -k check for both vanilla and changed GCC
to compare the results? These tests take ages to complete, so maybe
there is some subset of tests which is enough for regression checking
in this case? Currently I am only running following command in gcc
directory:
make check-c++ RUNTESTFLAGS="-v dg.exp=cpp/pr42014.c"
Krzesimir Nowak (1):
Fix PR preprocessor/42014
gcc/ChangeLog | 6 ++++++
gcc/diagnostic.c | 27 +++++++++++++++------------
gcc/testsuite/ChangeLog | 8 ++++++++
gcc/testsuite/c-c++-common/cpp/pr42014-1.h | 3 +++
gcc/testsuite/c-c++-common/cpp/pr42014-2.h | 2 ++
gcc/testsuite/c-c++-common/cpp/pr42014-3.h | 1 +
gcc/testsuite/c-c++-common/cpp/pr42014.c | 8 ++++++++
7 files changed, 43 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-1.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-2.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-3.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014.c
--
1.9.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Fix PR preprocessor/42014
2014-10-18 21:07 [PATCH] PR preprocessor/42014 Krzesimir Nowak
@ 2014-10-18 21:35 ` Krzesimir Nowak
2014-10-19 16:18 ` Joseph S. Myers
2014-10-20 8:09 ` [PATCH] " Krzesimir Nowak
1 sibling, 1 reply; 6+ messages in thread
From: Krzesimir Nowak @ 2014-10-18 21:35 UTC (permalink / raw)
To: gcc-patches; +Cc: Krzesimir Nowak
gcc/Changelog:
2014-10-18 Krzesimir Nowak <qdlacz@gmail.com>
* diagnostic.c (diagnostic_report_from): New function. It prints
one line from include chain.
(diagnostic_report_current_module): Use the above function.
gcc/testsuite/ChangeLog:
2014-10-18 Krzesimir Nowak <qdlacz@gmail.com>
* c-c++-common/cpp/pr42014.c: New.
* c-c++-common/cpp/pr42014-1.h: New.
* c-c++-common/cpp/pr42014-2.h: New.
* c-c++-common/cpp/pr42014-3.h: New.
---
gcc/ChangeLog | 6 ++++++
gcc/diagnostic.c | 27 +++++++++++++++------------
gcc/testsuite/ChangeLog | 8 ++++++++
gcc/testsuite/c-c++-common/cpp/pr42014-1.h | 3 +++
gcc/testsuite/c-c++-common/cpp/pr42014-2.h | 2 ++
gcc/testsuite/c-c++-common/cpp/pr42014-3.h | 1 +
gcc/testsuite/c-c++-common/cpp/pr42014.c | 8 ++++++++
7 files changed, 43 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-1.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-2.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-3.h
create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 667da04..421dd47 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-18 Krzesimir Nowak <qdlacz@gmail.com>
+
+ * diagnostic.c (diagnostic_report_from): New function. It prints
+ one line from include chain.
+ (diagnostic_report_current_module): Use the above function.
+
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..f0ac8ca 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -501,6 +501,19 @@ diagnostic_action_after_output (diagnostic_context *context,
}
}
+static void
+diagnostic_report_from (diagnostic_context *context,
+ const struct line_map *map,
+ const char *prefix)
+{
+ /* 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. */
+ pp_verbatim (context->printer,
+ "%s from %r%s:%d%R", prefix, "locus",
+ LINEMAP_FILE (map), LAST_SOURCE_LINE (map));
+}
+
void
diagnostic_report_current_module (diagnostic_context *context, location_t where)
{
@@ -525,21 +538,11 @@ 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));
+ diagnostic_report_from (context, map, "In file included");
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));
+ diagnostic_report_from (context, map, ",\n ");
}
pp_verbatim (context->printer, ":");
pp_newline (context->printer);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2134ada..fabdf7c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2014-10-18 Krzesimir Nowak <qdlacz@gmail.com>
+
+ PR preprocessor/42014
+ * c-c++-common/cpp/pr42014.c: New.
+ * c-c++-common/cpp/pr42014-1.h: New.
+ * c-c++-common/cpp/pr42014-2.h: New.
+ * c-c++-common/cpp/pr42014-3.h: New.
+
2014-09-19 Marat Zakirov <m.zakirov@samsung.com>
* c-c++-common/asan/bitfield-5.c: New test.
diff --git a/gcc/testsuite/c-c++-common/cpp/pr42014-1.h b/gcc/testsuite/c-c++-common/cpp/pr42014-1.h
new file mode 100644
index 0000000..33f3b44
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/pr42014-1.h
@@ -0,0 +1,3 @@
+
+
+ #include "pr42014-2.h"
diff --git a/gcc/testsuite/c-c++-common/cpp/pr42014-2.h b/gcc/testsuite/c-c++-common/cpp/pr42014-2.h
new file mode 100644
index 0000000..8ddce8e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/pr42014-2.h
@@ -0,0 +1,2 @@
+
+ #include "pr42014-3.h"
diff --git a/gcc/testsuite/c-c++-common/cpp/pr42014-3.h b/gcc/testsuite/c-c++-common/cpp/pr42014-3.h
new file mode 100644
index 0000000..75dcf7c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/pr42014-3.h
@@ -0,0 +1 @@
+int f(foo bar);
diff --git a/gcc/testsuite/c-c++-common/cpp/pr42014.c b/gcc/testsuite/c-c++-common/cpp/pr42014.c
new file mode 100644
index 0000000..ebddf96
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/pr42014.c
@@ -0,0 +1,8 @@
+/* PR preprocessor/42014 */
+/* { dg-do compile } */
+
+#include "pr42014-1.h"
+
+/* { dg-excess-errors "In file included from .*pr42014-2.h:2," } */
+/* { dg-excess-errors " from .*pr42014-1.h:3," } */
+/* { dg-excess-errors " from .*pr42014.c:4:" } */
--
1.9.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR preprocessor/42014
2014-10-18 21:35 ` [PATCH] Fix " Krzesimir Nowak
@ 2014-10-19 16:18 ` Joseph S. Myers
0 siblings, 0 replies; 6+ messages in thread
From: Joseph S. Myers @ 2014-10-19 16:18 UTC (permalink / raw)
To: Krzesimir Nowak; +Cc: gcc-patches
On Sat, 18 Oct 2014, Krzesimir Nowak wrote:
> + pp_verbatim (context->printer,
> + "%s from %r%s:%d%R", prefix, "locus",
> + diagnostic_report_from (context, map, "In file included");
We don't want to split up diagnostic text like that, because for
translation it may be necessary to translate the whole "In file included
from" text together rather than expecting two fragments to go together in
the same way they do in English. Now, right now this message isn't marked
for translation anyway (an independent bug there's no need for you to
fix), but still as a design principle things should be structured to avoid
splitting up English fragments like that.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR preprocessor/42014
2014-10-18 21:07 [PATCH] PR preprocessor/42014 Krzesimir Nowak
2014-10-18 21:35 ` [PATCH] Fix " Krzesimir Nowak
@ 2014-10-20 8:09 ` Krzesimir Nowak
1 sibling, 0 replies; 6+ messages in thread
From: Krzesimir Nowak @ 2014-10-20 8:09 UTC (permalink / raw)
To: gcc-patches; +Cc: Krzesimir Nowak
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.
>
> Also, do I need to run make -k check for both vanilla and changed GCC
> to compare the results? These tests take ages to complete, so maybe
> there is some subset of tests which is enough for regression checking
> in this case? Currently I am only running following command in gcc
> directory:
> make check-c++ RUNTESTFLAGS="-v dg.exp=cpp/pr42014.c"
>
> Krzesimir Nowak (1):
> Fix PR preprocessor/42014
>
> gcc/ChangeLog | 6 ++++++
> gcc/diagnostic.c | 27 +++++++++++++++------------
> gcc/testsuite/ChangeLog | 8 ++++++++
> gcc/testsuite/c-c++-common/cpp/pr42014-1.h | 3 +++
> gcc/testsuite/c-c++-common/cpp/pr42014-2.h | 2 ++
> gcc/testsuite/c-c++-common/cpp/pr42014-3.h | 1 +
> gcc/testsuite/c-c++-common/cpp/pr42014.c | 8 ++++++++
> 7 files changed, 43 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-1.h
> create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-2.h
> create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014-3.h
> create mode 100644 gcc/testsuite/c-c++-common/cpp/pr42014.c
>
> --
> 1.9.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR preprocessor/42014
2014-10-20 14:19 Manuel López-Ibáñez
@ 2014-10-20 21:17 ` Krzesimir Nowak
0 siblings, 0 replies; 6+ messages in thread
From: Krzesimir Nowak @ 2014-10-20 21:17 UTC (permalink / raw)
To: Manuel López-Ibáñez; +Cc: Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 5308 bytes --]
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.
[-- Attachment #2: v2-0001-Fix-PR-preprocessor-42014.patch --]
[-- Type: text/x-patch, Size: 3865 bytes --]
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR preprocessor/42014
@ 2014-10-20 14:19 Manuel López-Ibáñez
2014-10-20 21:17 ` Krzesimir Nowak
0 siblings, 1 reply; 6+ messages in thread
From: Manuel López-Ibáñez @ 2014-10-20 14:19 UTC (permalink / raw)
To: Gcc Patch List, Krzesimir Nowak
> 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.
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-20 20:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-18 21:07 [PATCH] PR preprocessor/42014 Krzesimir Nowak
2014-10-18 21:35 ` [PATCH] Fix " Krzesimir Nowak
2014-10-19 16:18 ` Joseph S. Myers
2014-10-20 8:09 ` [PATCH] " Krzesimir Nowak
2014-10-20 14:19 Manuel López-Ibáñez
2014-10-20 21:17 ` Krzesimir Nowak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).