GNOME Bugzilla – Bug 548969
Don't declare the same function twice.
Last modified: 2009-07-27 17:20:58 UTC
Please describe the problem: In ValacCodeFragments, there was no check if two (logically) identical CCodeNodes are added to the same fragments. At least for CCodeFunction, they should be added for once. It allows us to remove duplica for dynamic method wrappers. This patch adds a static equal method to CCodeNode, and for two CCodeFunctions if the name and return type are identical, it returns true. This patch also adds a test before adding CCodeNode to CCodeFragment if there is already an identical one, it doesn't do anything. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information: patch follows
Created attachment 117196 [details] [review] patch This patch is stripped from the vala CCode snippet patch(BUG 548897 )
Thanks for the patch, however, you seem to have other parts of the snippet patch mixed in. Also, I don't think that this is the right approach, duplicate function detection should be done in the code generator / the CCode binding classes, not in CCodeFragment. Do you have a test case where this issue actually happens?
I am soory, but apparently I attached a wrong file. Even worse I could not find the orignal patch now, and when I was trying to strip a new one, I got lost in the patch. It was long time ago and I can't remember exactly what happened. But you are right the patch should be done in CCodeNode. What I can recall about my idea was to add some new virtual methods to to a CCodeNode to test whether or not two CCodeNodes are identical, and when adding anything to any CCode 'scope' or (saying) fragment, the parent CCodeNode test whether an identical child is already there. If so, the parent node do nothing. By the word 'identical' i mean logically identical, ie: two same function definitions/declarations are identical, But two same 'i++' are not identical. For a test case(use case?), write any code with two dynamic DBus calls for the same function: there will be two wrappers, diffenciated only by a number. With this feature, we don't need to postfix the wrapper with a number anymore: all we need to do in 'dynamicmethodbinding.vala' is just simply adding the wrapper again and again. I don't think the posted patch will work. I can work on a patch purely to remove these identical DBus dynamic method wrappers, if the approach(adding overridable comparefunctions to CCodeNode) sounds reasonable.
I don't think we should do duplicate detection inside CCodeNodes, this should all happen in the code generator.