Bug 164498

Summary: [Cocoa] Enable two clang warnings recommended by Xcode
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, dbates, keith_miller, mark.lam, mitz, msaboff, saam, simon.fraser, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Andy Estes
Reported 2016-11-07 17:31:43 PST
[Cocoa] Enable two clang warnings recommended by Xcode
Attachments
Patch (11.80 KB, patch)
2016-11-29 11:44 PST, Andy Estes 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Andy Estes
Comment 1 2016-11-29 11:44:26 PST
Created attachment 295614 [details] Patch
Mark Lam
Comment 2 2016-11-29 13:10:57 PST
Comment on attachment 295614 [details] Patch r=me
Daniel Bates
Comment 3 2016-11-29 13:14:55 PST
Comment on attachment 295614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295614&action=review > Source/JavaScriptCore/Configurations/Base.xcconfig:40 > +CLANG_WARN_SUSPICIOUS_MOVE = YES; Does this mean that we can get rid of our std::move() overload/WTFMove()?
Andy Estes
Comment 4 2016-11-29 13:31:20 PST
(In reply to comment #3) > Comment on attachment 295614 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=295614&action=review > > > Source/JavaScriptCore/Configurations/Base.xcconfig:40 > > +CLANG_WARN_SUSPICIOUS_MOVE = YES; > > Does this mean that we can get rid of our std::move() overload/WTFMove()? No, we still need WTFMove() for -Wmove to function properly; clang's diagnostic only analyzes single-parameter functions named "move" in the std namespace.
WebKit Commit Bot
Comment 5 2016-11-29 13:37:10 PST
Comment on attachment 295614 [details] Patch Clearing flags on attachment: 295614 Committed r209088: <http://trac.webkit.org/changeset/209088>
WebKit Commit Bot
Comment 6 2016-11-29 13:37:15 PST
All reviewed patches have been landed. Closing bug.
Andy Estes
Comment 7 2016-11-29 13:40:30 PST
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 295614 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=295614&action=review > > > > > Source/JavaScriptCore/Configurations/Base.xcconfig:40 > > > +CLANG_WARN_SUSPICIOUS_MOVE = YES; > > > > Does this mean that we can get rid of our std::move() overload/WTFMove()? > > No, we still need WTFMove() for -Wmove to function properly; clang's > diagnostic only analyzes single-parameter functions named "move" in the std > namespace. Actually we didn't need to enable CLANG_WARN_SUSPICIOUS_MOVE, since -Wmove is included in -Wall. Looks like Xcode isn't smart enough to figure that out. It doesn't hurt to be explicit though, in case that changes.
Note You need to log in before you can comment on or make changes to this bug.