Re: [PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-29 Thread Dodji Seketeli
Jason Merrill ja...@redhat.com writes:

 On 04/25/2012 05:07 AM, Dodji Seketeli wrote:
 +  /* If the first token we got was a padding token, let's put
 + it back into the stream so that cpp_get_token will get it
 + first; and if we are currently expanding a macro, don't
 + forget that information.  */
 +  cpp_hashnode *macro =
 +(pfile-context-tokens_kind == TOKENS_KIND_EXTENDED)
 +? pfile-context-c.mc-macro_node
 +: pfile-context-c.macro;
 +  _cpp_push_token_context (pfile, macro, padding, 1);

 What about the other places that call _cpp_push_token_context with a
 NULL macro argument?  Don't we want to continue the current macro
 context in that case, too?  Perhaps we should move this new code
 inside _cpp_push_token_context for the case when the macro parameter
 is NULL.

Right.  I did that.

But then, now that there can be some contiguous contexts representing
the same macro, I had to adjust how _cpp_pop_context was re-enabling the
'expandability' of a given macro M.

For the background, When M is being expanded, it's flagged by
enter_macro_context as being non-expandable, to prevent its possible
recursive expansions.  And it's flagged back to being expandable when we
get out of its expansion context.

Now, getting out of the expansion context means to test that have
actually popped all the possibly contiguous contexts that are related to
M.  Otherwise, we get into situations of recursive expansion of M.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

libcpp/
* macro.c (macro_of_context): New static function.
(_cpp_push_token_context, push_extended_tokens_context): If the
macro argument is NULL, it means we are continuing the expansion
of the current macro, if any.  Update comments.
(_cpp_pop_context): Re-enable expansion of the macro only when we
are really out of the context of the current expansion.

gcc/testsuite/

* gcc.dg/debug/dwarf2/pr41445-5.c: Adjust.
* gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.
---
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c |5 ++-
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c |5 ++-
 libcpp/macro.c|   56 +
 3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
index 03af604..d21acd5 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
@@ -9,6 +9,9 @@
 #define B , varj
 int A(B) ;
 
-/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0x)?7\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
+/*  We want to check that both vari and varj have the same line
+number.  */
+
+/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\varj\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
index 8aa37d1..d6d79cc 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
@@ -4,5 +4,8 @@
 
 #include pr41445-5.c
 
-/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0x)?7\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
+/*  We want to check that both vari and varj have the same line
+number.  */
+
+/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)?\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\varj\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
diff --git a/libcpp/macro.c b/libcpp/macro.c
index f4638c4..ab3e8f6 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -165,6 +165,8 @@ static void consume_next_token_from_context (cpp_reader 
*pfile,
 source_location *);
 static const cpp_token* 

