[patch]: Add push_macro/pop_macro feature for all targets by moving it from C frontent into libcpp

[Kai Tietz-2]

Hello,

the current implementation of the push_macro/pop_macro feature for
windows targets was broken for C++ frontend and preprocessor. Just for
C it was working proper. So I would like to move this feature from C
frontend into libcpp as internal support pragma for all targets.

As example code see:

#include <stdio.h>
#undef _
#define _ 2
#pragma push_macro ("_")
#undef _
#define _ 1
#pragma pop_macro("_")
int crappy = _ ;

int main()
{
  printf ("Crap is %d\n", crappy);
  return 0;
}

It should show 'Crap is 2", but without this patch it shows for c++
(cygwin/mingw targets) 'Crap is 1".

ChangeLog for libcpp

2009-11-03  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (def_pragma_macro): New structure.
        (pushed_macro_table): New static variable.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.


ChangeLog for gcc

2009-11-03  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.

I tested this patch for i686-pc-linux, i686-pc-cygwin,
i686-pc-mingw32, and x86_64-pc-mingw32. Is this patch ok for apply?

Cheers,
Kai


--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Joseph S. Myers]

On Tue, 3 Nov 2009, Kai Tietz wrote:

> Hello,
>
> the current implementation of the push_macro/pop_macro feature for
> windows targets was broken for C++ frontend and preprocessor. Just for
> C it was working proper. So I would like to move this feature from C
> frontend into libcpp as internal support pragma for all targets.

I don't see a testcase added to the testsuite by the patch.  You need to
add at least one that fails before and passes after the patch, unless
there is already one there.

(This is not a review of the rest of the patch.)

--
Joseph S. Myers
joseph@...

[Kai Tietz-2]

2009/11/3 Joseph S. Myers <joseph@...>:

> On Tue, 3 Nov 2009, Kai Tietz wrote:
>
>> Hello,
>>
>> the current implementation of the push_macro/pop_macro feature for
>> windows targets was broken for C++ frontend and preprocessor. Just for
>> C it was working proper. So I would like to move this feature from C
>> frontend into libcpp as internal support pragma for all targets.
>
> I don't see a testcase added to the testsuite by the patch.  You need to
> add at least one that fails before and passes after the patch, unless
> there is already one there.
>
> (This is not a review of the rest of the patch.)
>
> --
> Joseph S. Myers
> joseph@...
>

Ok. Revised version with additional testcase.

2009-11-03  Kai Tietz  <kai.tietz@...>

        * gcc.c-torture/execute/pushpop_macro.c: New testcase.


Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Joseph S. Myers]

On Tue, 3 Nov 2009, Kai Tietz wrote:

> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>         * gcc.c-torture/execute/pushpop_macro.c: New testcase.

You said things worked for C but were broken for C++.  Now you propose
adding a C testcase.  Is this another case that was broken before but
works after the patch?  Are there cases failing for C++ but not for C (if
so, such a case should be added to the C++ testsuite)?

--
Joseph S. Myers
joseph@...

[Kai Tietz-2]

2009/11/3 Joseph S. Myers <joseph@...>:

> On Tue, 3 Nov 2009, Kai Tietz wrote:
>
>> 2009-11-03  Kai Tietz  <kai.tietz@...>
>>
>>         * gcc.c-torture/execute/pushpop_macro.c: New testcase.
>
> You said things worked for C but were broken for C++.  Now you propose
> adding a C testcase.  Is this another case that was broken before but
> works after the patch?  Are there cases failing for C++ but not for C (if
> so, such a case should be added to the C++ testsuite)?
>
> --
> Joseph S. Myers
> joseph@...
>

Yes, you are right. The C++ case is necessary to show the issue for
the cygwin/mingw targets.

New patch contains test for C++, too.

ChangeLog

2009-11-03  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.



Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Tom Tromey]

>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:

Kai> the current implementation of the push_macro/pop_macro feature for
Kai> windows targets was broken for C++ frontend and preprocessor. Just for
Kai> C it was working proper. So I would like to move this feature from C
Kai> frontend into libcpp as internal support pragma for all targets.

This plan sounds reasonable to me.

Kai> +static struct def_pragma_macro *pushed_macro_table;

This should be a field of struct cpp_reader, so that multiple readers
can be active at once.  Also, it should be freed when the reader is
destroyed.

