Bertrand/handlebars-objc

ARC restriction errors even with -fno-objc-arc

schell opened this issue · 30 comments

It seems I'm getting ARC restriction errors even when the files involved are being compiled with -fno-objc-arc. I'm guessing some other related files also need to be compiled with -fno-objc-arc.

screenshot 2015-05-01 10 44 28

No - that doesn't seem to solve the problem.

Theses same issues came up for me after switching to Cocoapods 0.37. I'd previously been using 0.35.1 because of this bug. Apparently that bug is fixed, but cocoapods still doesn't play nicely with handlebars-objc. @Bertrand - any ideas?

I've created the simplest possible project with handlebars-objc and cocoapods 0.37.0 and it fails. So this is a legitimate problem.

See here. Hopefully someone will have some ideas...

This is because Cocoapods apply the -fno-objc-arc flag (and others) on a file by file basis in Xcode instead of setting a global compiler flag for the whole target.
As some objc files are generated while compiling (by yacc and lex) these cannot have the flags applied in Xcode beforehand.

A solution is to add the flags added by Cocoapods (eg -fno-objc-arc -w -Xanalyzer -analyzer-disable-checker -Xanalyzer deadcode) in "Other C Flags" for the whole handlebars-objc target.

Please note however that these will be removed after each pod install.

capture-d ecran-2015-05-05-a-17 35 01

Hello,

sorry, I’m busy working on another project right now, but I will look into this as soon as I can.

— bertrand.

On 05 May 2015, at 17:36, Exotic Objects notifications@github.com wrote:

I've created the simplest possible project with handlebars-objc and cocoapods 0.37.0 and it fails. So this is a legitimate problem.

See here https://github.com/ExoticObjects/test_handlebars_w_cocoapods/tree/master. Hopefully someone will have some ideas...


Reply to this email directly or view it on GitHub #15 (comment).

@guillaume-algis - will pod update also remove them? Or just pod install?

Also, thanks for taking the time to make a screenshot :-)

Great fix, but as you said - temporary. pod update also clears these flags.

Still, it's nice to have a workaround until this is addressed. Thanks again.

Yes, thank you for that work around, nice sleuthing. 👍

@guillaume-algis - do you think it would be possible to use a post_install script to re-add these flags after pod install runs?

Something like this.

It makes sense to me conceptually, but I'm not sure which files would need altering. After adding the flags you recommended, I don't see them in any of the xcconfig files...

They are added in the Pod-handlebars-objc.xcodeproj/project.pbxproj file. But it could be possible to add them in the xcconfig instead.
Anyway, this is a rather ugly fix IMO, I'd much prefer fixing the bug upstream in Cocoapods.

But until the bug is addressed in Cocoapods, yes it should probably work.

I don't have a Pod-handlebars-objc.xcodeproj file in my test project.

I just have a top level .xcodeproj and a Pods.xcodeproj, and neither of them have the -fno-objc-arc -w -Xanalyzer -analyzer-disable-checker -Xanalyzer deadcode flags in them. (I've set the flags in XCode)

Am I missing something?

Also, this bug (or issue) has existed in Cocoapods for a long time. I'm not holding my breath for them to address it. So a hacky script might be a good thing to have around...

Oops, they are there like you said. In Pods.xcodeproj - thanks.

Also, this bug (or issue) has existed in Cocoapods for a long time. I'm not holding my breath for them to address it. So a hacky script might be a good thing to have around...

That's the beauty of open source: you can fix it yourself and open a PR ;)

After looking into Cocoapods XCodeproj, I decided to write a cocoapods post_install hook to re-add these flags after pod install (or update) runs. I'd initially thought that it would need to be a real hackjob, but the XCodeproj library is sensible and powerful. So this seems like (maybe) a semi-legit way to deal with the problem? I've tested it in two projects and it works.

Basically, it lets me run the latest version of Cocoapods (0.37.0) while using handlebars-objc. But it should be pretty good at setting any build settings that you might need to modify. Hopefully it will help someone out...

Here's the git repo for the script.

So, how would I go about adding handlebars-objc as a dependency in my podpsec? The post-install hook works in a podfile, but I don't think you can add that in a podspec (at least, not in a third party one).

That said, props for having it work with Carthage :)

@Bertrand - will this ever be addressed on your side? Now that we are forced to use_frameworks! with cocoapods (necessary to use any swift pods), the fixes above no longer work.

@ExoticObjects we're using the following post_install hook with use_frameworks!, and it's working fine:

