Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/24/2012 03:18 PM, Dodji Seketeli wrote: Like the below? Yep, thanks. The patch is OK. Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes: On 05/22/2012 05:04 AM, Dodji Seketeli wrote: The problem is that cpp_get_token_1 can be called when we are at the beginning of a macro expansion (inside enter_macro_expansion, called from cpp_get_token_1), *before* context-c.macro is set. This happens e.g, when we call funlike_invocation_p to know if the current macro is function-like or not. OK, sounds like we need some additional code to handle that. I guess we could do something in funlike_invocation_p to prevent cpp_get_token_1 from setting invocation_location, or change the check we use to decide whether or not we already have an invocation location, perhaps by looking at invocation_location itself (and clearing it when we finish a macro). So. After thinking about this a bit more, here is how I phrase the issue. There is a small interval of time between when we decide to start the expansion of a macro (when we get into enter_macro_context), and when we really start that expansion (when push the context of macro the macro) where we can recursively call cpp_get_token_1. In that time interval, cpp_get_token_1 might wrongly think that no macro expansion is in progress. And I think that's the issue root issue over which the cpp_reader::set_invocation_location flag was papering. It seems to me that a small and maintainable option to tackle this is to introduce a flag cpp_reader::about_to_expand_macro_p, that is 'on' during that time interval. A new predicate function in_macro_expansion_p then can now accurately tell cpp_get_token_1 if we are in the process of expanding a macro or not, doing away with the need for casual users of cpp_get_token_1 to set a flag to deal with macro expansion point handling. Tested and bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. libcpp/ PR preprocessor/53229 * internal.h (cpp_reader::set_invocation_location): Remove. (cpp_reader::about_to_expand_macro_p): New member flag. * directives.c (do_pragma): Remove Kludge as pfile-set_invocation_location is no more. * macro.c (cpp_get_token_1): Do away with the use of cpp_reader::set_invocation_location. Just collect the macro expansion point when we are about to expand the top-most macro. Do not override cpp_reader::about_to_expand_macro_p. This fixes gcc.dg/cpp/paste12.c by making get_token_no_padding properly handle locations of expansion points. (cpp_get_token_with_location): Adjust, as cpp_reader::set_invocation_location is no more. (paste_tokens): Take a virtual location parameter for the LHS of the pasting operator. Use it in diagnostics. Update comments. (paste_all_tokens): Tighten the assert. Propagate the location of the expansion point when no virtual locations are available. Pass the virtual location to paste_tokens. (in_macro_expansion_p): New static function. (enter_macro_context): Set the cpp_reader::about_to_expand_macro_p flag until we really start expanding the macro. (_cpp_push_token_context, push_ptoken_context) (push_extended_tokens_context): Unset the cpp_reader::about_to_expand_macro_p flag when we push the macro context. gcc/testsuite/ PR preprocessor/53229 * gcc.dg/cpp/paste6.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste8.c: Likewise. * gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste12.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste13.c: Likewise. * gcc.dg/cpp/paste14.c: Likewise. * gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste18.c: New test. --- gcc/testsuite/gcc.dg/cpp/paste12-2.c | 11 + gcc/testsuite/gcc.dg/cpp/paste12.c |5 ++- gcc/testsuite/gcc.dg/cpp/paste13.c |5 ++- gcc/testsuite/gcc.dg/cpp/paste14-2.c | 11 + gcc/testsuite/gcc.dg/cpp/paste14.c |5 ++- gcc/testsuite/gcc.dg/cpp/paste18.c | 16 +++ gcc/testsuite/gcc.dg/cpp/paste6.c|5 ++- gcc/testsuite/gcc.dg/cpp/paste8-2.c | 15 ++ gcc/testsuite/gcc.dg/cpp/paste8.c|2 +- libcpp/directives.c | 30 + libcpp/internal.h| 10 +++- libcpp/macro.c | 82 ++--- 12 files changed, 142 insertions(+), 55 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/paste12-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste14-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste18.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste8-2.c diff --git a/gcc/testsuite/gcc.dg/cpp/paste12-2.c
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/24/2012 12:03 PM, Dodji Seketeli wrote: + if (macro != NULL) +pfile-about_to_expand_macro_p = false; The approach sounds good, but why do this in the push_token_context functions rather than in enter_macro_context? Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes: On 05/24/2012 12:03 PM, Dodji Seketeli wrote: + if (macro != NULL) +pfile-about_to_expand_macro_p = false; The approach sounds good, but why do this in the push_token_context functions rather than in enter_macro_context? Because, it's not only in enter_macro_context that I'd have to put it, but also in replace_args, called from enter_macro_context, for instance. Another way of seeing it is to say that, from the beginning of enter_macro_context, we are in a state of about to expand a macro until we actually push the macro context. So it seems to make sense to flip the flag where the the macro context push happens, IMHO. -- Dodji
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/24/2012 01:41 PM, Dodji Seketeli wrote: Jason Merrillja...@redhat.com writes: The approach sounds good, but why do this in the push_token_context functions rather than in enter_macro_context? Because, it's not only in enter_macro_context that I'd have to put it, but also in replace_args, called from enter_macro_context, for instance. Why? Another way of seeing it is to say that, from the beginning of enter_macro_context, we are in a state of about to expand a macro until we actually push the macro context. So it seems to make sense to flip the flag where the the macro context push happens, IMHO. Sure, that makes sense. It just seems better to me to have the flag set a little longer than necessary in order to keep the setting and unsetting closer together for maintainability. Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes:
On 05/24/2012 01:41 PM, Dodji Seketeli wrote:
[...]
Another way of seeing it is to say that, from the beginning of
enter_macro_context, we are in a state of about to expand a macro
until we actually push the macro context. So it seems to make sense to
flip the flag where the the macro context push happens, IMHO.
Sure, that makes sense. It just seems better to me to have the flag
set a little longer than necessary in order to keep the setting and
unsetting closer together for maintainability.
Like the below? Bootstrap is under way.
libcpp/
PR preprocessor/53229
* internal.h (cpp_reader::set_invocation_location): Remove.
(cpp_reader::about_to_expand_macro_p): New member flag.
* directives.c (do_pragma): Remove Kludge as
pfile-set_invocation_location is no more.
* macro.c (cpp_get_token_1): Do away with the use of
cpp_reader::set_invocation_location. Just collect the macro
expansion point when we are about to expand the top-most macro.
Do not override cpp_reader::about_to_expand_macro_p.
This fixes gcc.dg/cpp/paste12.c by making get_token_no_padding
properly handle locations of expansion points.
(cpp_get_token_with_location): Adjust, as
cpp_reader::set_invocation_location is no more.
(paste_tokens): Take a virtual location parameter for
the LHS of the pasting operator. Use it in diagnostics. Update
comments.
(paste_all_tokens): Tighten the assert. Propagate the location of
the expansion point when no virtual locations are available.
Pass the virtual location to paste_tokens.
(in_macro_expansion_p): New static function.
(enter_macro_context): Set the cpp_reader::about_to_expand_macro_p
flag until we really start expanding the macro.
gcc/testsuite/
PR preprocessor/53229
* gcc.dg/cpp/paste6.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste8.c: Likewise.
* gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste13.c: Likewise.
* gcc.dg/cpp/paste14.c: Likewise.
* gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste18.c: New test.
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12-2.c
b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
new file mode 100644
index 000..6e2e4f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
@@ -0,0 +1,11 @@
+/*
+ { dg-options -ftrack-macro-expansion=2 }
+ { dg-do preprocess }
+ */
+
+/* Test correct diagnostics when pasting in #include.
+ Source: PR preprocessor/6780. */
+
+#define inc2(a,b) ##a.b /* { dg-error pasting \\ and \stdio\ does
not } */
+#define INC(X) inc2(X,h)
+#include INC(stdio)
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12.c
b/gcc/testsuite/gcc.dg/cpp/paste12.c
index e61ec51..3e0f7b9 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste12.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste12.c
@@ -1,4 +1,7 @@
-/* { dg-do preprocess } */
+/*
+ { dg-options -ftrack-macro-expansion=0 }
+ { dg-do preprocess }
+*/
/* Test correct diagnostics when pasting in #include.
Source: PR preprocessor/6780. */
diff --git a/gcc/testsuite/gcc.dg/cpp/paste13.c
b/gcc/testsuite/gcc.dg/cpp/paste13.c
index 62c72d4..f0f4fd8 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste13.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste13.c
@@ -1,6 +1,9 @@
/* Copyright (C) 2000 Free Software Foundation, Inc. */
-/* { dg-do preprocess } */
+/*
+ { dg-options -ftrack-macro-expansion=0 }
+ { dg-do preprocess }
+*/
/* This used to be recognized as a comment when lexing after pasting
spellings. Neil Booth, 9 Oct 2002. */
diff --git a/gcc/testsuite/gcc.dg/cpp/paste14-2.c
b/gcc/testsuite/gcc.dg/cpp/paste14-2.c
new file mode 100644
index 000..3b23ada
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/paste14-2.c
@@ -0,0 +1,11 @@
+/* PR preprocessor/28709 */
+/*
+ { dg-options -ftrack-macro-expansion=2 }
+ { dg-do preprocess }
+*/
+
+#define foo - ## /* { dg-error pasting \-\ and \\ } */
+foo
+#define bar = ## == /* { dg-error pasting \=\ and \==\ } */
+bar
+
diff --git a/gcc/testsuite/gcc.dg/cpp/paste14.c
b/gcc/testsuite/gcc.dg/cpp/paste14.c
index ec243c2..043d5e5 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste14.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste14.c
@@ -1,5 +1,8 @@
/* PR preprocessor/28709 */
-/* { dg-do preprocess } */
+/*
+ { dg-options -ftrack-macro-expansion=0 }
+ { dg-do preprocess }
+*/
#define foo - ##
foo/* { dg-error pasting \-\ and \\ } */
diff --git a/gcc/testsuite/gcc.dg/cpp/paste18.c
b/gcc/testsuite/gcc.dg/cpp/paste18.c
new
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Dodji Seketeli do...@redhat.com writes: Jason Merrill ja...@redhat.com writes: On 05/24/2012 01:41 PM, Dodji Seketeli wrote: [...] Another way of seeing it is to say that, from the beginning of enter_macro_context, we are in a state of about to expand a macro until we actually push the macro context. So it seems to make sense to flip the flag where the the macro context push happens, IMHO. Sure, that makes sense. It just seems better to me to have the flag set a little longer than necessary in order to keep the setting and unsetting closer together for maintainability. Like the below? Bootstrap is under way. FWIW, the patch passed bootstrap and testing on x86_64-unknown-linux-gnu against trunk. libcpp/ PR preprocessor/53229 * internal.h (cpp_reader::set_invocation_location): Remove. (cpp_reader::about_to_expand_macro_p): New member flag. * directives.c (do_pragma): Remove Kludge as pfile-set_invocation_location is no more. * macro.c (cpp_get_token_1): Do away with the use of cpp_reader::set_invocation_location. Just collect the macro expansion point when we are about to expand the top-most macro. Do not override cpp_reader::about_to_expand_macro_p. This fixes gcc.dg/cpp/paste12.c by making get_token_no_padding properly handle locations of expansion points. (cpp_get_token_with_location): Adjust, as cpp_reader::set_invocation_location is no more. (paste_tokens): Take a virtual location parameter for the LHS of the pasting operator. Use it in diagnostics. Update comments. (paste_all_tokens): Tighten the assert. Propagate the location of the expansion point when no virtual locations are available. Pass the virtual location to paste_tokens. (in_macro_expansion_p): New static function. (enter_macro_context): Set the cpp_reader::about_to_expand_macro_p flag until we really start expanding the macro. gcc/testsuite/ PR preprocessor/53229 * gcc.dg/cpp/paste6.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste8.c: Likewise. * gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste12.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste13.c: Likewise. * gcc.dg/cpp/paste14.c: Likewise. * gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste18.c: New test. Thanks. -- Dodji
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes: On 05/21/2012 10:07 AM, Dodji Seketeli wrote: Jason Merrillja...@redhat.com writes: When do we not want to set invocation_location if we're beginning to expand a macro? If M itself involves expanding another macro M', we are supposed to call the (internal function) cpp_get_token_1 that does not set set_invocation_location. So that the only expansion point recorded is the one for M, not the one for M'. But if we're already in a macro expansion (if context-c.macro is set) cpp_get_token_1 doesn't set invocation_location even if set_invocation_location is true. So we should only get the expansion point for M even if set_invocation_location is always true. The problem is that cpp_get_token_1 can be called when we are at the beginning of a macro expansion (inside enter_macro_expansion, called from cpp_get_token_1), *before* context-c.macro is set. This happens e.g, when we call funlike_invocation_p to know if the current macro is function-like or not. In that case, if we don't have the pfile-set_invocation_location flag, we'll record the expansion point of M', if M' is an argument of the function-like macro M. -- Dodji
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/22/2012 05:04 AM, Dodji Seketeli wrote: The problem is that cpp_get_token_1 can be called when we are at the beginning of a macro expansion (inside enter_macro_expansion, called from cpp_get_token_1), *before* context-c.macro is set. This happens e.g, when we call funlike_invocation_p to know if the current macro is function-like or not. OK, sounds like we need some additional code to handle that. I guess we could do something in funlike_invocation_p to prevent cpp_get_token_1 from setting invocation_location, or change the check we use to decide whether or not we already have an invocation location, perhaps by looking at invocation_location itself (and clearing it when we finish a macro). Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes: On 05/15/2012 10:17 AM, Dodji Seketeli wrote: It fixes the test case gcc.dg/cpp/paste12.c because that test case runs with -ftrack-macro-expansion turned off. Otherwise, you are right that the issue exists only when we aren't tracking virtual locations. I still don't understand why this change should be needed; it seems like a kludge disguised as a trivial change from one function to another. If we're going to kludge, I'd rather explicitly set the flag with a comment. But why do we need the set_invocation_location flag at all? I understand this set_invocation_location business as the poor man's approach of tracking the expansion point location prior to the ftrack-macro-expansion machinery. When we are about to expand a macro M that is the top-most macro of the expansion stack, we set it to record the expansion point of M. This is what cpp_get_token_with_location does. When do we not want to set invocation_location if we're beginning to expand a macro? If M itself involves expanding another macro M', we are supposed to call the (internal function) cpp_get_token_1 that does not set set_invocation_location. So that the only expansion point recorded is the one for M, not the one for M'. Said otherwise, cpp_get_token_with_location is the preferred external top-most entry point to use to get a token (otherwise, we don't handle expansion point locations), and cpp_get_token_1 should only used internally. Now, looking at how surprising this is turning out to be, I am thinking that cpp_get_token should, like cpp_get_token_with_location just set pfile-set_invocation_location, just like cpp_get_token_with_location. Would you agree? -- Dodji
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Tom Tromey tro...@redhat.com writes: Dodji == Dodji Seketeli do...@redhat.com writes: Dodji To properly fix this, I think libcpp should keep the token of the Dodji pasting operator '##', instead of representing it with flag on the LHS Dodji operand's token. That way, it could use its location. Originally I had thought that a pasted token should have a special virtual location, pointing to the locations of its source tokens. This would let later code undo the paste, if need be. I don't know any more if this idea makes sense or not. I think this makes sense, at least in the grand scheme of things. The same underlying mechanism that would associate one or several locations (for instance, the locations of the two operands of the pasting operator) to a given location (for instance the location for the token resulting from the pasting operation) could be used to e.g, make a token carry two locations: one for the beginning of the token, and another one for the end of the token. cpp_get_token_with_location would just return the location for the beginning of the token, like it does today, and we'd be able to retrieve the second token by mean of the earlier association. This whole shebang would be then used for the mighty range based diagnostic[1]. Does that make sense? [1]: http://gcc.gnu.org/wiki/Better_Diagnostics -- Dodji
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/21/2012 10:07 AM, Dodji Seketeli wrote: Jason Merrillja...@redhat.com writes: When do we not want to set invocation_location if we're beginning to expand a macro? If M itself involves expanding another macro M', we are supposed to call the (internal function) cpp_get_token_1 that does not set set_invocation_location. So that the only expansion point recorded is the one for M, not the one for M'. But if we're already in a macro expansion (if context-c.macro is set) cpp_get_token_1 doesn't set invocation_location even if set_invocation_location is true. So we should only get the expansion point for M even if set_invocation_location is always true. Now, looking at how surprising this is turning out to be, I am thinking that cpp_get_token should, like cpp_get_token_with_location just set pfile-set_invocation_location, just like cpp_get_token_with_location. Would you agree? Yes, except that I would go farther and do away with the flag entirely. Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/15/2012 10:17 AM, Dodji Seketeli wrote: It fixes the test case gcc.dg/cpp/paste12.c because that test case runs with -ftrack-macro-expansion turned off. Otherwise, you are right that the issue exists only when we aren't tracking virtual locations. I still don't understand why this change should be needed; it seems like a kludge disguised as a trivial change from one function to another. If we're going to kludge, I'd rather explicitly set the flag with a comment. But why do we need the set_invocation_location flag at all? When do we not want to set invocation_location if we're beginning to expand a macro? Jason
[PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Hello,
As stated in the audit trail of this problem report, consider this
test case:
$ cat test.c
1 struct x {
2int i;
3 };
4 struct x x;
5
6 #define TEST(X) x.##X
7
8 void foo (void)
9 {
10TEST(i) = 0;
11 }
$
$ cc1 -quiet test.c
test.c: In function 'foo':
test.c:10:1: error: pasting . and i does not give a valid preprocessing
token
TEST(i) = 0;
^
$
So, when pasting tokens, the error diagnostic uses the global and
imprecise input_location variable, leading to an imprecise output.
To properly fix this, I think libcpp should keep the token of the
pasting operator '##', instead of representing it with flag on the LHS
operand's token. That way, it could use its location. Doing that
would be quite intrusive though. So this patch just uses the location
of the LHS of the pasting operator, for now. It's IMHO better than
the current situation.
The patch makes paste_tokens take a location parameter that is used in
the diagnostics. This change can still be useful later when we can
use the location of the pasting operator, because paste_tokens will
just be passed the new, more precise location.
Incidentally, it appeared that when getting tokens from within
preprocessor directives (like what is done in gcc.dg/cpp/paste12.c),
with -ftrack-macro-expansion disabled, the location of the expansion
point of macros was being lost because get_token_no_padding doesn't
use cpp_get_token_with_location. The patch fixes that as well.
People seem to like screenshots.
Thus, after the patch, we now have:
$ cc1 -quiet test.c
test.c: In function 'foo':
test.c:6:18: error: pasting . and i does not give a valid preprocessing
token
#define TEST(X) x.##X
^
test.c:10:3: note: in expansion of macro 'TEST'
TEST(i) = 0;
^
$
Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.
libcpp/
PR preprocessor/53229
* directives.c (get_token_no_padding): Allow use of virtual
locations in diagnostics emitted by the processor when handling
directives. Update comments. This fixes gcc.dg/cpp/paste12.c.
* macro.c (paste_tokens): Take a virtual location parameter for
the LHS of the pasting operator. Use it in diagnostics. Update
comments.
(paste_all_tokens): Tighten the assert. Propagate the location of
the expansion point when no virtual locations are available.
Pass the virtual location to paste_tokens.
gcc/testsuite/
PR preprocessor/53229
* gcc.dg/cpp/paste6.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste8.c: Likewise.
* gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste13.c: Likewise.
* gcc.dg/cpp/paste14.c: Likewise.
* gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste18.c: New test.
---
gcc/testsuite/gcc.dg/cpp/paste12-2.c | 11 +++
gcc/testsuite/gcc.dg/cpp/paste12.c |5 -
gcc/testsuite/gcc.dg/cpp/paste13.c |5 -
gcc/testsuite/gcc.dg/cpp/paste14-2.c | 11 +++
gcc/testsuite/gcc.dg/cpp/paste14.c |5 -
gcc/testsuite/gcc.dg/cpp/paste18.c | 16
gcc/testsuite/gcc.dg/cpp/paste6.c|5 -
gcc/testsuite/gcc.dg/cpp/paste8-2.c | 15 +++
gcc/testsuite/gcc.dg/cpp/paste8.c|2 +-
libcpp/directives.c |7 +--
libcpp/macro.c | 26 ++
11 files changed, 93 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste12-2.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste14-2.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste18.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste8-2.c
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12-2.c
b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
new file mode 100644
index 000..6e2e4f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
@@ -0,0 +1,11 @@
+/*
+ { dg-options -ftrack-macro-expansion=2 }
+ { dg-do preprocess }
+ */
+
+/* Test correct diagnostics when pasting in #include.
+ Source: PR preprocessor/6780. */
+
+#define inc2(a,b) ##a.b /* { dg-error pasting \\ and \stdio\ does
not } */
+#define INC(X) inc2(X,h)
+#include INC(stdio)
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12.c
b/gcc/testsuite/gcc.dg/cpp/paste12.c
index e61ec51..3e0f7b9 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste12.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste12.c
@@ -1,4 +1,7 @@
-/* { dg-do preprocess
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 15 May 2012 13:18, Dodji Seketeli do...@redhat.com wrote: People seem to like screenshots. Thus, after the patch, we now have: $ cc1 -quiet test.c test.c: In function 'foo': test.c:6:18: error: pasting . and i does not give a valid preprocessing token #define TEST(X) x.##X ^ test.c:10:3: note: in expansion of macro 'TEST' TEST(i) = 0; ^ $ I like the screenshot. ;-) Thanks for fixing this! Cheers, Manuel.
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/15/2012 07:18 AM, Dodji Seketeli wrote: +paste_tokens (cpp_reader *pfile, source_location lhs_location, If in the long run we want the location passed in to be the ## location, let's drop the lhs from the parameter name. - const cpp_token *result = cpp_get_token (pfile); + const cpp_token *result = cpp_get_token_with_location (pfile, NULL); I find the difference between these two functions confusing, since you aren't passing in a pointer for the location to go into. I see that cpp_get_token_with_location sets pfile-set_invocation_location, which is documented to mean /* When expanding a macro at top-level, this is the location of the macro invocation. */ source_location invocation_location; /* True if this call to cpp_get_token should consider setting invocation_location. */ bool set_invocation_location; But presumably get_token_no_padding isn't only called when we're starting to expand a top-level macro. And as far as I can tell the value of invocation_location is only used when we aren't tracking virtual locations anyway. So why does the change above fix the testcase? Jason
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes:
On 05/15/2012 07:18 AM, Dodji Seketeli wrote:
+paste_tokens (cpp_reader *pfile, source_location lhs_location,
If in the long run we want the location passed in to be the ##
location, let's drop the lhs from the parameter name.
Dropped.
- const cpp_token *result = cpp_get_token (pfile);
+ const cpp_token *result = cpp_get_token_with_location (pfile, NULL);
I find the difference between these two functions confusing, since you
aren't passing in a pointer for the location to go into. I see that
cpp_get_token_with_location sets pfile-set_invocation_location, which
is documented to mean
/* When expanding a macro at top-level, this is the location of the
macro invocation. */
source_location invocation_location;
/* True if this call to cpp_get_token should consider setting
invocation_location. */
bool set_invocation_location;
But presumably get_token_no_padding isn't only called when we're
starting to expand a top-level macro. And as far as I can tell the
value of invocation_location is only used when we aren't tracking
virtual locations anyway. So why does the change above fix the
testcase?
It fixes the test case gcc.dg/cpp/paste12.c because that test case runs
with -ftrack-macro-expansion turned off. Otherwise, you are right that
the issue exists only when we aren't tracking virtual locations.
FWIW, here is the updated patch I have.
libcpp/
PR preprocessor/53229
* directives.c (get_token_no_padding): Allow use of virtual
locations in diagnostics emitted by the processor when handling
directives. Update comments. This fixes gcc.dg/cpp/paste12.c.
* macro.c (paste_tokens): Take a virtual location parameter for
the LHS of the pasting operator. Use it in diagnostics. Update
comments.
(paste_all_tokens): Tighten the assert. Propagate the location of
the expansion point when no virtual locations are available.
Pass the virtual location to paste_tokens.
gcc/testsuite/
PR preprocessor/53229
* gcc.dg/cpp/paste6.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste8.c: Likewise.
* gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12.c: Force to run without
-ftrack-macro-expansion.
* gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste13.c: Likewise.
* gcc.dg/cpp/paste14.c: Likewise.
* gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with
-ftrack-macro-expansion.
* gcc.dg/cpp/paste18.c: New test.
---
gcc/testsuite/gcc.dg/cpp/paste12-2.c | 11 +++
gcc/testsuite/gcc.dg/cpp/paste12.c |5 -
gcc/testsuite/gcc.dg/cpp/paste13.c |5 -
gcc/testsuite/gcc.dg/cpp/paste14-2.c | 11 +++
gcc/testsuite/gcc.dg/cpp/paste14.c |5 -
gcc/testsuite/gcc.dg/cpp/paste18.c | 16
gcc/testsuite/gcc.dg/cpp/paste6.c|5 -
gcc/testsuite/gcc.dg/cpp/paste8-2.c | 15 +++
gcc/testsuite/gcc.dg/cpp/paste8.c|2 +-
libcpp/directives.c |7 +--
libcpp/macro.c | 25 +
11 files changed, 92 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste12-2.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste14-2.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste18.c
create mode 100644 gcc/testsuite/gcc.dg/cpp/paste8-2.c
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12-2.c
b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
new file mode 100644
index 000..6e2e4f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/paste12-2.c
@@ -0,0 +1,11 @@
+/*
+ { dg-options -ftrack-macro-expansion=2 }
+ { dg-do preprocess }
+ */
+
+/* Test correct diagnostics when pasting in #include.
+ Source: PR preprocessor/6780. */
+
+#define inc2(a,b) ##a.b /* { dg-error pasting \\ and \stdio\ does
not } */
+#define INC(X) inc2(X,h)
+#include INC(stdio)
diff --git a/gcc/testsuite/gcc.dg/cpp/paste12.c
b/gcc/testsuite/gcc.dg/cpp/paste12.c
index e61ec51..3e0f7b9 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste12.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste12.c
@@ -1,4 +1,7 @@
-/* { dg-do preprocess } */
+/*
+ { dg-options -ftrack-macro-expansion=0 }
+ { dg-do preprocess }
+*/
/* Test correct diagnostics when pasting in #include.
Source: PR preprocessor/6780. */
diff --git a/gcc/testsuite/gcc.dg/cpp/paste13.c
b/gcc/testsuite/gcc.dg/cpp/paste13.c
index 62c72d4..f0f4fd8 100644
--- a/gcc/testsuite/gcc.dg/cpp/paste13.c
+++ b/gcc/testsuite/gcc.dg/cpp/paste13.c
@@ -1,6 +1,9 @@
/* Copyright (C) 2000 Free Software Foundation, Inc. */
-/* { dg-do preprocess } */
+/*
+ { dg-options -ftrack-macro-expansion=0 }
+ { dg-do preprocess }
+*/
/* This used to be recognized as a
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Dodji == Dodji Seketeli do...@redhat.com writes: Dodji To properly fix this, I think libcpp should keep the token of the Dodji pasting operator '##', instead of representing it with flag on the LHS Dodji operand's token. That way, it could use its location. Originally I had thought that a pasted token should have a special virtual location, pointing to the locations of its source tokens. This would let later code undo the paste, if need be. I don't know any more if this idea makes sense or not. Tom
