Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Hello Romain, Sorry for my late reply to this thread. Romain Geissler romain.geiss...@st.com a écrit: Just to be sure I understand, do you need to be notified about *uses* of types and decls as well? If so, maybe a new kind of event should probably be defined, because PLUGIN_FINISH_DECL and PLUGIN_FINISH_TYPE seem to have more to do with declaring/defining decls and types than using them. Personally i don't need to catch all struct uses, but i need to catch struct declarations. I want to apply some __attributes__ to a given struct type, for example let's say i need to mark *struct my_struct* as deprecated thanks to a plugin. I know how to apply some attributes to a type or a decl. See the plugin (test_plugin.c) and the test files attached. With test_pass.c, my_struct is declared first, then defined after, applying the deprecated attribute works. With test_fail.c, my_struct is declared and defined at the same time, applying the deprecated attribute doesn't work with the current trunk (and also with my patch). I got: test_fail.c:4:1: warning: type attributes ignored after type is already defined [-Wattributes] Right, you cannot use (cplus_)decl_attributes on type that is fully created. The function warns in that case. So i may need a PLUGIN_FINISH_TYPE_DECLARATION triggered when the type is declared but before it is finally defined. Hmmh. For this specific case, maybe just setting the TREE_DEPRECATED flag on the tree node of type could do what you want? -- Dodji
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Romain Geissler romain.geiss...@gmail.com a écrit: I tried to fix PLUGIN_FINISH_DECL as well to include typedefs in C++. The followings does not currently trigger the PLUGIN_FINISH_DECL (or not in all cases), but should them ? - function parameters (in the function prototype) - definition (with a function body) of a top-level function (while the exact same function definition enclosed in a class definition will trigger PLUGIN_FINISH_DECL) - label declaration - constants defined by enums - namespace Indeed. finish_decl is not called in those cases. As to if the PLUGIN_FINISH_DECL event should be emitted for those, I'd say yes, at least if I believe what the description in plugin.def says: /* After finishing parsing a declaration. */ DEFEVENT (PLUGIN_FINISH_DECL) But I'd rather ask what the maintainers think about it. Jason, Diego? -- Dodji
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
On 11-09-22 09:40 , Dodji Seketeli wrote: Romain Geisslerromain.geiss...@gmail.com a écrit: I tried to fix PLUGIN_FINISH_DECL as well to include typedefs in C++. The followings does not currently trigger the PLUGIN_FINISH_DECL (or not in all cases), but should them ? - function parameters (in the function prototype) - definition (with a function body) of a top-level function (while the exact same function definition enclosed in a class definition will trigger PLUGIN_FINISH_DECL) - label declaration - constants defined by enums - namespace Indeed. finish_decl is not called in those cases. As to if the PLUGIN_FINISH_DECL event should be emitted for those, I'd say yes, at least if I believe what the description in plugin.def says: /* After finishing parsing a declaration. */ DEFEVENT (PLUGIN_FINISH_DECL) But I'd rather ask what the maintainers think about it. Jason, Diego? Yes, those events should trigger a PLUGIN_FINISH_DECL call. Diego.
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Hello, Le 22 sept. 2011 à 15:22, Dodji Seketeli a écrit : So i may need a PLUGIN_FINISH_TYPE_DECLARATION triggered when the type is declared but before it is finally defined. Hmmh. For this specific case, maybe just setting the TREE_DEPRECATED flag on the tree node of type could do what you want The deprecated attribute was just an example. I may need to apply any type attribute when a new type is parsed, which is currently not possible with the current C++ front-end. Romain
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Le 22 sept. 2011 à 16:18, Diego Novillo a écrit : On 11-09-22 09:40 , Dodji Seketeli wrote: Romain Geisslerromain.geiss...@gmail.com a écrit: I tried to fix PLUGIN_FINISH_DECL as well to include typedefs in C++. The followings does not currently trigger the PLUGIN_FINISH_DECL (or not in all cases), but should them ? - function parameters (in the function prototype) - definition (with a function body) of a top-level function (while the exact same function definition enclosed in a class definition will trigger PLUGIN_FINISH_DECL) - label declaration - constants defined by enums - namespace Indeed. finish_decl is not called in those cases. As to if the PLUGIN_FINISH_DECL event should be emitted for those, I'd say yes, at least if I believe what the description in plugin.def says: /* After finishing parsing a declaration. */ DEFEVENT (PLUGIN_FINISH_DECL) But I'd rather ask what the maintainers think about it. Jason, Diego? Yes, those events should trigger a PLUGIN_FINISH_DECL call. Ok, i've already implemented it in the C front-end. I'll post the whole patch soon. Romain
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Romain Geissler romain.geiss...@gmail.com a écrit: The deprecated attribute was just an example. I may need to apply any type attribute when a new type is parsed, which is currently not possible with the current C++ front-end. So then it seems to me like you might need to emit an event when a new declared named is about to be put in the symbol table for a given scope. For the c++ FE, that would be in pushdecl_maybe_friend. That way, you can see the DECL even for the declaration of a type, before the type is fully parsed. -- Dodji
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Hi
2011/9/12 Dodji Seketeli do...@seketeli.org:
Romain Geissler romain.geiss...@gmail.com a écrit:
When i recontributed the PLUGIN_FINISH_DECL patch from the original
Brian Hackett, i didn't exactly checked what may or may not trigger
this new event. I know for example that declaring a function triggers
this event, but defining it does not.
I don't really know when those event should be triggered, we should
clarify the meaning of those.
According to me:
PLUGIN_FINISH_DECL should be triggered anytime the parser parse a
declaration (which includes declaration + definition of function,
typedef definition, namespaces, ..., all DECL_P trees built by the
parser).
The general idea sounds sensible, IMHO. However, we must keep in mind
that there are cases like, e.g, 'struct C;' where G++ creates a typedef
'typedef struct C C;' so that you can name that type 'C' instead of
having to type struct C' all the time. For these cases, I don't think
the PLUGIN_FINISH_DECL event should be emitted.
I agree.
For, PLUGIN_FINISH_TYPE i don't really know it means a new type
declaration (or declaration + definition) or if it means usage of a
type (in a function prototype, the type of a local variable.
I'd say it's a definition of a new type, IMHO.
Ok, so it only involves struct, unions, enum and class declaration /
definitions.
I would rather vote for new type definition (including typedefs)
My understanding is that a typedef declaration doesn't define a new
type. Rather, it declares an alias for an existing type. As such, I
would think that notifying typedef declarations via PLUGIN_FINISH_DECL
would be the way to go.
Ok
but for special cases of struct, you can declare and use them at the
same time:
Just to be sure I understand, do you need to be notified about *uses* of
types and decls as well? If so, maybe a new kind of event should
probably be defined, because PLUGIN_FINISH_DECL and PLUGIN_FINISH_TYPE
seem to have more to do with declaring/defining decls and types than
using them.
Personally i don't need to catch all struct uses, but i need to catch struct
declarations.
I want to apply some __attributes__ to a given struct type, for example let's say i need
to mark *struct my_struct* as deprecated thanks to a plugin. I know how to apply some
attributes to a type or a decl. See the plugin (test_plugin.c) and the test files attached.
With test_pass.c, my_struct is declared first, then defined after, applying the deprecated
attribute works.
With test_fail.c, my_struct is declared and defined at the same time, applying the
deprecated attribute doesn't work with the current trunk (and also with my patch).
I got:
test_fail.c:4:1: warning: type attributes ignored after type is already defined
[-Wattributes]
So i may need a PLUGIN_FINISH_TYPE_DECLARATION triggered when the type is declared but
before it is finally defined.
Does two different events PLUGIN_FINISH_TYPE_DECLARATION and PLUGIN_FINISH_TYPE_DEFINITION
make sens to you ?
--
Dodji
Romain Geissler
struct my_struct {
int field_1;
int field_2;
};
struct my_struct *my_function();
struct my_struct x = {1, 2};
struct my_struct *my_function(){
return x;
}
struct my_struct *my_function();
struct my_struct {
int field_1;
int field_2;
};
struct my_struct x = {1, 2};
struct my_struct *my_function(){
return x;
}
#include gcc-plugin.h
#include plugin-version.h
#include tree.h
#include cp/cp-tree.h
#pragma weak cplus_decl_attributes
bool my_struct_found = false;
void finish_type_callback (void *gcc_data, void *data ATTRIBUTE_UNUSED) {
tree type = (tree)gcc_data;
const char *type_name;
if (my_struct_found || type == error_mark_node || DECL_P (type)) {
//current PLUGIN_FINISH_TYPE is buggy in C++
//and my current patch may return TYPE_DECL for typedefs
return;
}
if (TREE_CODE (TYPE_NAME (type)) == IDENTIFIER_NODE) {
type_name = IDENTIFIER_POINTER (TYPE_NAME (type));
} else {
type_name = IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (type)));
}
if (strcmp (type_name, my_struct)) {
//only affect my_struct type
return;
}
my_struct_found = true;
tree deprecated_attribute = tree_cons (get_identifier (deprecated),
NULL_TREE, NULL_TREE);
inform(input_location, my_struct found);
//apply the deprecated attribute to the my_struct type.
if (cplus_decl_attributes) {
cplus_decl_attributes (type, deprecated_attribute,
ATTR_FLAG_TYPE_IN_PLACE);
} else {
decl_attributes (type, deprecated_attribute,
ATTR_FLAG_TYPE_IN_PLACE);
}
}
int plugin_is_GPL_compatible;
int plugin_init (struct plugin_name_args *plugin_info,
struct plugin_gcc_version *version ATTRIBUTE_UNUSED)
{
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Hello Romain,
Romain Geissler romain.geiss...@st.com a écrit:
This patch solves some lacks with the PLUGIN_FINISH_TYPE event
triggering. For now, both C and C++ parser won't trigger it for enums or
for typedef.
AFAICT, typedef declarations are reported by the PLUGIN_FINISH_DECL
event. The predicate is_typedef_decl then returns TRUE when passed the
decl carried by the event. Maybe the plugin's documentation could use
an addendum for this.
[...]
gcc/cp/
2011-09-12 Romain Geissler romain.geiss...@gmail.com
* decl.c (grokdeclarator): Trigger PLUGIN_FINISH_TYPE for typedefs.
[...]
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 178252)
+++ gcc/cp/decl.c (working copy)
@@ -9682,6 +9682,9 @@ grokdeclarator (const cp_declarator *dec
memfn_quals != TYPE_UNQUALIFIED,
inlinep, friendp, raises != NULL_TREE);
+ if (TREE_TYPE (decl) != error_mark_node !DECL_ARTIFICIAL (decl))
+invoke_plugin_callbacks (PLUGIN_FINISH_TYPE, decl);
+
return decl;
}
So I think this change is not necessary.
[...]
Index: gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c
===
--- gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c (revision 0)
@@ -0,0 +1,56 @@
+// Test case for the dumb plugin.
+// { dg-do compile }
+// { dg-options -O -fplugin-arg-dumb_plugin-ref-pass-name=ccp
-fplugin-arg-dumb_plugin-ref-pass-instance-num=1 }
+
Here, it would be nice to add a comment saying which plugin acts on this
plugin's test and what the plugin does (what the output is supposed to
be). That redundant information eases the review, at very least.
Thanks.
--
Dodji
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Hi,
2011/9/12 Dodji Seketeli do...@seketeli.org:
Hello Romain,
Romain Geissler romain.geiss...@st.com a écrit:
This patch solves some lacks with the PLUGIN_FINISH_TYPE event
triggering. For now, both C and C++ parser won't trigger it for enums or
for typedef.
AFAICT, typedef declarations are reported by the PLUGIN_FINISH_DECL
event. The predicate is_typedef_decl then returns TRUE when passed the
decl carried by the event. Maybe the plugin's documentation could use
an addendum for this.
I just checked again, and PLUGIN_FINISH_DECL is triggered for a
typedef in C mode, but not in C++ mode. I'll patch that.
When i recontributed the PLUGIN_FINISH_DECL patch from the original
Brian Hackett, i didn't exactly checked what may or may not trigger
this new event. I know for example that declaring a function triggers
this event, but defining it does not.
I don't really know when those event should be triggered, we should
clarify the meaning of those.
According to me:
PLUGIN_FINISH_DECL should be triggered anytime the parser parse a
declaration (which includes declaration + definition of function,
typedef definition, namespaces, ..., all DECL_P trees built by the
parser).
For, PLUGIN_FINISH_TYPE i don't really know it means a new type
declaration (or declaration + definition) or if it means usage of a
type (in a function prototype, the type of a local variable. I would
rather vote for new type definition (including typedefs) but for
special cases of struct, you can declare and use them at the same
time:
#include stdlib.h
struct my_struct x;
struct my_struct *my_function ();
struct my_struct {
int field_1;
int field_2;
};
struct my_struct *my_function () {
---return NULL;
}
which GCC accepts even with -pedantic.
That's why in this patch i launch a PLUGIN_FINISH_TYPE event every
time the parser meet the struct keyword.
So, what the real meaning of all those events ?
[...]
gcc/cp/
2011-09-12 Romain Geissler romain.geiss...@gmail.com
* decl.c (grokdeclarator): Trigger PLUGIN_FINISH_TYPE for typedefs.
[...]
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 178252)
+++ gcc/cp/decl.c (working copy)
@@ -9682,6 +9682,9 @@ grokdeclarator (const cp_declarator *dec
memfn_quals != TYPE_UNQUALIFIED,
inlinep, friendp, raises != NULL_TREE);
+ if (TREE_TYPE (decl) != error_mark_node !DECL_ARTIFICIAL (decl))
+ invoke_plugin_callbacks (PLUGIN_FINISH_TYPE, decl);
+
return decl;
}
So I think this change is not necessary.
[...]
Index: gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c
===
--- gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/plugin/dumb-plugin-test-1.c (revision 0)
@@ -0,0 +1,56 @@
+// Test case for the dumb plugin.
+// { dg-do compile }
+// { dg-options -O -fplugin-arg-dumb_plugin-ref-pass-name=ccp
-fplugin-arg-dumb_plugin-ref-pass-instance-num=1 }
+
Here, it would be nice to add a comment saying which plugin acts on this
plugin's test and what the plugin does (what the output is supposed to
be). That redundant information eases the review, at very least.
Ok
Thanks.
--
Dodji
Re: [PLUGIN] Fix PLUGIN_FINISH_TYPE
Romain Geissler romain.geiss...@gmail.com a écrit: I just checked again, and PLUGIN_FINISH_DECL is triggered for a typedef in C mode, but not in C++ mode. I'll patch that. Correct. This is because the event is emitted at the end of cp_finish_decl, but that function has many return points. Maybe unifying all the those return points by replacing them with goto out; and adding an out label right at the end, before emitting the event. When i recontributed the PLUGIN_FINISH_DECL patch from the original Brian Hackett, i didn't exactly checked what may or may not trigger this new event. I know for example that declaring a function triggers this event, but defining it does not. I don't really know when those event should be triggered, we should clarify the meaning of those. According to me: PLUGIN_FINISH_DECL should be triggered anytime the parser parse a declaration (which includes declaration + definition of function, typedef definition, namespaces, ..., all DECL_P trees built by the parser). The general idea sounds sensible, IMHO. However, we must keep in mind that there are cases like, e.g, 'struct C;' where G++ creates a typedef 'typedef struct C C;' so that you can name that type 'C' instead of having to type struct C' all the time. For these cases, I don't think the PLUGIN_FINISH_DECL event should be emitted. For, PLUGIN_FINISH_TYPE i don't really know it means a new type declaration (or declaration + definition) or if it means usage of a type (in a function prototype, the type of a local variable. I'd say it's a definition of a new type, IMHO. I would rather vote for new type definition (including typedefs) My understanding is that a typedef declaration doesn't define a new type. Rather, it declares an alias for an existing type. As such, I would think that notifying typedef declarations via PLUGIN_FINISH_DECL would be the way to go. but for special cases of struct, you can declare and use them at the same time: Just to be sure I understand, do you need to be notified about *uses* of types and decls as well? If so, maybe a new kind of event should probably be defined, because PLUGIN_FINISH_DECL and PLUGIN_FINISH_TYPE seem to have more to do with declaring/defining decls and types than using them. So, what the real meaning of all those events ? From plugin.def, I read: /* After finishing parsing a type. */ DEFEVENT (PLUGIN_FINISH_TYPE) /* After finishing parsing a declaration. */ DEFEVENT (PLUGIN_FINISH_DECL) I agree that's rather terse, but it doesn't seem to be related to the use of the type/decls. -- Dodji