post_install do |installer|
    installer.pods_project.targets.each do |target|
        target.build_configurations.each do |config|

            # Fix an issue with this pod.
            # See https://github.com/Bertrand/handlebars-objc/issues/15
            if target.name == 'handlebars-objc'
                config.build_settings['OTHER_CFLAGS'] = '-fno-objc-arc -w -Xanalyzer -analyzer-disable-all-checks'
            end
        end
    end
end

Excellent! You saved me some time! Thanks.

EDIT: Well, I spoke too soon. After running your post_install hook, I'm getting the same errors as before.

I pushed a simple demo project to github that describes the problem.

Basically, using the simplest possible Podfile with the post install hook above will allow handlebars-objc to compile (hence my early enthusiasm...). But as soon as I attempt to use handlebars in any way, I get the same old errors.

NSString * str = [HBHandlebars renderTemplateString:@"" withContext:@"" error:nil];

results in:

ARC forbids Objective-C objects in struct

Hopefully I'm missing something simple!

Is it a very large undertaking to update the code for ARC? Or are there other interests at play here?

@ExoticObjects comment out the line that is throwing the error. Not a perfect solution (by any means) but it works.

I submitted a pull request. @Bertrand, please have a look! I can't think of any reason that this tiny change would break anything, but...

@ExoticObjects Just use #import "HBHandlebars.h" instead of #import <HBHandlebars/HBHandlebars.h>.

Also, your example project overrides the OTHER_CFLAGS setting at the target level. Delete all the occurrences of OTHER_CFLAGS = ""; in your pbxproj.

(Cocoapods complain about this after a pod install btw: )

[!] The `MainTarget [Debug]` target overrides the `OTHER_CFLAGS` build setting defined in `Pods/Target Support Files/Pods/Pods.debug.xcconfig'. This can lead to problems with the CocoaPods installation
    - Use the `$(inherited)` flag, or
    - Remove the build settings from the target.

[!] The `MainTarget [Release]` target overrides the `OTHER_CFLAGS` build setting defined in `Pods/Target Support Files/Pods/Pods.release.xcconfig'. This can lead to problems with the CocoaPods installation
    - Use the `$(inherited)` flag, or
    - Remove the build settings from the target.

@schell ARC slightly slows down code when a lot of objects are created. It's totally fine to use it in an application, but since in objc we have a choice, it's better to not use it in libraries. In swift of course it's a different story :)

it's better to not use [ARC] in libraries

I respectfully disagree. Not using ARC today is a major downside for 99.99% of the libraries (only exceptions are maybe performance-critical libs and, let's be honest, handlebars-objc isn't one of them).

ARC "performance downside" (if this is even a thing) is largely compensated by what it brings to the table.

Let me also use this as an occasion to remind you that I had a PR (#12) opened for more than a year to fix multiple memory leaks in this lib. Leaks which would have never existed with ARC enabled.

@guillaume-algis In my experience, on (big) real-life projects, moving to ARC incurrs at least 20% performance loss scattered all over the code and thus very hard to fix (due mostly to additional retain-release + atomic RC by default).

Performance loss may be fine for some apps (I generally use ARC for apps), but as a library, handlebars-objc cannot make that choice on behalf of developers.

GRMustache made the same choice, for exactly the same reason.

"handlebars-objc is not performance critical"
??????
I used handlebars-objc for batch-generation of text file, and had to optimise the lib quite a bit (in particular string generation) to get this fast.

Regarding the leaks, I'm sorry for that. I was away from all this this last year. I'm looking at your PR asap.

I'm glad your lib has the performance that it does!
On Wed, Jan 20, 2016 at 8:21 AM Bertrand Guiheneuf notifications@github.com
wrote:

@guillaume-algis https://github.com/guillaume-algis In my experience,
on (big) real-life projects, moving to ARC incurrs at least 20% performance
loss scattered all over the code and thus very hard to fix (due mostly to
additional retain-release + atomic RC by default).

Performance loss may be fine for some apps (I generally use ARC for apps),
but as a library, handlebars-objc cannot make that choice on behalf of
developers.

GRMustache made the same choice, for exactly the same reason.

"handlebars-objc is not performance critical"
??????
I used handlebars-objc for batch-generation of text file, and had to
optimise the lib quite a bit (in particular string generation) to get this
fast.

Regarding the leaks, I'm sorry for that. I was away from all this this
last year. I'm looking at your PR asap.


Reply to this email directly or view it on GitHub
#15 (comment)
.

Hmm, did not mean to close this one.

I just released v1.4.3, still based on Handlebars 2.0 but which should fix the issue.
Thanks to everyone who helped tracking down the issue.

@ExoticObjects is the issue solved on your side with the new pod version?

Yes, thanks!