Add the -Wobjc-missing-property-synthesis flag.

Add the -Wobjc-missing-property-synthesis flag.

Add the -Wobjc-missing-property-synthesis compiler flag for iOS and Mac builds. Because Clang now automatically
synthesizes properties if it's not done explicitly, forgetting to add a @synthesize
directive may result in difficult-to-uncover bugs, as Clang silently synthesizes instance
variables and binds them to the wrong property (see, for example, https://chromereviews.googleplex.com/5618018/).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167783

[Miranda Callahan, 2012-11-13 17:54:34]

Here's a patch that will force an error if auto-property synthesis kicks in. The
error generated is "Auto property synthesis is synthesizing property not
explicitly synthesized."

[rohitrao (ping after 24h), 2012-11-13 18:27:57]

Does this uncover any other problems, or does our whole codebase build with this
flag on?

[Miranda Callahan, 2012-11-13 19:24:04]

On 2012/11/13 18:27:57, rohitrao wrote:
> Does this uncover any other problems, or does our whole codebase build with
this
> flag on?

Uncovers one problem with a popup controller -- as discussed with you on IRC.

[Miranda Callahan, 2012-11-13 19:24:50]

Added Mark for upstream review as discussed.

[Miranda Callahan, 2012-11-14 20:33:33]

On 2012/11/13 19:24:50, Miranda Callahan wrote:
> Added Mark for upstream review as discussed.

Ping to you happy pair.

[Nico, 2012-11-14 21:06:55]

https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi#n...
build/common.gypi:3073: '-Wobjc-missing-property-synthesis',
If you say 'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES' instead, then the
UI will know about this warning too. (ninja won't, but that'd be easy to add.)

If you want to keep the flag, it should probably go in the clang==1 block below

[Miranda Callahan, 2012-11-14 21:21:03]

Excellent, thanks -- please take another look.

On 2012/11/14 21:06:55, Nico wrote:
> https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi
> File build/common.gypi (right):
> 
>
https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi#n...
> build/common.gypi:3073: '-Wobjc-missing-property-synthesis',
> If you say 'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES' instead, then
the
> UI will know about this warning too. (ninja won't, but that'd be easy to add.)
> 
> If you want to keep the flag, it should probably go in the clang==1 block
below

[Nico, 2012-11-14 21:30:33]

https://chromiumcodereview.appspot.com/11361240/diff/11004/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/11361240/diff/11004/build/common.gypi#...
build/common.gypi:3322: 'CLANG_WARN_CXX0X_EXTENSIONS': 'NO',
THis probably wants 'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES' too (or
only this wants this flag, since we don't use property synthesis on os x i
think?)

[mirandac, 2012-11-14 21:51:52]

!

https://chromiumcodereview.appspot.com/11361240/diff/11004/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/11361240/diff/11004/build/common.gypi#...
build/common.gypi:3322: 'CLANG_WARN_CXX0X_EXTENSIONS': 'NO',
On 2012/11/14 21:30:33, Nico wrote:
> THis probably wants 'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES' too
(or
> only this wants this flag, since we don't use property synthesis on os x i
> think?)

It seems so -- I added it above in the IOS and OS_MAC block, but the comment on
line 3314 indicates that this isn't enough; will repeat it here.

[Nico, 2012-11-14 21:52:26]

lgtm

[commit-bot: I haz the power, 2012-11-14 21:57:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mirandac@chromium.org/11361240/5004

[Mark Mentovai, 2012-11-15 00:06:52]

I’m glad Nico had the same comment as me. I don’t know why this didn’t actually
get sent. I sent it like six minutes after you sent me the review yesterday.

For the record, then…

https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/11361240/diff/1009/build/common.gypi#n...
build/common.gypi:3073: '-Wobjc-missing-property-synthesis',
There’s a symbolic name for this

CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS

which you can set to YES above (and then put a comment saying
-Wobjc-missing-property-synthesis per the rest of the compiler options). Does
that work?

We would also need to add ninja support for this symbolic name. We generally use
those symbolic names when they’re available.

[Mark Mentovai, 2012-11-15 00:07:51]

Ensuring I press send this time.

https://chromiumcodereview.appspot.com/11361240/diff/5004/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/11361240/diff/5004/build/common.gypi#n...
build/common.gypi:3094: 'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES',
One nit: we almost always put the -W name in a comment next to the symbolic
name, so people searching for things by the -W name that they might find in a
verbose build log have something in the .gyp files to hit on.

[commit-bot: I haz the power, 2012-11-15 00:25:54]

Change committed as 167783