Kai> +static void
Kai> +do_pragma_push_macro (cpp_reader *pfile)

This needs an introductory comment, as do the other new functions.

Kai> +      cpp_error (pfile, CPP_DL_ERROR,
Kai> + "invalid #pragma push_macro directive");

I would prefer that all new errors pass an explicit location.
This occurs a couple of times.

Tom

[Kai Tietz-2]

Hello Tom,

I reworked the patch as you suggested.

2009/11/3 Tom Tromey <tromey@...>:
> Kai> +      cpp_error (pfile, CPP_DL_ERROR,
> Kai> +           "invalid #pragma push_macro directive");
>
> I would prefer that all new errors pass an explicit location.
> This occurs a couple of times.
This is didn't got. What you mean here about explicit location? You
mean special messages for syntactical errors here?

ChangeLog for libcpp

2009-11-03  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        (def_pragma_macro): New structure.
        (cpp_reader): New member pushed_macros.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.
        * init.c (cpp_create_reader): Initialize pushed_macros
        member.
        (cpp_destroy): Free elements in pushed_macros member.

ChangeLog for gcc

2009-11-03  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.

ChangeLog for gcc/testsuite

2009-11-03  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.
        * gcc.c-torture/execute/pushpop_macro.c: New testcase.


Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

2009/11/3 Kai Tietz <ktietz70@...>:

> Hello Tom,
>
> I reworked the patch as you suggested.
>
> 2009/11/3 Tom Tromey <tromey@...>:
>> Kai> +      cpp_error (pfile, CPP_DL_ERROR,
>> Kai> +           "invalid #pragma push_macro directive");
>>
>> I would prefer that all new errors pass an explicit location.
>> This occurs a couple of times.
> This is didn't got. What you mean here about explicit location? You
> mean special messages for syntactical errors here?
>
> ChangeLog for libcpp
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * directives.c (do_pragma_push_macro): New pragma handler.
>        (do_pragma_pop_macro): Likewise.
>        (_cpp_init_internal_pragmas): Add push_macro and
>        pop_macro handler to internal pragmas.
>        (lex_macro_node_from_str): Removed.
>        (cpp_push_definition): Replace lex_macro_node_from_str
>        by _cpp_lex_identifier.
>        (cpp_pop_definition): Likewise.
>        * internal.h (_cpp_lex_identifier): New prototype.
>        (def_pragma_macro): New structure.
>        (cpp_reader): New member pushed_macros.
>        * lex.c (_cpp_lex_identifier): New function.
>        (lex_identifier_intern): New function.
>        * init.c (cpp_create_reader): Initialize pushed_macros
>        member.
>        (cpp_destroy): Free elements in pushed_macros member.
>
> ChangeLog for gcc
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
>        Removed.
>        * c-pragma.c (def_pragma_macro_value): Likewise.
>        (def_pragma_macro): Likewise.
>        (pushed_macro_table): Likewise.
>        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
>        code.
>
> ChangeLog for gcc/testsuite
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * g++.dg/torture/pushpop_macro.C: New testcase.
>        * gcc.c-torture/execute/pushpop_macro.c: New testcase.
>
>
> Kai
>
> --
> |  (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
>

By testing patch for i686-pc-linux, i686-pc-cygwin, i686-pc-mingw32,
and x86_64-pc-mingw32 I found two issues I corrected in this patch. In
function lex_identifier_intern I passed an unused argument, which I
removed in the attached patch. Also I corrected the starting comment
of the new C++ testcase, which was copy/pasted by accident.

Is this patch ok for apply?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Tom Tromey]

>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:

Tom> I would prefer that all new errors pass an explicit location.
Tom> This occurs a couple of times.

Kai> This is didn't got. What you mean here about explicit location? You
Kai> mean special messages for syntactical errors here?

What I meant is, for a new error, use the cpp_error_with_line function
and pass in the proper location explicitly.  Sometimes (I didn't check
this code) this is not easy, in which case it is ok to skip it.

Kai> -/* Like lex_macro_node, but read the input from STR.  */
Kai> -static cpp_hashnode *
Kai> -lex_macro_node_from_str (cpp_reader *pfile, const char *str)

What is the rationale for this part of the change?

Kai> -static GTY((param_is (struct def_pragma_macro))) htab_t pushed_macro_table;

