groue / GRMustache

Flexible and production-ready Mustache templates for MacOS Cocoa and iOS
http://mustache.github.com/
MIT License
1.44k stars 190 forks source link

Xcode 5 fixes #52

Closed samdeane closed 11 years ago

samdeane commented 11 years ago

An updated project and some warning fixes for Xcode 5.

groue commented 11 years ago

Thanks for your pull request, Sam. Let me check that :-)

groue commented 11 years ago

Thanks again, Sam.

I won't take the project update (9ac513eb07adb5802edebc89449099e34e8c405d) because Xcode 5 is still beta software, and GRMustache is production code (I won't compile it with Xcode 5).

I also won't take 05c97877f432c4fc37680faf2925023e0c3dbc4b, because there is no guarantee that NSInteger has the size of void*, and the compiler just happens to stop emitting a warning here. It looks that C99 defines intptr_t just for that (an integer type that is guaranteed to be able to hold a pointer).

38a6bdffbdbfea8bf1da827c88fd4fbf4e7c62d5 is perfectly legit, though :-)

Would you update your pull request?

samdeane commented 11 years ago

Fair enough about the boxing, I'd forgotten that you pass in retain/release callbacks when making a CFMutableDictionary - the prospect of the values getting spuriously retained/released was what was bothering me.

An enum for the values would do no harm mind you ;)

I've updated the code to use intptr_t.

samdeane commented 11 years ago

With regard to XC5 being pre-release, sure it is, but that doesn't mean people won't be using it.

The changes I made are intended to be backward compatible with XC4, and if we can verify that, I would have thought it would just make sense to adopt them now. If they aren't compatible, by all means I'll fix them (I'm using XC4 too).

People will still be using XC4 once XC5 has been fully released, so there's never going to be a point where you can safely push an incompatible change (well, maybe not never, but it will be a long time before it would be reasonable to require XC5).

samdeane commented 11 years ago

The current project changes, incidentally, merely turn on some extra warnings and update Xcode's LastUpgradeCheck value.

I think that the unknown warning variables should be ignored by previous compilers, and the LastUpgradeCheck change will just prevent everyone who uses the project under XC5 from being nagged to update the project.

groue commented 11 years ago

Hi again, Sam!

Fair enough about the boxing, I'd forgotten that you pass in retain/release callbacks when making a CFMutableDictionary - the prospect of the values getting spuriously retained/released was what was bothering me.

An enum for the values would do no harm mind you ;)

I've updated the code to use intptr_t.

Thanks for that. I may give those "dictionaries of bools" a proper interface, yes. But actually I don't really care about them. They are just micro-benchmark assistants, and they may well be replaced whenever I want to gain an extra 5% :)

With regard to XC5 being pre-release, sure it is, but that doesn't mean people won't be using it.

The changes I made are intended to be backward compatible with XC4, and if we can verify that, I would have thought it would just make sense to adopt them now. If they aren't compatible, by all means I'll fix them (I'm using XC4 too).

People will still be using XC4 once XC5 has been fully released, so there's never going to be a point where you can safely push an incompatible change (well, maybe not never, but it will be a long time before it would be reasonable to require XC5).

The current project changes, incidentally, merely turn on some extra warnings and update Xcode's LastUpgradeCheck value.

I think that the unknown warning variables should be ignored by previous compilers, and the LastUpgradeCheck change will just prevent everyone who uses the project under XC5 from being nagged to update the project.

All right, you are convincing.

About the warnings, the worse they can do is trigger false positive in Xcode 4... Let's check it.... Yes, they do :-) The "undeclared selector" warning was not robust enough in Xcode4: it would not register selectors implicitly declared by @property declarations.

I can't put my finger (Google used to be good at that) on an amazing stackoverflow answer written by a LLVM guy, who was thoroughly explaining how warnings come to live buggy (nearly blind, or overly picky), get improved with each compiler release, until they eventually become robust enough, and an actual help.

Anyway. Thanks a lot for your help, Sam, I'm happily merging your pull request - I'll just add a few tweaks.

Happy Mustache!

samdeane commented 11 years ago

Thanks - and thanks for GRMustache :)