«
»

Warnings I turn on, and why

2009-11-07 20:51:10 -08:00

I’ve started turning on most of Xcode’s warning options and one warning-related build setting in all of my personal projects. I suggest you do the same.

ADDED 2013-09-27: The easiest way to do this nowadays is to use this xcconfig file, which turns all of the settings described here on at once.

There are some warnings I don’t turn on, for any of several reasons:

  • They’re inapplicable. For example, “‘Effective C++’ Violations” doesn’t apply to me, because I don’t write C++ code.
  • They don’t help anything. An example is “Four Character Literals”, which is about 'abcd' literals for four-byte-code types such as OSType. These sacrifice something convenient for no benefit, so I leave them off.
  • They are impossible for me to fix. An example is “Multiple Definition Types for Selector”. Cocoa raises that one all over its headers, and I can’t do anything about Cocoa.

The rest of the warnings, I turn on because either they make something clearer or they tell me about either real or potential (i.e., future real) bugs. They are:

  • Check Switch Statements

    Warn whenever a switch statement has an index of enumeral type and lacks a case for one or more of the named codes of that enumeration. The presence of a default label prevents this warning.

    Leave no case unhandled.

    Consider whether a default: label is appropriate for your enumeration. If your switch statement handles all possible values, cut out the default and assert that the value is one of the possible ones instead. An easy way to do this, if the enumeration values are serial and the enumeration is not part of an API you expose, is to have one extra name defined as the number of possible values:

    enum Foo {
	kFooFoo,
	kFooBar,
	kFooBaz,
	kFooNumberOfValidFooValues
};

    Then, in your assertion macro, compare the value against that name:

    #define MyParameterAssertValidFoo(foo) \
	NSAssert1((foo) < kFooNumberOfValidFooValues, @"Invalid Foo value: %d", (foo));

    When you add kFooQux, insert it above kFooNumberOfValidFooValues, and the value of kFooNumberOfValidFooValues will increase by one to fit it.

    The result is that your switch statement covers all known values for the enumeration (or you get a warning because it doesn't), and your method throws an exception (from the assertion) whenever anyone passes an unknown value.

  • Hidden Local Variables

    Warn whenever a local variable shadows another local variable, parameter or global variable or whenever a built-in function is shadowed.

    One common way to get this warning is to name a variable index, because there is a function by that name in the standard C library. That's not as much of a false positive as you may think: If you fail to declare your index variable, all your references to it will actually refer to the index function. You can see how it would be bad to send a message such as [myArray objectAtIndex:index] with this bug.

    The solution is simple: Never, ever name a variable index.

  • Implicit Conversion to 32 Bit Type

    Warn if a value is implicitly converted from a 64 bit type to a 32 bit type.

    This is most useful when converting old code to work correctly in a 64-bit architecture. Storing a pointer into an int variable (such as a reference constant) when targeting an LP64 architecture is a good way to get this warning, and rightly so.

  • Initializer Not Fully Bracketed

    Example, Here initializer for a is not fully bracketed, but that for b is fully bracketed.

    	int a[2][2] = { 0, 1, 2, 3 };
	int b[2][2] = { { 0, 1 }, { 2, 3 } };

    This is a cleanliness warning. It also applies to structures, such as NSRect:

    NSRect warns = { 0.0f, 0.0f, 640.0f, 480.0f };
NSRect doesNotWarn = { { 0.0f, 0.0f }, { 640.0f, 480.0f } };

    (In real code, I'm more likely to use NSZeroPoint instead of the { 0.0f, 0.0f } element above. It's harder to spell that wrong and get away with it than it is to get away with typing 9.9f, 1.1f, or 2.2f instead of 0.0f.)

  • Mismatched Return Type

    Causes warnings to be emitted when a function with a defined return type (not void) contains a return statement without a return-value. Also emits a warning when a function is defined without specifying a return type.

  • Missing Braces and Parentheses

    Warn if parentheses are omitted in certain contexts, such as when there is an assignment in a context where a truth value is expected, or when operators are nested whose precedence people often get confused about.

    Also warn about constructions where there may be confusion to which if statement an else branch belongs. Here is an example of such a case:

    	if (a)
		if (b)
			foo ();
	else
		bar ();

    In C, every else branch belongs to the innermost possible if statement, which in this example is if (b). This is often not what the programmer expected, as illustrated in the above example by indentation the programmer chose.

    This may appear to be just a cleanliness warning, but as you can see from the example, it can also warn you about code that may not flow the way you expect it to.

  • Missing Fields in Structure Initializers

    Warn if a structure's initializer has some fields missing. For example, the following code would cause such a warning, because "x.h" is implicitly zero:

        struct s { int f, g, h; };
    struct s x = { 3, 4 };

    This option does not warn about designated initializers, so the following modification would not trigger a warning:

        struct s { int f, g, h; };
    struct s x = { .f = 3, .g = 4 };

    I'm not sure why it warns about the former and not the latter, since all the members get initialized in both code examples (C99 §6.7.8 ¶21). If nothing else, this warning is good motivation for you to switch to designated initializers, which make your code more explicit about which members it's initializing.

  • Missing Newline At End Of File

    Another cleanliness warning—this one, about the cleanliness of diffs.

  • Sign Comparison

    Warn when a comparison between signed and unsigned values could produce an incorrect result when the signed value is converted to unsigned.

  • Strict Selector Matching

    Warn if multiple methods with differing argument and/or return types are found for a given selector when attempting to send a message using this selector to a receiver of type "id" or "Class". When this setting is disabled, the compiler will omit such warnings if any differences found are confined to types which share the same size and alignment.

    I don't turn this one on, because it's unnecessary. When the multiple declarations differ significantly (e.g., one method returns an object and the other returns a float), the compiler will raise the warning whether it's turned on or not. When the declarations don't differ significantly (e.g., both methods return an object), the difference won't cause a problem, so you don't need to worry about it.

    So, you should leave this one off.

  • Typecheck Calls to printf/scanf

    Check calls to printf and scanf , etc, to make sure that the arguments supplied have types appropriate to the format string specified, and that the conversions specified in the format string make sense.

    The biggest reason to turn this on is that it checks your use of methods that take a nil-terminated list of arguments:

    NSArray *array = [NSArray arrayWithObjects:@"foo", @"bar"];

    That message should have a nil after the last argument. With this warning turned on, the compiler will point out that I don't.

    The ostensible main reason to turn this on is to have the compiler check your uses of printf and scanf formats. I don't use printf often (and I never use scanf), so that's not so important for me, but when I do, this could come in handy.

    Sadly, it doesn't work on NSLog calls. This has been fixed in Clang as of 2013. Your printf, NSLog, and stringWithFormat calls all get checked (despite the name of the setting not having changed).

  • Undeclared Selector

    Warn if a "@selector(...)" expression referring to an undeclared selector is found. A selector is considered undeclared if no method with that name has been declared before the "@selector(...)" expression, either explicitly in an @interface or @protocol declaration, or implicitly in an @implementation section.

    Another benefit of this warning is that you can use it to get a warning when you pass a wrong key to a KVC, KVO, KVV, or Bindings method. Uli Kusterer has a macro for that.

  • Unused Functions

    Warn whenever a static function is declared but not defined or a non-inline static function is unused.

    Works best with a policy of declaring any function as static that you don't need to be visible elsewhere in your program.

  • Unused Labels

  • Unused Values

  • Unused Variables

    These follow the general rule of “code you don't have is code you don't have to debug”. If you're not using a label, expression statement, or variable, you don't need it, and you will find your code clearer without it.

    You may notice that I don't turn on Unused Parameters. Most times when I trip that warning, it's a callback function or method, so I can't get rid of the argument. Rather than litter my code with bright yellow #pragma unused(foo) directives, I prefer to just turn this one off. (See my rule above about less code being better.)

Once I have turned on all of these warnings and then eradicated them from my code, I turn on two more build settings:

  • Treat Warnings as Errors

    I call this “hardass mode”.

    Remember what I said above: Almost all of these warnings represent real or potential (i.e., future) bugs in my program. Rather than tolerate them, I turn this setting on so that any time I write such a bug, I break the build.

    I haven't been able to turn this on yet in Adium or Growl, although I have turned it on in Adium's Spotlight-importer project. I do, however, turn it on in all of my solo projects.

  • Run Static Analyzer

    Activating this setting will cause Xcode to run the Clang static analysis tool on qualifying source files.

    The Clang Static Analyzer is the find-your-bugs-for-you tool you've heard so much about. This setting makes Xcode run it whenever you build. Thus, every build, you get all those warnings errors and your analysis results.

    Whenever possible, I leave this on; if there's a source file that it takes a long time to analyze (e.g., GrowlPluginController.m), then I turn it off, but only then.

UPDATE 2009-11-22: Jonathan “Wolf” Rentzsch wrote a script to turn on all of these settings in all of the projects you have open.

UPDATE 2009-11-28: Updated the entry on “Typecheck Calls to printf/scanf” after seeing that Jeremy W. Sherman pointed out a much better benefit of it in a comment on a Stack Overflow answer.

UPDATE 2009-12-05: Corrected the discussion of the index problem. You can't use index, or any other function, as a C-array subscript, so the problem only affects higher-level arrays, such as NSArray.

UPDATE 2013-09-27: Added link to xcconfig; updated section on “Typecheck Calls to printf/scanf”.

Categories: Best practices; C; Cocoa; GCC; Programming; Toolchain; Xcode. | Comments: 26 (feed).

26 Responses to “Warnings I turn on, and why”

  1. Amagrammer Says:
    November 7th, 2009 at 21:42:30

    Where is the setting to turn on warnings for undeclared selectors? I”m not seeing it. I haven’t migrated to Snow Leopard yet — could that be the reason?

  2. Peter Hosey Says:
    November 7th, 2009 at 22:03:22

    Yup. In Xcode 3.2, the build setting’s human-readable name is simply “Undeclared Selectors”. Its under-the-hood name is GCC_WARN_UNDECLARED_SELECTOR, in case you’d like to turn it on now in advance of upgrading. (I doubt Xcode 3.1 will use it, but the setting won’t hurt anything, either; Xcode 3.1 will just ignore it.)

  3. julian Says:
    November 8th, 2009 at 06:12:09

    so which one of these aren’t contained in -Wmost?

  4. Amagrammer Says:
    November 8th, 2009 at 08:39:33

    Thanks for the quick response. Now maybe you can tell me: is there a way to make those settings the defaults for all projects created in the future? I already have about a dozen projects — just going into the old ones to make those changes will be tedious enough without having to do it on the new ones as well. Thanks

  5. Peter Hosey Says:
    November 8th, 2009 at 08:45:21

    julian: No idea.

    Amagrammer: Nope. Every “New” project is actually a copy of a project template, with a few variables (e.g., product name) replaced with values. You would have to change the templates, or create new ones.

  6. Blake C. Says:
    November 8th, 2009 at 14:32:24

    Peter and Amagrammer, you can do that with xcconfig files:

    http://developer.apple.com/mac/library/documentation/DeveloperTools/Conceptual/XcodeBuildSystem/400-Build_Configurations/build_configs.html

    Look for the “Build Configuration Files” section.

  7. dave Says:
    November 8th, 2009 at 15:06:23

    I’m a bit of an xcode noob, so forgive my noobishness…
    How do I change these settings? I found references to some of them in the xcode build settings reference, not sure where to go from here.

  8. Jesper Says:
    November 8th, 2009 at 16:38:58

    dave: Select either your entire project (the blueprint icon at the top) or a specific target in the Groups & Files list in your Xcode project, then choose Get Info from the right click menu. You change these settings on the Build tab; just watch out for the selected Configuration at the top.

    For more information, see http://developer.apple.com/mac/library/documentation/DeveloperTools/Conceptual/A_Tour_of_Xcode/010-Xcode_Features_Overview/FeaturesTake2.html#//apple_ref/doc/uid/TP30000890-CH220-SW17 . That whole document (“A Tour of Xcode”) is worth reading since you’re new to it.

  9. Peter Hosey Says:
    November 8th, 2009 at 18:03:08

    Blake: That still requires copying the configuration file(s) into every project directory and configuring every project to use them.

  10. Christopher Lloyd Says:
    November 8th, 2009 at 19:11:18

    “Multiple Definition Types for Selector” is only relevant for the GNU Objective-C runtime which has the mis-feature of trying to associate types with a selector. Largely pointless these days, should probably be deprecated all together.

  11. Blake C. Says:
    November 8th, 2009 at 21:29:16

    Peter: That’s better than editing all of those settings for each project, but you make a good point. I’d say file a bug :)

  12. Uli Kusterer Says:
    November 9th, 2009 at 08:39:14

    Re: Treat Warnings as Errors:

    If you can use that, do it. But I can’t use that in most of my shipping projects, because they need to build and run on several platforms (10.4, 10.5, 10.6 ATM), and Apple’s been madly changing their compilers with each version of Xcode, so I get warnings about settings that used to be necessary to build correctly, and are now deprecated, that would stop the build immediately :-(

  13. Ahruman Says:
    November 28th, 2009 at 10:18:04

    I habitually set “-Wall -Wextra -Wno-unused-parameter” in Other Warning Flags, and turn on Treat Warnings as Errors (or -Werror if you like to copy and paste). The only downside is I sometimes need to deal with other people’s code. :-)

  14. Uli Kusterer Says:
    December 1st, 2009 at 02:19:52

    The Four-Character Literals warning isn’t completely pointless. Since they’re essentially an Apple addition to C, you want that warning turned on in cross-platform code. So, I wouldn’t say “for no benefit”, I’d say “I’m writing Mac-only code, so don’t need it.”

  15. Jonathan Mitchell Says:
    December 9th, 2009 at 07:47:13

    Personally I like the unused parameter warning.

    Yes, it creates, a log of pragma clutter, especially for callbacks, but when reviewing code I find that those pesky pragmas do provide a bit of instant feedback that would otherwise escape me.

    The Apple docs on warnings are helpful too.
    http://developer.apple.com/tools/xcode/compilercodewarnings.html

    Good post.

  16. Sean M Says:
    March 3rd, 2010 at 14:29:27

    I once spent quite some time trying to enable as many warnings as possible by scouring the gcc man page. Below is my .xcconfig file that contains various warning flags. When Xcode has a specific option for a warning, I use it, if it doesn’t exist I use the gcc flag. Note that some warnings do/don’t work with some compilers. Our large Obj-C/Obj-C++ app builds warning-free with these settings. (RR is our company prefix, BW stands for Build Warning). This assumes gcc 4.2 and Xcode 3.2. Hope it’s helpful.

    // C only warnings.
