Bug 135889

Summary: Clean up build warnings: control reaches end of non-void function
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, commit-queue, darin, ysuzuki
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Yusuke Suzuki
Reported 2014-08-13 11:07:53 PDT
Clean up build warnings: control reaches end of non-void function
Attachments
Patch (6.15 KB, patch)
2014-08-13 11:08 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (6.15 KB, patch)
2014-08-13 11:18 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (6.24 KB, patch)
2014-08-18 14:40 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2014-08-13 11:08:19 PDT
Created attachment 236535 [details] Patch
Yusuke Suzuki
Comment 2 2014-08-13 11:18:10 PDT
Created attachment 236536 [details] Patch
Alexey Proskuryakov
Comment 3 2014-08-15 21:00:22 PDT
What compiler complains about this? These are strongly type enums, so I don't see how this is a valid warning. Perhaps it should be disabled instead?
Yusuke Suzuki
Comment 4 2014-08-15 22:12:20 PDT
(In reply to comment #3) > What compiler complains about this? These are strongly type enums, so I don't see how this is a valid warning. Perhaps it should be disabled instead? Compiler version is gcc 4.9.1, that's a default compiler for Ubuntu 14.04.
Alexey Proskuryakov
Comment 5 2014-08-15 23:46:46 PDT
What do you think about disabling this warning? It seems clearly invalid to me.
Benjamin Poulain
Comment 6 2014-08-17 22:37:13 PDT
(In reply to comment #5) > What do you think about disabling this warning? It seems clearly invalid to me. We decided last year not to disable the warning for GCC. See the thread 'Can we disable "control reaches end of non-void function" warning on Qt?' on webkit-dev.
Alexey Proskuryakov
Comment 7 2014-08-17 22:44:08 PDT
Wasn't that discussion about old-style enums, not strongly typed enums?
Yusuke Suzuki
Comment 8 2014-08-17 22:56:20 PDT
(In reply to comment #7) > Wasn't that discussion about old-style enums, not strongly typed enums? Reading the thread, maybe that's correct. My concern is that disabling this warning is not limited for strongly-typed enums. So personally I think disabling this causes issues when some non-strongly-typed enum value is appended to the existing enums. But I'd like to hear Alexey and Benjamin's opinions :)
Alexey Proskuryakov
Comment 9 2014-08-18 09:40:46 PDT
I don't have a strong opinion. Just like to keep the code clean.
Oliver Hunt
Comment 10 2014-08-18 13:33:01 PDT
Comment on attachment 236536 [details] Patch Replace ASSERT_NOT_REACHED with RELEASE_ASSERT_NOT_REACHED()
Yusuke Suzuki
Comment 11 2014-08-18 14:37:37 PDT
(In reply to comment #10) > (From update of attachment 236536 [details]) > Replace ASSERT_NOT_REACHED with RELEASE_ASSERT_NOT_REACHED() Thanks for your review. That's reasonable and safe. I'll change to it and update the patch.
Yusuke Suzuki
Comment 12 2014-08-18 14:40:52 PDT
Created attachment 236787 [details] Patch
Benjamin Poulain
Comment 13 2014-08-19 22:54:57 PDT
I am okay with this. Alexey?
Yusuke Suzuki
Comment 14 2014-08-22 23:41:25 PDT
(In reply to comment #13) > I am okay with this. > > Alexey? Oh! I've missed Alexey's r+. Thank you for the discussion Alexey and Benjamin. And thx for your review, Oliver. I'll land it :)
WebKit Commit Bot
Comment 15 2014-08-23 00:13:12 PDT
Comment on attachment 236787 [details] Patch Clearing flags on attachment: 236787 Committed r172886: <http://trac.webkit.org/changeset/172886>
WebKit Commit Bot
Comment 16 2014-08-23 00:13:16 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.