Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Support compiling with ARC under Clang using ifdefs. #17

Merged
merged 1 commit into from

3 participants

@atombender

No description provided.

@shpakovski
Owner

Thank you very much Alex, but I prefer compiler flag -fobjc-no-arc for ARC-enabled projects. Sorry for rejecting this commit :)

@shpakovski shpakovski closed this
@atombender

I don't see how you can prefer the compilation flag unless you consider your code to be legacy code. The point of the commit is to support ARC — seamlessly, without breaking older apps. Granted, it's a small project, but it's still information to maintain.

@shpakovski
Owner

I personally don't like this kind of #ifdef in code, but you must be right. Seamless integration will be appreciated by the newcomers.

@shpakovski shpakovski reopened this
@shpakovski shpakovski merged commit efeaf31 into shpakovski:master
@atombender

Thanks. I don't like ifdefs either, but new projects have ARC enabled, and you will get a compilation error if you don't edit the compilation flags.

@shpakovski
Owner

Thanks for understanding and insisting on this pull request. I much appreciate :)

@sorbits
Collaborator

@alexstaubo This commit complicates future maintenance.

I would suggest instead wrapping the code in retain/(auto)release calls and define them as no-ops for when ARC is enabled.

See XCMemoryUtils.h for an example of such macros.

Also see how that file tests for ARC: We could be building on a compiler w/o the __has_feature function, so we need to test for that as well.

Not sure what kind of future maintenance you are thinking of, but your project is 302 loc, not exactly a maintenance nightmare? What makes most sense in my view is to fork the code into a (frozen) legacy branch and a current one. Unless you plan on maintaining the legacy non-ARC version?

@sorbits
Collaborator

I don’t care about how big the project is or how likely we currently think it is that the code will be changed, adding redundancy is never a good idea!

As for a frozen non-ARC branch, that certainly beats the inline duplication, but I reckon there’s still a lot of software out there not on the latest toolchain (I think you need to build with 10.7 SDK for ARC), so just handling this via macros seems the better choice to me.

That said, this is not my project, I am just providing feedback.

@atombender

(Sorry, didn't see who was replying, assumed it was @shpakovski.)

@shpakovski
Owner

Thank you for the feedback guys!

XCMemoryUtils.h promotes a good idea, but using macros like RELEASE and AUTORELEASE looks even worse than using #ifdef to me. Overall I think that people who are going to reuse this component on the old systems, will have enough experience to change current approach to be compatible with a compiler missing __has_feature.

Let's live with current code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 3 deletions.
  1. +20 −3 MASPreferencesWindowController.m
View
23 MASPreferencesWindowController.m
@@ -38,7 +38,11 @@ - (id)initWithViewControllers:(NSArray *)viewControllers title:(NSString *)title
{
if ((self = [super initWithWindowNibName:@"MASPreferencesWindow"]))
{
+#if __has_feature(objc_arc)
+ _viewControllers = viewControllers;
+#else
_viewControllers = [viewControllers retain];
+#endif
_minimumViewRects = [[NSMutableDictionary alloc] init];
_title = [title copy];
}
@@ -49,13 +53,13 @@ - (void)dealloc
{
[[NSNotificationCenter defaultCenter] removeObserver:self];
[[self window] setDelegate:nil];
-
+#if !__has_feature(objc_arc)
[_viewControllers release];
[_selectedViewController release];
[_minimumViewRects release];
[_title release];
-
[super dealloc];
+#endif
}
#pragma mark -
@@ -160,7 +164,10 @@ - (NSToolbarItem *)toolbar:(NSToolbar *)toolbar itemForItemIdentifier:(NSString
toolbarItem.target = self;
toolbarItem.action = @selector(toolbarItemDidClick:);
}
- return [toolbarItem autorelease];
+#if !__has_feature(objc_arc)
+ [toolbarItem autorelease];
+#endif
+ return toolbarItem;
}
#pragma mark -
@@ -216,11 +223,17 @@ - (void)setSelectedViewController:(NSViewController <MASPreferencesViewControlle
return;
}
+#if __has_feature(objc_arc)
+ [self.window setContentView:[[NSView alloc] init]];
+#else
[self.window setContentView:[[[NSView alloc] init] autorelease]];
+#endif
if ([_selectedViewController respondsToSelector:@selector(viewDidDisappear)])
[_selectedViewController viewDidDisappear];
+#if !__has_feature(objc_arc)
[_selectedViewController release];
+#endif
_selectedViewController = nil;
}
@@ -267,7 +280,11 @@ - (void)setSelectedViewController:(NSViewController <MASPreferencesViewControlle
[self.window setFrame:newFrame display:YES animate:[self.window isVisible]];
+#if __has_feature(objc_arc)
+ _selectedViewController = controller;
+#else
_selectedViewController = [controller retain];
+#endif
if ([controller respondsToSelector:@selector(viewWillAppear)])
[controller viewWillAppear];
Something went wrong with that request. Please try again.