Bug 113547 - Compiler warning in IDBTransaction::modeToString
Summary: Compiler warning in IDBTransaction::modeToString
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philip Rogers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-28 17:11 PDT by Philip Rogers
Modified: 2013-03-29 20:03 PDT (History)
4 users (show)

See Also:


Attachments
Fix compiler warning (1.51 KB, patch)
2013-03-28 17:19 PDT, Philip Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2013-03-28 17:11:21 PDT
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20Heapcheck/builds/15910/steps/compile/logs/stdio

third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp: In static member function 'static const WTF::AtomicString& WebCore::IDBTransaction::modeToString(WebCore::IndexedDB::TransactionMode)':
third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:370:1: warning: control reaches end of non-void function [-Wreturn-type]

Patch forthcoming.
Comment 1 Philip Rogers 2013-03-28 17:19:46 PDT
Created attachment 195683 [details]
Fix compiler warning
Comment 2 Darin Adler 2013-03-28 17:22:56 PDT
Comment on attachment 195683 [details]
Fix compiler warning

This looks like a good change to me. Normally, this is our preferred style for switches on enums. We like to leave out the default case from the switch so that some compilers (gcc family ones) can warn if we fail to include one of the enum values. And we like to have an ASSERT_NOT_REACHED after the switch so that we get predictable behavior in release builds, and an assertion in debug builds, if we have a bizarre value at runtime that is not one of the enum values.
Comment 3 Philip Rogers 2013-03-28 17:32:33 PDT
Comment on attachment 195683 [details]
Fix compiler warning

Thanks for the quick and educational review! I wasn't aware of the preferred enum style.
Comment 4 WebKit Review Bot 2013-03-28 17:48:34 PDT
Comment on attachment 195683 [details]
Fix compiler warning

Clearing flags on attachment: 195683

Committed r147187: <http://trac.webkit.org/changeset/147187>
Comment 5 WebKit Review Bot 2013-03-28 17:48:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Joshua Bell 2013-03-29 10:13:54 PDT
Interesting... I'd expect there to be a lurking compiler warning for the similarly structured IDBCursor::directionToString() function.

pdr, is it showing up anywhere?
Comment 7 Philip Rogers 2013-03-29 20:03:50 PDT
(In reply to comment #6)
> Interesting... I'd expect there to be a lurking compiler warning for the similarly structured IDBCursor::directionToString() function.
> 
> pdr, is it showing up anywhere?

It looks like this bot just returns the first warning it encounters. I agree we should be seeing the same issue for directionToString() too.

The bot is now hitting the following warning:
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20Heapcheck/builds/15928/steps/compile/logs/warnings%20%281%29

I've put up a patch in Chromium to fix this new warning: https://codereview.chromium.org/13378002. Obviously, this isn't a sustainable way to fix these warnings so in parallel I'll search around for a presubmit check.