Do pushed macros work properly with PCH after this patch?

This is looking pretty good.

Tom

[Kai Tietz-2]

2009/11/4 Tom Tromey <tromey@...>:

>>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:
>
> Tom> I would prefer that all new errors pass an explicit location.
> Tom> This occurs a couple of times.
>
> Kai> This is didn't got. What you mean here about explicit location? You
> Kai> mean special messages for syntactical errors here?
>
> What I meant is, for a new error, use the cpp_error_with_line function
> and pass in the proper location explicitly.  Sometimes (I didn't check
> this code) this is not easy, in which case it is ok to skip it.
>
> Kai> -/* Like lex_macro_node, but read the input from STR.  */
> Kai> -static cpp_hashnode *
> Kai> -lex_macro_node_from_str (cpp_reader *pfile, const char *str)
>
> What is the rationale for this part of the change?

The issue I had here was, that the old code wasn't usable within the
pragma parsing. As I use the internal identifier lex function for
getting the cpp_hashnode for an name, this static function was simply
not used anymore.

> Kai> -static GTY((param_is (struct def_pragma_macro))) htab_t pushed_macro_table;
>
> Do pushed macros work properly with PCH after this patch?

As far as my test have shown for i686-pc-linux, it works now pretty well.

> This is looking pretty good.
>
> Tom
>

I added to the patch the cleanup of the tm.texi part about target
macro HANDLE_PRAGMA_PUSH_POP_MACRO,

ChangeLog for libcpp

2009-11-04  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        (def_pragma_macro): New structure.
        (cpp_reader): New member pushed_macros.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.
        * init.c (cpp_create_reader): Initialize pushed_macros
        member.
        (cpp_destroy): Free elements in pushed_macros member.

ChangeLog for gcc

2009-11-04  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.
        * doc/tm.texi (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.

ChangeLog for gcc/testsuite

2009-11-04  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.
        * gcc.c-torture/execute/pushpop_macro.c: New testcase.


Ok for apply?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Dave Korn-6]

Kai Tietz wrote:

> ChangeLog for gcc/testsuite
>
> 2009-11-04  Kai Tietz  <kai.tietz@...>
>
> * g++.dg/torture/pushpop_macro.C: New testcase.
> * gcc.c-torture/execute/pushpop_macro.c: New testcase.

  We have a C and C++ common dir in the testsuite now, I think you could
probably just put one copy of the testcase in there?  Not sure if it does
execute tests or just compile tests, but in that case you could just tweak the
testcase so that instead of defining the underscore to 2 or 1, you could
define it to 2 or something that creates a syntax or link error.

     cheers,
      DaveK

[Kai Tietz-2]

2009/11/4 Dave Korn <dave.korn.cygwin@...>:

> Kai Tietz wrote:
>
>> ChangeLog for gcc/testsuite
>>
>> 2009-11-04  Kai Tietz  <kai.tietz@...>
>>
>>       * g++.dg/torture/pushpop_macro.C: New testcase.
>>       * gcc.c-torture/execute/pushpop_macro.c: New testcase.
>
>  We have a C and C++ common dir in the testsuite now, I think you could
> probably just put one copy of the testcase in there?  Not sure if it does
> execute tests or just compile tests, but in that case you could just tweak the
> testcase so that instead of defining the underscore to 2 or 1, you could
> define it to 2 or something that creates a syntax or link error.
>
>     cheers,
>      DaveK

Well, as this feature is more or less IMHO a torture testcase and the
C and C++ cases are different, as exactly at the moment we have a
working C version, but C++ fails badly, I think they are well placed
where they are. If it would be better to make out of those testcase
compile-only tests is IMHO more an esthetically question.
But good to know that there is now a common C/C++ directory. Thanks
for this head up.

Cheers,
Kai


--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

2009/11/4 Tom Tromey <tromey@...>:

>>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:
>
> Tom> I would prefer that all new errors pass an explicit location.
> Tom> This occurs a couple of times.
>
> Kai> This is didn't got. What you mean here about explicit location? You
> Kai> mean special messages for syntactical errors here?
>
> What I meant is, for a new error, use the cpp_error_with_line function
> and pass in the proper location explicitly.  Sometimes (I didn't check
> this code) this is not easy, in which case it is ok to skip it.
>
> Kai> -/* Like lex_macro_node, but read the input from STR.  */
> Kai> -static cpp_hashnode *
> Kai> -lex_macro_node_from_str (cpp_reader *pfile, const char *str)
>
> What is the rationale for this part of the change?
>
> Kai> -static GTY((param_is (struct def_pragma_macro))) htab_t pushed_macro_table;
>
> Do pushed macros work properly with PCH after this patch?
>
> This is looking pretty good.
>
> Tom
>

I changed the error output from cpp_error to cpp_error_with_line as
you suggested. And it works pretty well.

ChangeLog for libcpp

2009-11-03  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        (def_pragma_macro): New structure.
        (cpp_reader): New member pushed_macros.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.
        * init.c (cpp_create_reader): Initialize pushed_macros
        member.
        (cpp_destroy): Free elements in pushed_macros member.

ChangeLog for gcc

2009-11-03  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.
        * doc/tm.texi (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.

ChangeLog for gcc/testsuite

2009-11-03  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.
        * gcc.c-torture/execute/pushpop_macro.c: New testcase.

Tested for i686-pc-linux, i686-pc-mingw32, and x86_64-pc-mingw32. Ok
for apply to trunk? And possibly a back-merge to 4.4 branch, too?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Paolo Bonzini-6]

On 11/04/2009 05:25 PM, Tom Tromey wrote:
> Kai>  -static GTY((param_is (struct def_pragma_macro))) htab_t pushed_macro_table;
>
> Do pushed macros work properly with PCH after this patch?

I don't think so.  There is nothing that saves the state, so placing

--- test.h ---
#define FOO bar
#pragma push_macro("FOO")
#define FOO baz

--- test.c ---
#include "test.h"
#pragma pop_macro("FOO")


would fail because pop_macro would not find FOO on the stack.  It is
extremely unlikely to happen in practice, but it is still a bug.  You
need to modify libcpp/pch.c to fix this.

Paolo

[Kai Tietz-2]

2009/11/5 Paolo Bonzini <bonzini@...>:

> On 11/04/2009 05:25 PM, Tom Tromey wrote:
>>
>> Kai>  -static GTY((param_is (struct def_pragma_macro))) htab_t
>> pushed_macro_table;
>>
>> Do pushed macros work properly with PCH after this patch?
>
> I don't think so.  There is nothing that saves the state, so placing
>
> --- test.h ---
> #define FOO bar
> #pragma push_macro("FOO")
> #define FOO baz
>
> --- test.c ---
> #include "test.h"
> #pragma pop_macro("FOO")
>
>
> would fail because pop_macro would not find FOO on the stack.  It is
> extremely unlikely to happen in practice, but it is still a bug.  You need
> to modify libcpp/pch.c to fix this.
>
> Paolo
>
>

Thanks Paolo for pointing this out. It is right that this case should
be extremely unlikely, but it would be a bug too. So here the patch
treating the pch case, too.

ChangeLog for libcpp

2009-11-03  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        (def_pragma_macro): New structure.
        (cpp_reader): New member pushed_macros.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.
        * init.c (cpp_create_reader): Initialize pushed_macros
        member.
        (cpp_destroy): Free elements in pushed_macros member.
        * pch.c (_cpp_save_pushed_macros): New function.
        (_cpp_restore_pushed_macros): Likewise.
        (_cpp_restore_pushed_macros): Use _cpp_restore_pushed_macros.
        (cpp_read_state): Use _cpp_save_pushed_macros.

ChangeLog for gcc

