135889 – Clean up build warnings: control reaches end of non-void function

Bug 135889 - Clean up build warnings: control reaches end of non-void function

Summary: Clean up build warnings: control reaches end of non-void function

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	New Bugs (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Yusuke Suzuki

URL:
Keywords:

Depends on:
Blocks:

Reported:	2014-08-13 11:07 PDT by Yusuke Suzuki
Modified:	2014-08-23 00:13 PDT (History)
CC List:	5 users (show)

See Also:

Attachments
Patch (6.15 KB, patch) 2014-08-13 11:08 PDT, Yusuke Suzuki	no flags	Details \| Formatted Diff \| Diff
Patch (6.15 KB, patch) 2014-08-13 11:18 PDT, Yusuke Suzuki	no flags	Details \| Formatted Diff \| Diff
Patch (6.24 KB, patch) 2014-08-18 14:40 PDT, Yusuke Suzuki	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Yusuke Suzuki 2014-08-13 11:07:53 PDT

Clean up build warnings: control reaches end of non-void function

Comment 1 Yusuke Suzuki 2014-08-13 11:08:19 PDT

Created attachment 236535 [details]
Patch

Comment 2 Yusuke Suzuki 2014-08-13 11:18:10 PDT

Created attachment 236536 [details]
Patch

Comment 3 Alexey Proskuryakov 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?

Comment 4 Yusuke Suzuki 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.

Comment 5 Alexey Proskuryakov 2014-08-15 23:46:46 PDT

What do you think about disabling this warning? It seems clearly invalid to me.

Comment 6 Benjamin Poulain 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.

Comment 7 Alexey Proskuryakov 2014-08-17 22:44:08 PDT

Wasn't that discussion about old-style enums, not strongly typed enums?

Comment 8 Yusuke Suzuki 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 :)

Comment 9 Alexey Proskuryakov 2014-08-18 09:40:46 PDT

I don't have a strong opinion. Just like to keep the code clean.

Comment 10 Oliver Hunt 2014-08-18 13:33:01 PDT

Comment on attachment 236536 [details]
Patch

Replace ASSERT_NOT_REACHED with RELEASE_ASSERT_NOT_REACHED()

Comment 11 Yusuke Suzuki 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.

Comment 12 Yusuke Suzuki 2014-08-18 14:40:52 PDT

Created attachment 236787 [details]
Patch

Comment 13 Benjamin Poulain 2014-08-19 22:54:57 PDT

I am okay with this.

Alexey?

Comment 14 Yusuke Suzuki 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 :)

Comment 15 WebKit Commit Bot 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>

Comment 16 WebKit Commit Bot 2014-08-23 00:13:16 PDT

All reviewed patches have been landed.  Closing bug.