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 792942 - async constructors that throw errors result in code, unbuildable with clang
async constructors that throw errors result in code, unbuildable with clang
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Async
0.38.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2018-01-26 23:06 UTC by Ernestas Kulik
Modified: 2018-02-01 19:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: add test for async ctors that throw (1.41 KB, patch)
2018-01-27 13:46 UTC, Ernestas Kulik
none Details | Review
codegen: fix returns in creation methods (1.41 KB, patch)
2018-01-27 13:46 UTC, Ernestas Kulik
none Details | Review
codegen: Fix return-type for cancelled async creation methods of classes (2.00 KB, patch)
2018-01-27 18:04 UTC, Rico Tzschichholz
committed Details | Review

Description Ernestas Kulik 2018-01-26 23:06:49 UTC
Things like https://gitlab.gnome.org/GNOME/gnome-boxes/blob/72cf086f819a72e49a2127ba36c2e819a3e58be0/src/installed-media.vala#L59 generate code that is unbuildable with clang, because empty returns in non-void functions are a hard error:

/home/ernestas/jhbuild/checkout/gnome-boxes/src/installer-media.vala:64:3: error: non-void function 'boxes_installer_media_construct_for_path_finish' should return a value [-Wreturn-type]
                return;


Reproducible with current stable (0.38.4) and current master (0.39.5.16-fbae7).
Comment 1 Ernestas Kulik 2018-01-26 23:09:02 UTC
(In reply to Ernestas Kulik from comment #0)
> /home/ernestas/jhbuild/checkout/gnome-boxes/src/installer-media.vala:64:3:
> error: non-void function 'boxes_installer_media_construct_for_path_finish'
> should return a value [-Wreturn-type]
>                 return;
> 

That’s from a different file, my bad, but those are sprinkled throughout the codebase.
Comment 2 Ernestas Kulik 2018-01-27 11:44:22 UTC
I’m cooking up a patch. Got Boxes to build, but now working on adding a test.
Comment 3 Ernestas Kulik 2018-01-27 13:46:37 UTC
Created attachment 367518 [details] [review]
tests: add test for async ctors that throw
Comment 4 Ernestas Kulik 2018-01-27 13:46:45 UTC
Created attachment 367519 [details] [review]
codegen: fix returns in creation methods

Currently, returns are generated with the return type of the creation
method, but that fails if the creation method is also the constructor of
a class/struct. This commit adds code to return valid values of parent
types in those cases.
Comment 5 Ernestas Kulik 2018-01-27 13:47:17 UTC
(In reply to Ernestas Kulik from comment #3)
> Created attachment 367518 [details] [review] [review]
> tests: add test for async ctors that throw

This actually reveals more broken things, but I’m not yet sure how to go about those.
Comment 6 Rico Tzschichholz 2018-01-27 18:04:39 UTC
Created attachment 367530 [details] [review]
codegen: Fix return-type for cancelled async creation methods of classes

clang actually fails with -Werror=return-type
Comment 7 Rico Tzschichholz 2018-01-27 18:09:49 UTC
Hi, thanks for the patch, I have optimized it a bit and reduced it to only ObjectTypeSymbols, since as you noticed this looks like even more trouble with Structs.

Struct "constructors" mostly have two types depending on if the struct can be handled as pointer/nullable type or simple-type.

I prefer adding a test-case simultaneously.
Comment 8 Rico Tzschichholz 2018-01-27 21:27:45 UTC
Comment on attachment 367530 [details] [review]
codegen: Fix return-type for cancelled async creation methods of classes

Attachment 367530 [details] pushed as 6c50cfb - codegen: Fix return-type for cancelled async creation methods of classes