GCC_WARN_ABOUT_MISSING_PROTOTYPES = YES
RR_BW_C_NOTCLANG = -Wdiv-by-zero
RR_BW_C = -Wstrict-prototypes -Wold-style-definition -Wnested-externs -Wmissing-prototypes -Wmissing-declarations $(RR_BW_C_NOTCLANG)
OTHER_CFLAGS = $(RR_BW_C)

// C++ only warnings.
GCC_WARN_HIDDEN_VIRTUAL_FUNCTIONS = YES
GCC_WARN_ABOUT_INVALID_OFFSETOF_MACRO = YES
GCC_WARN_NON_VIRTUAL_DESTRUCTOR = YES
RR_BW_CPP = -Wabi -Wctor-dtor-privacy -Wsign-promo -Woverloaded-virtual
OTHER_CPLUSPLUSFLAGS = $(RR_BW_CPP)

// Objective C only.
GCC_WARN_STRICT_SELECTOR_MATCH = YES
GCC_WARN_UNDECLARED_SELECTOR = YES
RR_BW_OBJC = 

// For all C-based languages.
GCC_WARN_FOUR_CHARACTER_CONSTANTS = YES
GCC_WARN_ABOUT_MISSING_NEWLINE = YES
GCC_WARN_ABOUT_DEPRECATED_FUNCTIONS = YES
GCC_WARN_SHADOW = YES
GCC_WARN_CHECK_SWITCH_STATEMENTS = YES
GCC_WARN_INITIALIZER_NOT_FULLY_BRACKETED = YES
GCC_WARN_MISSING_PARENTHESES = YES
GCC_WARN_SIGN_COMPARE = YES
GCC_WARN_TYPECHECK_CALLS_TO_PRINTF = YES
GCC_WARN_ABOUT_RETURN_TYPE = YES
GCC_WARN_64_TO_32_BIT_CONVERSION = YES