2009-11-03  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.
        * doc/tm.texi (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.

ChangeLog for gcc/testsuite

2009-11-03  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.
        * gcc.c-torture/execute/pushpop_macro.c: New testcase.


Tested for i686-pc-mingw32, i686-pc-linux, x86_64-pc-mingw32, and for
i686-pc-cygwin. Ok for applying to trunk and 4.4 branch?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

2009/11/5 Kai Tietz <ktietz70@...>:

> 2009/11/5 Paolo Bonzini <bonzini@...>:
>> On 11/04/2009 05:25 PM, Tom Tromey wrote:
>>>
>>> Kai>  -static GTY((param_is (struct def_pragma_macro))) htab_t
>>> pushed_macro_table;
>>>
>>> Do pushed macros work properly with PCH after this patch?
>>
>> I don't think so.  There is nothing that saves the state, so placing
>>
>> --- test.h ---
>> #define FOO bar
>> #pragma push_macro("FOO")
>> #define FOO baz
>>
>> --- test.c ---
>> #include "test.h"
>> #pragma pop_macro("FOO")
>>
>>
>> would fail because pop_macro would not find FOO on the stack.  It is
>> extremely unlikely to happen in practice, but it is still a bug.  You need
>> to modify libcpp/pch.c to fix this.
>>
>> Paolo
>>
>>
>
> Thanks Paolo for pointing this out. It is right that this case should
> be extremely unlikely, but it would be a bug too. So here the patch
> treating the pch case, too.
>
> ChangeLog for libcpp
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * directives.c (do_pragma_push_macro): New pragma handler.
>        (do_pragma_pop_macro): Likewise.
>        (_cpp_init_internal_pragmas): Add push_macro and
>        pop_macro handler to internal pragmas.
>        (lex_macro_node_from_str): Removed.
>        (cpp_push_definition): Replace lex_macro_node_from_str
>        by _cpp_lex_identifier.
>        (cpp_pop_definition): Likewise.
>        * internal.h (_cpp_lex_identifier): New prototype.
>        (def_pragma_macro): New structure.
>        (cpp_reader): New member pushed_macros.
>        * lex.c (_cpp_lex_identifier): New function.
>        (lex_identifier_intern): New function.
>        * init.c (cpp_create_reader): Initialize pushed_macros
>        member.
>        (cpp_destroy): Free elements in pushed_macros member.
>        * pch.c (_cpp_save_pushed_macros): New function.
>        (_cpp_restore_pushed_macros): Likewise.
>        (_cpp_restore_pushed_macros): Use _cpp_restore_pushed_macros.
>        (cpp_read_state): Use _cpp_save_pushed_macros.
>
> ChangeLog for gcc
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
>        Removed.
>        * c-pragma.c (def_pragma_macro_value): Likewise.
>        (def_pragma_macro): Likewise.
>        (pushed_macro_table): Likewise.
>        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
>        code.
>        * doc/tm.texi (HANDLE_PRAGMA_PUSH_POP_MACRO):
>        Removed.
>
> ChangeLog for gcc/testsuite
>
> 2009-11-03  Kai Tietz  <kai.tietz@...>
>
>        * g++.dg/torture/pushpop_macro.C: New testcase.
>        * gcc.c-torture/execute/pushpop_macro.c: New testcase.
>
>
> Tested for i686-pc-mingw32, i686-pc-linux, x86_64-pc-mingw32, and for
> i686-pc-cygwin. Ok for applying to trunk and 4.4 branch?
>
> Cheers,
> Kai

Sorry, just noticed that I attached an old patch (which wasn't
tested). Here is the version I've tested.

Kai


--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

I have added to this patch a tests for pch.

ChangeLog for libcpp

2009-11-03  Kai Tietz  <kai.tietz@...>

        * directives.c (do_pragma_push_macro): New pragma handler.
        (do_pragma_pop_macro): Likewise.
        (_cpp_init_internal_pragmas): Add push_macro and
        pop_macro handler to internal pragmas.
        (lex_macro_node_from_str): Removed.
        (cpp_push_definition): Replace lex_macro_node_from_str
        by _cpp_lex_identifier.
        (cpp_pop_definition): Likewise.
        * internal.h (_cpp_lex_identifier): New prototype.
        (def_pragma_macro): New structure.
        (cpp_reader): New member pushed_macros.
        * lex.c (_cpp_lex_identifier): New function.
        (lex_identifier_intern): New function.
        * init.c (cpp_create_reader): Initialize pushed_macros
        member.
        (cpp_destroy): Free elements in pushed_macros member.
        * pch.c (_cpp_save_pushed_macros): New function.
        (_cpp_restore_pushed_macros): Likewise.
        (_cpp_restore_pushed_macros): Use _cpp_save_pushed_macros.
        (cpp_read_state): Use _cpp_restore_pushed_macros.

ChangeLog for gcc

2009-11-03  Kai Tietz  <kai.tietz@...>

        * config/i386/cygming.h (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.
        * c-pragma.c (def_pragma_macro_value): Likewise.
        (def_pragma_macro): Likewise.
        (pushed_macro_table): Likewise.
        (HANDLE_PRAGMA_PUSH_POP_MACRO): Remove guarded
        code.
        * doc/tm.texi (HANDLE_PRAGMA_PUSH_POP_MACRO):
        Removed.

ChangeLog for gcc/testsuite

2009-11-03  Kai Tietz  <kai.tietz@...>

        * g++.dg/torture/pushpop_macro.C: New testcase.
        * gcc.c-torture/execute/pushpop_macro.c: New testcase.
        * gcc.dg/cpp/pragma-pop_macro-1.c: Allow test for all
        targets.
        * gcc.dg/pch/pushpop-1.c: New.
        * gcc.dg/pch/pushpop-1.hs: New.

Tested for i686-pc-mingw32, x86_64-pc-mingw32, i686-pc-linux, and
i686-pc-cygwin.
Ok for apply?
--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Tom Tromey]

>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:

Kai> ChangeLog for libcpp
Kai> 2009-11-03  Kai Tietz  <kai.tietz@...>
Kai> * directives.c (do_pragma_push_macro): New pragma handler.
Kai> (do_pragma_pop_macro): Likewise.
Kai> (_cpp_init_internal_pragmas): Add push_macro and
Kai> pop_macro handler to internal pragmas.
Kai> (lex_macro_node_from_str): Removed.
Kai> (cpp_push_definition): Replace lex_macro_node_from_str
Kai> by _cpp_lex_identifier.
Kai> (cpp_pop_definition): Likewise.
Kai> * internal.h (_cpp_lex_identifier): New prototype.
Kai> (def_pragma_macro): New structure.
Kai> (cpp_reader): New member pushed_macros.
Kai> * lex.c (_cpp_lex_identifier): New function.
Kai> (lex_identifier_intern): New function.
Kai> * init.c (cpp_create_reader): Initialize pushed_macros
Kai> member.
Kai> (cpp_destroy): Free elements in pushed_macros member.
Kai> * pch.c (_cpp_save_pushed_macros): New function.
Kai> (_cpp_restore_pushed_macros): Likewise.
Kai> (_cpp_restore_pushed_macros): Use _cpp_save_pushed_macros.
Kai> (cpp_read_state): Use _cpp_restore_pushed_macros.

The libcpp parts are ok.

I can't approve the other bits.

Tom

[Kai Tietz-2]

2009/11/10 Tom Tromey <tromey@...>:

>>>>>> "Kai" == Kai Tietz <ktietz70@...> writes:
>
> Kai> ChangeLog for libcpp
> Kai> 2009-11-03  Kai Tietz  <kai.tietz@...>
> Kai>    * directives.c (do_pragma_push_macro): New pragma handler.
> Kai>    (do_pragma_pop_macro): Likewise.
> Kai>    (_cpp_init_internal_pragmas): Add push_macro and
> Kai>    pop_macro handler to internal pragmas.
> Kai>    (lex_macro_node_from_str): Removed.
> Kai>    (cpp_push_definition): Replace lex_macro_node_from_str
> Kai>    by _cpp_lex_identifier.
> Kai>    (cpp_pop_definition): Likewise.
> Kai>    * internal.h (_cpp_lex_identifier): New prototype.
> Kai>    (def_pragma_macro): New structure.
> Kai>    (cpp_reader): New member pushed_macros.
> Kai>    * lex.c (_cpp_lex_identifier): New function.
> Kai>    (lex_identifier_intern): New function.
> Kai>    * init.c (cpp_create_reader): Initialize pushed_macros
> Kai>    member.
> Kai>    (cpp_destroy): Free elements in pushed_macros member.
> Kai>    * pch.c (_cpp_save_pushed_macros): New function.
> Kai>    (_cpp_restore_pushed_macros): Likewise.
> Kai>    (_cpp_restore_pushed_macros): Use _cpp_save_pushed_macros.
> Kai>    (cpp_read_state): Use _cpp_restore_pushed_macros.
>
> The libcpp parts are ok.
>
> I can't approve the other bits.
>
> Tom
>

Thanks for reviewing. As the parts changed by this patch in c-pragma.c
where just active for cygwin/mingw targets, and this patch just
removes implementation and TARGET macro there, I think that I can
approve it by  myself. I will commit it later, if there aren't an
objections.

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination