GNOME Bugzilla – Bug 621220
Redundant C code when appending to array
Last modified: 2018-05-22 13:37:31 UTC
When I convert the following code into C language. ---------------- void main () { int[] a = { }; a += 1; a += 1; } ---------------- It is converted into a redundant following C code. ---------------- static void _vala_array_add1 (gint** array, int* length, int* size, gint value) { if ((*length) == (*size)) { *size = (*size) ? (2 * (*size)) : 4; *array = g_renew (gint, *array, *size); } (*array)[(*length)++] = value; } static void _vala_array_add2 (gint** array, int* length, int* size, gint value) { if ((*length) == (*size)) { *size = (*size) ? (2 * (*size)) : 4; *array = g_renew (gint, *array, *size); } (*array)[(*length)++] = value; } ---------------- I think enough by one.
Confirming but it's mostly a cosmetic issue.
*** Bug 622145 has been marked as a duplicate of this bug. ***
Created attachment 187735 [details] [review] Don't generate duplicate dop/copy/add code Only generate dup/copy/add code for distinct parameter list.
Review of attachment 187735 [details] [review]: Why is next_array_*_id still needed if we use the hashmap?
To generate a distinct name for each distinct parameter list.
(In reply to comment #5) > To generate a distinct name for each distinct parameter list. I don't see anything discriminating parameters in your patch that need that index, what am I missing? Can you provide a case where I can see _copy_wrapper2?
void main () { int[] a = { }; double[] b = { }; a += 1; a += 1; b += 1.0; b += 1.0; }
Oh, indeed I somehow read in the patch you were using the hash to get the function name as well, I was wrong sorry. I think it is sane enough to not use the type cname because of possible pointers/clashes. We should use this method in other parts of the codegen as well. What about using right the type cname as hash rather than concatenating _vala_array_* at this point, if I'm not missing anything again?
Alternatively, we can keep concatenating "_vala_array_add" and take advantage of add_wrapper rather than creating new hashmaps, which I think better suits the current code.
Created attachment 187787 [details] [review] Patch to remove duplicated wrappers modify add_wrapper codegen/valaccodearraymodule.vala | 97 ++++++++++++++++++++------------ codegen/valaccodebasemodule.vala | 40 +++++++++----- codegen/valaccodedelegatemodule.vala | 2 +- codegen/valaccodemethodcallmodule.vala | 2 +- codegen/valagasyncmodule.vala | 4 +- codegen/valagdbusservermodule.vala | 2 +-
Created attachment 190144 [details] [review] A short patch to fix this.
Oh sorry, I wrote too much soon. My patch is buggy ! I'll fix that.
Created attachment 190152 [details] [review] A short patch to fix it (Not buggy version) Tested successfully !
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/105.