// Managed Object Model compiler.
MOMC_NO_INVERSE_RELATIONSHIP_WARNINGS = YES

// Note that it is important to start with -Wall and -Wextra because if they were at the end, they may reset some other options.
RR_BW_ALL_NOTCLANG = -Wlarger-than-30000 -Winvalid-pch
RR_BW_ALL_NOTGCC40 = -Wstrict-overflow=2 -Wvolatile-register-var -fdiagnostics-show-option
RR_BW_ALL = -Wall -Wextra -Wundef -Wendif-labels -Wpointer-arith -Wcast-align -Wwrite-strings -Wformat=2 -Wno-format-nonliteral -Wmissing-format-attribute -Wpacked -Wredundant-decls -Wdisabled-optimization -Wno-strict-aliasing -Wfloat-equal -Winit-self -Wextra-tokens -Wmissing-field-initializers $(RR_BW_ALL_NOTGCC40) $(RR_BW_ALL_NOTCLANG)

// Warnings to enable occasionally.  They give some good warnings, but not always.
//GCC_WARN_EFFECTIVE_CPLUSPLUS_VIOLATIONS = YES
//RR_BW_ALL_OCCASIONAL = -Wcast-qual -Wunsafe-loop-optimizations -funsafe-loop-optimizations -Wswitch-enum -Wstack-protector -Wformat-nonliteral

