After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778224 - Codegen creating void returns throwing error in method returning a SimpleType
Codegen creating void returns throwing error in method returning a SimpleType
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
0.34.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-06 03:11 UTC by Michael Gratton
Modified: 2017-02-10 08:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Minimal-ish test case (341 bytes, text/plain)
2017-02-06 03:14 UTC, Michael Gratton
  Details
Don't return void for non-nullable SimpleType structs by default (1.51 KB, patch)
2017-02-06 05:55 UTC, Michael Gratton
rejected Details | Review
codegen: Don't return void for non-nullable simple-type structs (2.28 KB, patch)
2017-02-06 14:43 UTC, Rico Tzschichholz
committed Details | Review

Description Michael Gratton 2017-02-06 03:11:05 UTC
If a method returns a SimpleType struct, and has a block in which an exception is thrown, then the return statement in the generated code when the error is thrown is void, rather than some default value such as NULL.

Eg:

> [SimpleType]
> struct Foo { ... }
> 
> Foo foo() {
>   if (blah) {
>     throw new Example.EXAMPLE("");
>     // void return generated here
>   }
>   ...
> }

Generates for foo:

> Foo foo (GError** error) {
>   Foo result = {0};
>   ...
>   if (_tmp0_) {
>     GError* _tmp1_ = NULL;
>     _tmp1_ = g_error_new_literal (EXAMPLE, EXAMPLE_EXAMPLE, "");
>     _inner_error_ = _tmp1_;
>     g_propagate_error (error, _inner_error_);
>     return;
>   }
>   ...
>   return result;
> }

Note the void return at the end of the if block, I'd expect it to be "return result" or "return NULL".

This happens for both 0.34.4 and git master.
Comment 1 Michael Gratton 2017-02-06 03:12:14 UTC
NB, while gcc generates a warning for the void return, clang I think errors out on it by default, see Bug 778046.
Comment 2 Michael Gratton 2017-02-06 03:14:09 UTC
Created attachment 345009 [details]
Minimal-ish test case

A compilable example that triggers the bug.
Comment 3 Michael Gratton 2017-02-06 05:55:48 UTC
Created attachment 345013 [details] [review]
Don't return void for non-nullable SimpleType structs by default

Attached patch fixes the problem for me.

Both Vala.GErrorModule::return_with_exception and ::uncaught_error_statement were calling ::return_default_value to output the return value, so I updated that to check for a non-null SimpleType struct and if found, 0-initialise a new blank var and return that.
Comment 4 Rico Tzschichholz 2017-02-06 09:35:05 UTC
Please recheck your patch, make sure your patch compiles and passes make check.

It is weird that gcc doesn't fail with -Werror=return-type in the first place.
Comment 5 Rico Tzschichholz 2017-02-06 14:43:46 UTC
Created attachment 345034 [details] [review]
codegen: Don't return void for non-nullable simple-type structs
Comment 6 Michael Gratton 2017-02-06 23:24:12 UTC
(In reply to Rico Tzschichholz from comment #4)
> Please recheck your patch, make sure your patch compiles and passes make
> check.
> 
> It is weird that gcc doesn't fail with -Werror=return-type in the first
> place.

Ah, sorry, will do. Updating it seems to be a bit non-trivial, will get to it when I have a moment to find out what happened to ::create_local.
Comment 7 Rico Tzschichholz 2017-02-07 13:00:32 UTC
I guess you noticed I already pushed an updated patch for testing?
Comment 8 Rico Tzschichholz 2017-02-10 08:21:44 UTC
Attachment 345034 [details] pushed as c73d19c - codegen: Don't return void for non-nullable simple-type structs