Re: [PATCH] c-pragma: adding a data field to pragma_handler
thanks!
I formatted as you requested.
I cannot commit myself as I haven't a write after approval status,
maye you can do it, or I can wait my GSOC mentor, Basile Starynkevitch
to do this (He mights be busy for a few days).
Pierre Vittet
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1147,13 +1147,12 @@ handle_pragma_float_const_decimal64 (cpp_reader *A
}
}
-/* A vector of registered pragma callbacks. */
+/* A vector of registered pragma callbacks, which is never freed. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
-static VEC(pragma_handler, heap) *registered_pragmas;
-
typedef struct
{
const char *space;
@@ -1216,7 +1215,7 @@ c_pp_lookup_pragma (unsigned int id, const char **
static void
c_register_pragma_1 (const char *space, const char *name,
-pragma_handler handler, bool allow_expansion)
+ internal_pragma_handler ihandler, bool allow_expansion)
{
unsigned id;
@@ -1235,8 +1234,9 @@ c_register_pragma_1 (const char *space, const char
}
else
{
- VEC_safe_push (pragma_handler, heap, registered_pragmas, handler);
- id = VEC_length (pragma_handler, registered_pragmas);
+ VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+ ihandler);
+ id = VEC_length (internal_pragma_handler, registered_pragmas);
id += PRAGMA_FIRST_EXTERNAL - 1;
/* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1248,28 +1248,95 @@ c_register_pragma_1 (const char *space, const char
allow_expansion, false);
}
+/* Register a C pragma handler, using a space and a name. It disallows pragma
+ expansion (if you want it, use c_register_pragma_with_expansion instead).
*/
void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name,
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, false);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, false);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+ extra data field which can be used by the handler. It disallows pragma
+ expansion (if you want it, use c_register_pragma_with_expansion_and_data
+ instead). */
void
+c_register_pragma_with_data (const char *space, const char *name,
+ pragma_handler_2arg handler, void * data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, false);
+}
+
+/* Register a C pragma handler, using a space and a name. It allows pragma
+ expansion as in the following example:
+
+ #define NUMBER 10
+ #pragma count (NUMBER)
+
+ Name expansion is still disallowed. */
+void
c_register_pragma_with_expansion (const char *space, const char *name,
- pragma_handler handler)
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, true);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, true);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+ extra data field which can be used by the handler. It allows pragma
+ expansion as in the following example:
+
+ #define NUMBER 10
+ #pragma count (NUMBER)
+
+ Name expansion is still disallowed. */
void
+c_register_pragma_with_expansion_and_data (const char *space, const char *name,
+ pragma_handler_2arg handler,
+ void *data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, true);
+}
+
+void
c_invoke_pragma_handler (unsigned int id)
{
- pragma_handler handler;
+ internal_pragma_handler *ihandler;
+ pragma_handler_1arg handler_1arg;
+ pragma_handler_2arg handler_2arg;
id -= PRAGMA_FIRST_EXTERNAL;
- handler = *VEC_index (pragma_handler, registered_pragmas, id);
-
- handler (parse_in);
+ ihandler = VEC_index (internal_pragma_handler, registered_pragmas, id);
+ if (ihandler-extra_data)
+{
+ handler_2arg =
Re: [PATCH] c-pragma: adding a data field to pragma_handler
Please make sure that with each revision you include *both* the patch *and* the ChangeLog entries so they can be reviewed together. The last version of the ChangeLog entries that I saw still needed more work to follow the normal style for ChangeLog entries. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c-pragma: adding a data field to pragma_handler
I guess this is better now.
Changelog (gcc/c-family):
2011-06-10 Pierre Vittet pier...@pvittet.com
* c-pragma.h (pragma_handler_1arg, pragma_handler_2arg): New handler.
(gen_pragma_handler): New union.
(internal_pragma_handler): New type.
(c_register_pragma_with_data,
c_register_pragma_with_expansion_and_data): New functions.
* c-pragma.c (registered_pragmas, c_register_pragma_1,
c_register_pragma, c_register_pragma_with_expansion,
c_invoke_pragma_handler): Changed to work with internal_pragma_handler.
(c_register_pragma_with_data,
c_register_pragma_with_expansion_and_data): New functions.
Changelog (gcc/testsuite):
2011-06-10 Pierre Vittet pier...@pvittet.com
* g++.dg/plugin/pragma_plugin_with_data.c: New test file.
* g++.dg/plugin/pragma_plugin_with_data-test-1.C: New test file.
* g++.dg/plugin/plugin.exp (plugin_test_list): Add the new test
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1147,13 +1147,12 @@ handle_pragma_float_const_decimal64 (cpp_reader *A
}
}
-/* A vector of registered pragma callbacks. */
+/* A vector of registered pragma callbacks, which is never freed. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
-static VEC(pragma_handler, heap) *registered_pragmas;
-
typedef struct
{
const char *space;
@@ -1216,7 +1215,7 @@ c_pp_lookup_pragma (unsigned int id, const char **
static void
c_register_pragma_1 (const char *space, const char *name,
-pragma_handler handler, bool allow_expansion)
+ internal_pragma_handler ihandler, bool allow_expansion)
{
unsigned id;
@@ -1235,8 +1234,9 @@ c_register_pragma_1 (const char *space, const char
}
else
{
- VEC_safe_push (pragma_handler, heap, registered_pragmas, handler);
- id = VEC_length (pragma_handler, registered_pragmas);
+ VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+ ihandler);
+ id = VEC_length (internal_pragma_handler, registered_pragmas);
id += PRAGMA_FIRST_EXTERNAL - 1;
/* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1248,28 +1248,95 @@ c_register_pragma_1 (const char *space, const char
allow_expansion, false);
}
+/* Register a C pragma handler, using a space and a name. It disallows pragma
+ expansion (if you want it, use c_register_pragma_with_expansion instead).
*/
void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name,
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, false);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, false);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+ extra data field which can be used by the handler. It disallows pragma
+ expansion (if you want it, use c_register_pragma_with_expansion_and_data
+ instead). */
void
+c_register_pragma_with_data (const char *space, const char *name,
+ pragma_handler_2arg handler, void * data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, false);
+}
+
+/* Register a C pragma handler, using a space and a name. It allows pragma
+ expansion as in the following example:
+
+ #define NUMBER 10
+ #pragma count (NUMBER)
+
+ Name expansion is still disallowed. */
+void
c_register_pragma_with_expansion (const char *space, const char *name,
- pragma_handler handler)
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, true);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, true);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+ extra data field which can be used by the handler. It allows pragma
+ expansion as in the following example:
+
+ #define NUMBER 10
+ #pragma count (NUMBER)
+
+ Name expansion is still disallowed. */
void
+c_register_pragma_with_expansion_and_data (const char *space, const char *name,
+ pragma_handler_2arg handler,
+ void
Re: [PATCH] c-pragma: adding a data field to pragma_handler
On Wed, 08 Jun 2011 23:26:39 +0200
Pierre Vittet pier...@pvittet.com wrote:
I have written a test for this patch and run it (it works correctly). I
guess there is no reason why it should not be accepted now.
To recap, this patch add a void * data field to the pragma handler,
allowing to pass extra data. If we want to use this field, we need to
use the function c_register_pragma_with_data or
c_register_pragma_with_expansion_and_data. The old
c_register_pragma(_with_expansion) is kept compatible.
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1148,12 +1148,12 @@ handle_pragma_float_const_decimal64 (cpp_reader
*A }
/* A vector of registered pragma callbacks. */
+/* This is never freed as we need it during the whole execution. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
Sorry to be picky Pierre, but that comment is not correct. It should be
instead.
/* A vector of registered pragma callbacks, which is never freed. */
What I mean is that you are right that the vector is never freed, but
it is not because it is needed during the entire execution, since
middle-end and back-end passes don't know about pragmas.
I hope your patch will be ok-ed with that small change.
Perhaps a future patch would free that registered_pragmas vector, but I
feel that is not necessary, since it is not a big vector in practice.
Regards.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basileatstarynkevitchdotnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [PATCH] c-pragma: adding a data field to pragma_handler
You are right, the new version is in the diff.
The diff for the test hasn't changed and is in the previous mail.
In the previous version of the file, the registered_pragmas was not
better freed. I don't know if it is really important (it would need a
callback at the end of the front-end passes).
Thanks.
On 09/06/2011 08:16, Basile Starynkevitch wrote:
On Wed, 08 Jun 2011 23:26:39 +0200
Pierre Vittetpier...@pvittet.com wrote:
I have written a test for this patch and run it (it works correctly). I
guess there is no reason why it should not be accepted now.
To recap, this patch add a void * data field to the pragma handler,
allowing to pass extra data. If we want to use this field, we need to
use the function c_register_pragma_with_data or
c_register_pragma_with_expansion_and_data. The old
c_register_pragma(_with_expansion) is kept compatible.
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1148,12 +1148,12 @@ handle_pragma_float_const_decimal64 (cpp_reader
*A }
/* A vector of registered pragma callbacks. */
+/* This is never freed as we need it during the whole execution. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
Sorry to be picky Pierre, but that comment is not correct. It should be
instead.
/* A vector of registered pragma callbacks, which is never freed. */
What I mean is that you are right that the vector is never freed, but
it is not because it is needed during the entire execution, since
middle-end and back-end passes don't know about pragmas.
I hope your patch will be ok-ed with that small change.
Perhaps a future patch would free that registered_pragmas vector, but I
feel that is not necessary, since it is not a big vector in practice.
Regards.
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1147,13 +1147,12 @@ handle_pragma_float_const_decimal64 (cpp_reader *A
}
}
-/* A vector of registered pragma callbacks. */
+/* A vector of registered pragma callbacks, which is never freed. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
-static VEC(pragma_handler, heap) *registered_pragmas;
-
typedef struct
{
const char *space;
@@ -1216,7 +1215,7 @@ c_pp_lookup_pragma (unsigned int id, const char **
static void
c_register_pragma_1 (const char *space, const char *name,
-pragma_handler handler, bool allow_expansion)
+ internal_pragma_handler ihandler, bool allow_expansion)
{
unsigned id;
@@ -1235,8 +1234,9 @@ c_register_pragma_1 (const char *space, const char
}
else
{
- VEC_safe_push (pragma_handler, heap, registered_pragmas, handler);
- id = VEC_length (pragma_handler, registered_pragmas);
+ VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+ ihandler);
+ id = VEC_length (internal_pragma_handler, registered_pragmas);
id += PRAGMA_FIRST_EXTERNAL - 1;
/* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1248,28 +1248,90 @@ c_register_pragma_1 (const char *space, const char
allow_expansion, false);
}
+/* Register a C pragma handler, using a space and a name. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name,
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, false);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, false);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+extra data field which can be used by the handler. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
+c_register_pragma_with_data (const char *space, const char *name,
+ pragma_handler_2arg handler, void * data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, false);
+}
+
+/* Register a C pragma handler, using a space and a name. It allows pragma
+expansion as in the following exemple:
+ #define NUMBER 10
+ #pragma count (NUMBER)
+Name expansion is still disallowed. */
+void
Re: [PATCH] c-pragma: adding a data field to pragma_handler
Thanks for this patch. You need to adjust the formatting of the comments to match the existing style (in particular, indentation of second and subsequent lines of comments, and watch out for spelling errors (exemple, abstact). For a struct, the explanations of individual fields should be in the comments on those fields rather than at the top of the struct. The comment on c_register_pragma_with_data should refer to c_register_pragma_with_expansion_and_data rather than c_register_pragma_with_expansion. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c-pragma: adding a data field to pragma_handler
I have written a test for this patch and run it (it works correctly). I
guess there is no reason why it should not be accepted now.
To recap, this patch add a void * data field to the pragma handler,
allowing to pass extra data. If we want to use this field, we need to
use the function c_register_pragma_with_data or
c_register_pragma_with_expansion_and_data. The old
c_register_pragma(_with_expansion) is kept compatible.
I give two diff and two ChangeLog, the first are for the patch itself,
the second are for the test.
I have tried to make things as good as possible, if there is a remark,
please, send me it. Especially, I am not sure about the format of my
ChangeLog, if there is an issue, I am ready to change it.
Changelog gcc:
2011-06-08 Pierre Vittet pier...@pvittet.com
* c-pragma.h (pragma_handler_1arg, pragma_handler_2arg,
gen_pragma_handler, internal_pragma_handler, c_register_pragma,
c_register_pragma_with_data, c_register_pragma_with_expansion,
c_register_pragma_with_expansion_and_data): allows to add data to a
pragma handler using a new c_register. Old c_register keep old
behaviour for compatibility.
* c-pragma.c (registered_pragmas, c_register_pragma_1,
c_register_pragma, c_register_pragma_with_data,
c_register_pragma_with_expansion,
c_register_pragma_with_expansion_and_data, c_invoke_pragma_handler,
init_pragma): allows to add data to a pragma handler using a new
c_register. Old registers keep old behaviour for compatibility.
Changelog testsuite
2011-06-08 Pierre Vittet pier...@pvittet.com
* g++.dg/plugin/pragma_plugin_with_data.c: New test.
Thanks!
Pierre Vittet
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1148,12 +1148,12 @@ handle_pragma_float_const_decimal64 (cpp_reader *A
}
/* A vector of registered pragma callbacks. */
+/* This is never freed as we need it during the whole execution. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
-static VEC(pragma_handler, heap) *registered_pragmas;
-
typedef struct
{
const char *space;
@@ -1216,7 +1216,7 @@ c_pp_lookup_pragma (unsigned int id, const char **
static void
c_register_pragma_1 (const char *space, const char *name,
-pragma_handler handler, bool allow_expansion)
+ internal_pragma_handler ihandler, bool allow_expansion)
{
unsigned id;
@@ -1235,8 +1235,9 @@ c_register_pragma_1 (const char *space, const char
}
else
{
- VEC_safe_push (pragma_handler, heap, registered_pragmas, handler);
- id = VEC_length (pragma_handler, registered_pragmas);
+ VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+ ihandler);
+ id = VEC_length (internal_pragma_handler, registered_pragmas);
id += PRAGMA_FIRST_EXTERNAL - 1;
/* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1248,28 +1249,90 @@ c_register_pragma_1 (const char *space, const char
allow_expansion, false);
}
+/* Register a C pragma handler, using a space and a name. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name,
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, false);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, false);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+extra data field which can be used by the handler. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
+c_register_pragma_with_data (const char *space, const char *name,
+ pragma_handler_2arg handler, void * data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, false);
+}
+
+/* Register a C pragma handler, using a space and a name. It allows pragma
+expansion as in the following exemple:
+ #define NUMBER 10
+ #pragma count (NUMBER)
+Name expansion is still disallowed. */
+void
c_register_pragma_with_expansion (const char *space, const char *name,
- pragma_handler handler)
+
Re: [PATCH] c-pragma: adding a data field to pragma_handler
Thank you for your answer!
I send you a new patch
I have corrected the errors you raised.
I have make my patch compatible with the old use of c_register_pragma
and c_register_pragma_with_expansion.
I don't know what is the best solution, maybe changing every call
c_register_pragma allows to keep a more clear code. I can do it, if you
think it is better.
I have successfully compiled gcc with my patch and I have tried it with
a modified version of gcc/testsuite/g++.dg/plugin/pragma_plugin.c.
Pierre Vittet
On 02/06/2011 19:51, Tom Tromey wrote:
Pierre == Pierrep.vit...@laposte.net writes:
Pierre I have changed this handler in order to accept a second parameter
Pierre which is a void *, allowing to give extra datas to the handler. I
Pierre think this data field might be of general use: we can have condition
Pierre or data at register time that we want to express in the handler. I
Pierre guess this is a common way to pass data to an handler function.
I can't approve or reject this patch, but the idea seems reasonable
enough to me.
Pierre I would like your opinion on this patch! Thanks!
It has a number of formatting issues.
Pierre +typedef void (*pragma_handler)(struct cpp_reader *, void * );
No space after the final *.
Pierre +/* Internally use to keep the data of the handler. */
Pierre +struct internal_pragma_handler_d{
Space before the {.
Pierre + pragma_handler handler;
Pierre + void * data;
No space. Lots of instances of this.
Pierre /* A vector of registered pragma callbacks. */
Pierre +/*This is never freed as we need it during the whole execution */
Coalesce the two comments. The comment formatting is wrong, see GNU
standards.
Pierre ns_name.space = space;
Pierre ns_name.name = name;
Pierre +
Pierre VEC_safe_push (pragma_ns_name, heap,
registered_pp_pragmas,ns_name);
Gratuitous newline addition.
Pierre + ihandler-handler = handler;
Pierre + ihandler-data = data;
I didn't see anything that initialized ihandler.
Pierre + VEC_safe_push (internal_pragma_handler, heap,
registered_pragmas,
Pierre +ihandler);
I think you wanted just `internal_pragma_handler ihandler', no *, for
the definition.
Pierre +c_register_pragma (const char *space, const char *name,
pragma_handler handler,
Pierre + void * data)
There are lots of calls to this that you did not update.
Do a recursive grep to see.
One way to avoid a massive change is to add a new overload that passes
in the data to c_register_pragma_1; and then change the legacy
functions to pass NULL.
I don't know if that approach is ok (it is typical in gdb...), so if
not, you have to update all callers.
Tom
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -53,7 +53,7 @@ typedef struct GTY(()) align_stack {
static GTY(()) struct align_stack * alignment_stack;
-static void handle_pragma_pack (cpp_reader *);
+static void handle_pragma_pack (cpp_reader *, void * data);
/* If we have a global #pragma pack(n) in effect when the first
#pragma pack(push,n) is encountered, this stores the value of
@@ -133,7 +133,7 @@ pop_alignment (tree id)
#pragma pack (pop)
#pragma pack (pop, ID) */
static void
-handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy), void * ARG_UNUSED (data))
{
tree x, id = 0;
int align = -1;
@@ -247,7 +247,7 @@ DEF_VEC_ALLOC_O(pending_weak,gc);
static GTY(()) VEC(pending_weak,gc) *pending_weaks;
static void apply_pragma_weak (tree, tree);
-static void handle_pragma_weak (cpp_reader *);
+static void handle_pragma_weak (cpp_reader *, void * data);
static void
apply_pragma_weak (tree decl, tree value)
@@ -334,7 +334,7 @@ maybe_apply_pending_pragma_weaks (void)
/* #pragma weak name [= value] */
static void
-handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy), void * ARG_UNUSED (data))
{
tree name, value, x, decl;
enum cpp_ttype t;
@@ -411,11 +411,12 @@ DEF_VEC_ALLOC_O(pending_redefinition,gc);
static GTY(()) VEC(pending_redefinition,gc) *pending_redefine_extname;
-static void handle_pragma_redefine_extname (cpp_reader *);
+static void handle_pragma_redefine_extname (cpp_reader *, void * data);
/* #pragma redefine_extname oldname newname */
static void
-handle_pragma_redefine_extname (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_redefine_extname (cpp_reader * ARG_UNUSED (dummy),
+void * ARG_UNUSED (data))
{
tree oldname, newname, decl, x;
enum cpp_ttype t;
@@ -481,7 +482,8 @@ static GTY(()) tree pragma_extern_prefix;
/* #pragma extern_prefix prefix */
static void
-handle_pragma_extern_prefix (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_extern_prefix (cpp_reader * ARG_UNUSED (dummy),
Re: [PATCH] c-pragma: adding a data field to pragma_handler
On Fri, 03 Jun 2011 17:31:25 +0200
Pierre Vittet pier...@pvittet.com wrote:
Thank you for your answer!
I send you a new patch
I have corrected the errors you raised.
I have make my patch compatible with the old use of c_register_pragma
and c_register_pragma_with_expansion.
I find the patch quite interesting, but I cannot approve it.
void
+c_register_pragma_with_expansion_and_data (const char *space, const char *name,
+
pragma_handler_2arg handler,
+ void * data)
Perhaps there are some spaces (vs tabs) issues here.
Cheers
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basileatstarynkevitchdotnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [PATCH] c-pragma: adding a data field to pragma_handler
Hello,
I am sorry, my editor (vim) was not correctly configure (I used
http://gcc.gnu.org/ml/gcc/2011-03/msg00425.html to improve it). I guess
it is ok now.
If I still have issue, I will post on the mailing list if there is some
tips for vim otherway I will use Emacs (I am not very comfortable with
it for now ;).
Pierre Vittet
On 03/06/2011 17:47, Basile Starynkevitch wrote:
On Fri, 03 Jun 2011 17:31:25 +0200
Pierre Vittetpier...@pvittet.com wrote:
Thank you for your answer!
I send you a new patch
I have corrected the errors you raised.
I have make my patch compatible with the old use of c_register_pragma
and c_register_pragma_with_expansion.
I find the patch quite interesting, but I cannot approve it.
void
+c_register_pragma_with_expansion_and_data (const char *space, const char *name,
+
pragma_handler_2arg handler,
+ void * data)
Perhaps there are some spaces (vs tabs) issues here.
Cheers
Index: gcc/c-family/c-pragma.c
===
--- gcc/c-family/c-pragma.c (revision 174521)
+++ gcc/c-family/c-pragma.c (working copy)
@@ -1148,12 +1148,12 @@ handle_pragma_float_const_decimal64 (cpp_reader *A
}
/* A vector of registered pragma callbacks. */
+/* This is never freed as we need it during the whole execution. */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
-static VEC(pragma_handler, heap) *registered_pragmas;
-
typedef struct
{
const char *space;
@@ -1216,7 +1216,7 @@ c_pp_lookup_pragma (unsigned int id, const char **
static void
c_register_pragma_1 (const char *space, const char *name,
-pragma_handler handler, bool allow_expansion)
+ internal_pragma_handler ihandler, bool allow_expansion)
{
unsigned id;
@@ -1235,8 +1235,9 @@ c_register_pragma_1 (const char *space, const char
}
else
{
- VEC_safe_push (pragma_handler, heap, registered_pragmas, handler);
- id = VEC_length (pragma_handler, registered_pragmas);
+ VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+ ihandler);
+ id = VEC_length (internal_pragma_handler, registered_pragmas);
id += PRAGMA_FIRST_EXTERNAL - 1;
/* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1248,28 +1249,90 @@ c_register_pragma_1 (const char *space, const char
allow_expansion, false);
}
+/* Register a C pragma handler, using a space and a name. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name,
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, false);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, false);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+extra data field which can be used by the handler. It disallows pragma
+expansion (if you want it, use c_register_pragma_with_expansion instead). */
void
+c_register_pragma_with_data (const char *space, const char *name,
+ pragma_handler_2arg handler, void * data)
+{
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_2arg = handler;
+ ihandler.extra_data = true;
+ ihandler.data = data;
+ c_register_pragma_1 (space, name, ihandler, false);
+}
+
+/* Register a C pragma handler, using a space and a name. It allows pragma
+expansion as in the following exemple:
+ #define NUMBER 10
+ #pragma count (NUMBER)
+Name expansion is still disallowed. */
+void
c_register_pragma_with_expansion (const char *space, const char *name,
- pragma_handler handler)
+ pragma_handler_1arg handler)
{
- c_register_pragma_1 (space, name, handler, true);
+ internal_pragma_handler ihandler;
+
+ ihandler.handler.handler_1arg = handler;
+ ihandler.extra_data = false;
+ ihandler.data = NULL;
+ c_register_pragma_1 (space, name, ihandler, true);
}
+/* Register a C pragma handler, using a space and a name, it also carries an
+extra data field which can be used by the handler. It allows pragma expansion
+as in the following exemple:
+ #define NUMBER 10
+ #pragma count (NUMBER)
+Name expansion is still disallowed. */
void
+c_register_pragma_with_expansion_and_data (const char *space, const char *name,
+
Re: [PATCH] c-pragma: adding a data field to pragma_handler
Pierre == Pierre p.vit...@laposte.net writes:
Pierre I have changed this handler in order to accept a second parameter
Pierre which is a void *, allowing to give extra datas to the handler. I
Pierre think this data field might be of general use: we can have condition
Pierre or data at register time that we want to express in the handler. I
Pierre guess this is a common way to pass data to an handler function.
I can't approve or reject this patch, but the idea seems reasonable
enough to me.
Pierre I would like your opinion on this patch! Thanks!
It has a number of formatting issues.
Pierre +typedef void (*pragma_handler)(struct cpp_reader *, void * );
No space after the final *.
Pierre +/* Internally use to keep the data of the handler. */
Pierre +struct internal_pragma_handler_d{
Space before the {.
Pierre + pragma_handler handler;
Pierre + void * data;
No space. Lots of instances of this.
Pierre /* A vector of registered pragma callbacks. */
Pierre +/*This is never freed as we need it during the whole execution */
Coalesce the two comments. The comment formatting is wrong, see GNU
standards.
Pierrens_name.space = space;
Pierrens_name.name = name;
Pierre +
PierreVEC_safe_push (pragma_ns_name, heap, registered_pp_pragmas,
ns_name);
Gratuitous newline addition.
Pierre + ihandler-handler = handler;
Pierre + ihandler-data = data;
I didn't see anything that initialized ihandler.
Pierre + VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
Pierre +ihandler);
I think you wanted just `internal_pragma_handler ihandler', no *, for
the definition.
Pierre +c_register_pragma (const char *space, const char *name, pragma_handler
handler,
Pierre + void * data)
There are lots of calls to this that you did not update.
Do a recursive grep to see.
One way to avoid a massive change is to add a new overload that passes
in the data to c_register_pragma_1; and then change the legacy
functions to pass NULL.
I don't know if that approach is ok (it is typical in gdb...), so if
not, you have to update all callers.
Tom
Re: [PATCH] c-pragma: adding a data field to pragma_handler
On Wed, 01 Jun 2011 18:54:38 +0200
Pierre p.vit...@laposte.net wrote:
This patch is about the pragmas.
In c-family/c-pragma.h, we declare a pragma_handler which is a function
accepting cpp_reader as parameter.
I have changed this handler in order to accept a second parameter which
is a void *, allowing to give extra datas to the handler. I think this
data field might be of general use: we can have condition or data at
register time that we want to express in the handler. I guess this is a
common way to pass data to an handler function.
I find this patch interesting and useful ( not only for MELT).
A general coding rule in C seems to be that every time function can be
variably called thru indirect pointers (which can have several
different functions as value), they better take an extra data argument.
This is the case, in particular, inside Glib GTK, inside the Linux
kernel, and on several other occurrences in GCC.
A use case of such pragma handlers with data for pragmas would be a
plugin which permit some messages to other channels than stdout/stderr
(e.g. other files, or a pipe, or the D-Bus, or a widget, or a web
service...). Then the same routine would handle
#pragma GCCPLUGIN message_to_file foo
and
#pragma GCCPLUGIN message_to_pipe bar
and the data pointer would be different (an fopen-ed or popen-ed
FILE*, or even an std::ostream if the plugin is coded in C++).
I am not authorized to ok the patch (I believe the changelog had some
typos), but I hope someone will review ok it.
Regards
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basileatstarynkevitchdotnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
