Bug 107838

Summary: Objective-C API: testapi.mm should use ARC
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: mrowe, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 108860    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch mrowe: review+

Mark Hahnenberg
Reported 2013-01-24 09:21:44 PST
In ToT testapi.mm uses the Obj-C garbage collector, which hides a lot of our object lifetime bugs. We should enable ARC, since that is what most of our clients will be using.
Attachments
Patch (16.20 KB, patch)
2013-02-04 14:01 PST, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (16.66 KB, patch)
2013-02-04 15:53 PST, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (3.62 KB, patch)
2013-02-07 14:30 PST, Mark Hahnenberg 		mrowe: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-01-24 11:48:07 PST
<rdar://problem/13079712>
Mark Hahnenberg
Comment 2 2013-02-04 14:01:31 PST
Created attachment 186456 [details] Patch
Mark Hahnenberg
Comment 3 2013-02-04 15:53:40 PST
Created attachment 186483 [details] Patch
Mark Hahnenberg
Comment 4 2013-02-04 15:55:46 PST
New patch that disables building with ARC on 32-bit.
Oliver Hunt
Comment 5 2013-02-04 16:14:35 PST
Comment on attachment 186483 [details] Patch Does this compile on 32bit? You've disabled arc but still have @autoreleasepool
Mark Hahnenberg
Comment 6 2013-02-04 16:18:06 PST
(In reply to comment #5) > (From update of attachment 186483 [details]) > Does this compile on 32bit? You've disabled arc but still have @autoreleasepool Yes, it builds. @autoreleasepools are unrelated to ARC.
WebKit Review Bot
Comment 7 2013-02-05 12:40:22 PST
Comment on attachment 186483 [details] Patch Clearing flags on attachment: 186483 Committed r141914: <http://trac.webkit.org/changeset/141914>
WebKit Review Bot
Comment 8 2013-02-05 12:40:25 PST
All reviewed patches have been landed. Closing bug.
Mark Hahnenberg
Comment 9 2013-02-07 11:37:52 PST
Reopening because we should fix the build files so that all of the settings are in the relevant xcconfig files.
Mark Hahnenberg
Comment 10 2013-02-07 14:30:10 PST
Created attachment 187177 [details] Patch
Mark Rowe (bdash)
Comment 11 2013-02-07 14:38:54 PST
Comment on attachment 187177 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187177&action=review r=me with those changes. Thanks for cleaning this up! > Source/JavaScriptCore/Configurations/ToolExecutable.xcconfig:36 > +CLANG_ENABLE_OBJC_ARC = $(CLANG_ENABLE_OBJC_ARC_$(CURRENT_ARCH)) This should have a trailing ; for consistency. > Source/JavaScriptCore/Configurations/ToolExecutable.xcconfig:39 > +CLANG_ENABLE_OBJC_ARC_ = NO; > +CLANG_ENABLE_OBJC_ARC_i386 = NO; > +CLANG_ENABLE_OBJC_ARC_ppc = NO; Since ARC defaults to off, you can omit all of these lines that explicitly set it to NO.
Mark Hahnenberg
Comment 12 2013-02-07 15:13:15 PST
Committed r142184: <http://trac.webkit.org/changeset/142184>
Note You need to log in before you can comment on or make changes to this bug.