// Combine.
WARNING_CFLAGS = $(RR_BW_ALL) $(RR_BW_OBJC)

// TODO: would like to use:
//  GCC_WARN_PEDANTIC = YES • too annoying with Obj-C
//  -Wcast-qual • gives false positives with Obj-C 4625600. But turn on periodically, as it finds real problems!
//  -Wstrict-aliasing • is unnecessarily annoying 5509903 5509977
//  -Wstrict-aliasing=2 would be even nicer, though it is documented to give false positives
//  -Wformat-nonliteral (part of -Wformat=2) • causes problems with NSLocalizedStringFromTable 6835692
  17. Cédric Luthi Says:
    May 5th, 2010 at 22:38:30

    Actually, setting GCC_WARN_UNDECLARED_SELECTOR in Xcode 3.1 may hurt if you also define Treat Warnings as Errors and if someone else uses the same project with Xcode 3.2. Warnings will go unnoticed with Xcode 3.1 and will break the build with Xcode 3.2.

  18. Peter Hosey Says:
    May 5th, 2010 at 23:05:38

    Cédric Luthi: It’ll break the build in Xcode 3.1, too. “Treat Warnings as Errors” isn’t new in 3.2; it’s existed for a long, long time. It just worked differently before (it’d add a warning saying “warnings treated as errors”, then display the warnings as warnings but fail the build anyway; now it displays both warnings and errors as errors).

    And I don’t see what any specific warning has to do with that.

  19. Peter Hosey Says:
    July 10th, 2010 at 21:47:50

    Guh. This reading, Cédric, I understood what you were saying just fine: While you’re using Xcode 3.1, you may accumulate undeclared selectors that it won’t warn about, which will break your build when you finally do upgrade your Xcode. Sorry about that.

  20. Otto Says:
    February 16th, 2011 at 12:53:48

    Even if someone doesn’t program in C++, if they are hardcore about their craft I still recommend reading Effective C++. There’s a lot of stuff there that will make them a better Objective-C programmer.

    Question: Are there endian issues with the ‘abcd’ literal? Does it compile to a different sequence of bytes on PPC vs Intel?

  21. Peter Hosey Says:
    February 16th, 2011 at 15:31:10

    Otto: It does, but it generally will be the right sequence of bytes—most functions will do the right thing. Core Endian provides built-in “flippers” for most places that need them (e.g., Apple Events).

  22. Mark Aufflick Says:
    May 18th, 2011 at 22:54:52

    The implicit 32 to 64 bit conversion warning triggers on constants in recent gcc/clang versions. This requires putting an f after every inline eg. CGFloat literal. This isn’t such a big deal, except when you\’re importing external code you don’t want to fork.

    Instead you can add -fsingle-precision-constant as a compiler flag – that will effectively treat all literals as 32 bit. I don’t know about you, but I’ve never had time to type a 64 bit value by hand!

  23. Conor Says:
    December 21st, 2011 at 03:02:22

    I can’t get past the first example “checking switch statement” for lack of examples on how to write the enum. How do I declare an enum to be able to type check but also keep it compatible with 32-64 bit conversions as required by Apple? The following example below of what I use in my projects requires enums to be anonymous and hence all switches would require a default to avoid the warning.

    enum {
    NSNullCellType = 0,
    NSTextCellType = 1,
    NSImageCellType = 2
    };
    typedef NSUInteger NSCellType;

  24. Peter Hosey Says:
    December 21st, 2011 at 14:32:08

    Conor: There’s currently no good solution. However, a future version of Clang (actually the current version, but yet to be included with a release of Xcode) will include a solution originally from C++:

    typedef enum : unsigned char { Red, Green, Blue } Color;

    Presumably, you’ll be able to use NSUInteger in the middle there as the base type.

    It’s listed on the Clang language extensions page.

  25. Ortwin Gentz Says:
    January 28th, 2013 at 01:05:29

    I turned on the \”Hidden Local Variables\” warning and it complains about

    float value = 0.5;
    value = MAX(0, MIN(1, value);

    which I like to use for bounds checking. It works when I split the MIN and the MAX part into two lines but I find this annoying at times. Any idea how to fix this by using a different macro definition?

  26. Ortwin Gentz Says:
    January 28th, 2013 at 03:32:33

    Answering my own question this redefinition of the MAX macro helps:

    #undef MAX
    #define MAX(C,D) ({ __typeof__(C) __c = (C); __typeof__(D) __d = (D); __c < __d ? __d : __c; })

Leave a Reply

Do not delete the second sentence.