Re: [PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-29 Thread Jason Merrill

OK.

Jason


Re: [PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-28 Thread Jason Merrill

On 04/25/2012 05:07 AM, Dodji Seketeli wrote:

+ /* If the first token we got was a padding token, let's put
+it back into the stream so that cpp_get_token will get it
+first; and if we are currently expanding a macro, don't
+forget that information.  */
+ cpp_hashnode *macro =
+   (pfile-context-tokens_kind == TOKENS_KIND_EXTENDED)
+   ? pfile-context-c.mc-macro_node
+   : pfile-context-c.macro;
+ _cpp_push_token_context (pfile, macro, padding, 1);


What about the other places that call _cpp_push_token_context with a 
NULL macro argument?  Don't we want to continue the current macro 
context in that case, too?  Perhaps we should move this new code inside 
_cpp_push_token_context for the case when the macro parameter is NULL.


Jason


Re: [PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-25 Thread Dodji Seketeli
Jason Merrill ja...@redhat.com writes:

 On 04/10/2012 03:42 PM, Dodji Seketeli wrote:
 In that case, besides returning NULL, enter_macro_context sets
 pfile-context-c.macro to NULL, making cpp_get_token_1 forget to set
 the location of the vari to the expansion point of A.

 This seems like a bug that should be fixed rather than worked around;
 we are still expanding A.

Right.  Below is an updated patch (with an updated introductory text)
that addresses the core of the issue.

Consider the test case gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c.
Its interesting part is:

#define A(x) vari x /* line 7.  */
#define vari(x)
#define B , varj
int A(B) ;  /* line 10.  */

In its initial version, this test was being pre-processed as:

# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 1 build/gcc//
# 1 command-line
# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 10 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
int
# 7 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
 vari

, varj ;

Note how int and vari are on separate lines, whereas int and
, varj are on the same line.

This looks like a bug to me, even independantly from the macro
location tracking work.

With macro location tracking turned on, the preprocessed output
becomes:

# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 1 command-line
# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 10 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
int vari , varj ;

Which, IMO, is what we'd expect.

This is due to an unexpected side effect of enter_macro_context when
passed a token that might look like a function-like macro at first
sight, but that it eventually considers to not be a macro after all.

This is the case for the vari token which looks like a macro when it
is first lexed, but is eventually considered to be a normal token by
enter_macro_context because it's not used as a function-like macro
invocation.

In that case, besides returning NULL, enter_macro_context sets
pfile-context-c.macro to NULL, making cpp_get_token_1 forget to set
the location of the vari to the expansion point of A.

enter_macro_context sets pfile-context-c.macro to NULL in that case
because funlike_invocation_p reads one token pass foo, sees that
there is no '(' token, so we are not invoking the function-like
parameter.  It then puts the tokens (which it has read after foo)
back into the tokens stream by calling _cpp_push_token_context on it,
which sets pfile-context-c.macro to NULL.

The fix here is to prevent funlike_invocation_p from forgetting the
current macro-ness.

Tested on x86_64-unknown-linux-gnu against trunk.  Now this test has
the same output with and without tracking locations accross macro
expansions.

Note that the bootstrap with -ftrack-macro-expansion exhibits other
separate issues that are addressed in subsequent patches.  This patch
just fixes one class of problems.

The patch does pass bootstrap with -ftrack-macro-expansion turned off,
though.

libcpp/
* macro.c (funlike_invocation_p): Don't forget macro-ness.

gcc/testsuite/

* gcc.dg/debug/dwarf2/pr41445-5.c: Adjust.
* gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.
---
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c |5 -
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c |5 -
 libcpp/macro.c|   12 +++-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
index 03af604..d21acd5 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
@@ -9,6 +9,9 @@
 #define B , varj
 int A(B) ;
 
-/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0x)?7\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
+/*  We want to check that both vari and varj have the same line
+number.  */
+
+/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\varj\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
index 8aa37d1..d6d79cc 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
@@ -4,5 +4,8 @@
 
 #include pr41445-5.c
 
-/* { dg-final { scan-assembler 

Re: [PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-11 Thread Jason Merrill

On 04/10/2012 03:42 PM, Dodji Seketeli wrote:

In that case, besides returning NULL, enter_macro_context sets
pfile-context-c.macro to NULL, making cpp_get_token_1 forget to set
the location of the vari to the expansion point of A.


This seems like a bug that should be fixed rather than worked around; we 
are still expanding A.


Jason


[PATCH 04/11] Fix expansion point loc for macro-like tokens

2012-04-10 Thread Dodji Seketeli
Consider the test case gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c.
Its interesting part is:

#define A(x) vari x /* line 7.  */
#define vari(x)
#define B , varj
int A(B) ;  /* line 10.  */

In its initial version, this test was being pre-processed as:

# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 1 build/gcc//
# 1 command-line
# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 10 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
int
# 7 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
 vari

, varj ;

Note how int and vari are on separate lines, whereas int and
, varj are on the same line.

This looks like a bug to me, even independantly from the macro
location tracking work.

With macro location tracking turned on, the preprocessed output
becomes:

# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 1 command-line
# 1 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
# 10 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
int vari , varj ;

Which, IMO, is what we'd expect.

This is due to an unexpected side effect of enter_macro_context when
passed a token that might look like a function-like macro at first
sight, but that it eventually considers to not be a macro after all.

This is the case for the vari token which looks like a macro when it
is first lexed, but is eventually considered to be a normal token by
enter_macro_context because it's not used as a function-like macro
invocation.

In that case, besides returning NULL, enter_macro_context sets
pfile-context-c.macro to NULL, making cpp_get_token_1 forget to set
the location of the vari to the expansion point of A.

Please look at the extensive comments I have added to cpp_get_token_1
to understand the details.

The easy fix here is to speculatively set the location of the returned
token before calling enter_macro_context.

Tested on x86_64-unknown-linux-gnu against trunk.  Now this test has
the same output with and without tracking locations accross macro
expansions.

Note that the bootstrap with -ftrack-macro-expansion exhibits other
separate issues that are addressed in subsequent patches.  This patch
just fixes one class of problems.

The patch does pass bootstrap with -ftrack-macro-expansion turned off,
though.

libcpp/
* macro.c (enter_macro_context): Update comment.
(cpp_get_token_1): Set virtual location before calling
enter_macro_context.

gcc/testsuite/

* gcc.dg/debug/dwarf2/pr41445-5.c: Adjust.
* gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.

---
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c |5 ++-
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c |5 ++-
 libcpp/macro.c|   43 +++-
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
index 03af604..d21acd5 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c
@@ -9,6 +9,9 @@
 #define B , varj
 int A(B) ;
 
-/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0x)?7\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
+/*  We want to check that both vari and varj have the same line
+number.  */
+
+/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\varj\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
index 8aa37d1..d6d79cc 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c
@@ -4,5 +4,8 @@
 
 #include pr41445-5.c
 
-/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0x)?7\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
+/*  We want to check that both vari and varj have the same line
+number.  */
+
+/* { dg-final { scan-assembler 
DW_TAG_variable\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\vari\[^\\r\\n\]*DW_AT_name(\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_)*\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*\[^0-9a-fA-FxX](0xa|10)?\[^0-9a-fA-FxX]\[^\\r\\n\]*DW_AT_decl_line
 } } */
 /* { dg-final { scan-assembler