Oops, I think I broke Travis CI.
I am returning a (small) structure when I should be using pointers. This commit should fix this mess, my apologies. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/517
-- Commit Summary --
* lineoperations: fixed -Werror=aggregate-return
-- File Changes --
M lineoperations/src/lineoperations.c (72)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/517.patch https://github.com/geany/geany-plugins/pull/517.diff
@smostertdev pushed 1 commit.
83cab98 lineoperations: removed check for doc
Weird that Travis breaks for perfectly valid code.
I think -Werror=aggregate-return is too aggressive and shouldn't be used. I run into the same problem in https://github.com/geany/geany/pull/1263 where some some functions in ctags return MIOPos which is a tiny structure. I'll prepare a patch to remove this flag from the build - this pull request will then be unnecessary.
Created https://github.com/geany/geany-plugins/pull/529. Depending whether it's OK the first patch from this pull request could be dropped. The second is still necessary as build fails without it.
b4n commented on this pull request.
Looks good apart from the unnecessary allocations.
Also, I'm find with depending on @techee's PR, so you can drop the first commit if you want.
gint num_chars = 0;
gint i = 0; gint lines_affected = 0;
+ struct lo_lines *sel = g_malloc(sizeof(struct lo_lines));
instead of allocating on the heap you could pass `&sel` to the `get_current_sel_lines()` call below
@@ -223,18 +227,26 @@ action_sci_manip_item(GtkMenuItem *menuitem, gpointer gdata)
/* function pointer to gdata -- function to be used */ gint (*func)(ScintillaObject *, gint, gint) = gdata; GeanyDocument *doc = document_get_current(); - struct lo_lines sel = get_current_sel_lines(doc->editor->sci); + + struct lo_lines *sel = g_malloc(sizeof(struct lo_lines));
ditto
Thanks for all the input, guys. Since the aggregate-return check was removed from Travis, should I still go ahead and make the small changes for this PR? Do you have a preference on returning small structs/pointers @b4n? Either way is fine with me, I'm just lazy.
@smostertdev For now I prefer not returning aggregates, but it's really your call as I can't really back my opinion up with facts right now, and @elextr and @codebrainz seem happy about it.
@b4n I'm indifferent, I just mentioned that it's legal code.
@b4n, the more its discussed the more convinced I become that returns of small values via pointer parameters instead of the function return value is ugly and inefficient. It limits the benefits of inlining and defeats the compiler's ability to employ copy elision and requires the caller to explicitly declare a variable. GCC's `aggregate-return` warning should be changed to `large-aggregate-return` instead.
@codebrainz @b4n How going on here?
@b4n Please review as soon as you have some spare time. Will postpone it to another release
@b4n Ping ;)
@frlan, be patient, your previous ping isn't quite a year ago yet :)
LGBI
Merged #517 into master.
github-comments@lists.geany.org