-
Notifications
You must be signed in to change notification settings - Fork 21
Warnings Explained
(Imported from the original blog post and updated.)
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 asOSType
. 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:
-
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 yourswitch
statement handles all possible values, cut out thedefault
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) > -1) && ((foo) < kFooNumberOfValidFooValues), @"Invalid Foo value: %d", (foo));
When you add
kFooQux
, insert it abovekFooNumberOfValidFooValues
, and the value ofkFooNumberOfValidFooValues
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.
-
This is deep in “warn you when the compiler thinks you forgot something” territory. I don't know about you, but I forget to finish writing a method before trying to use it rather frequently. Anything that shortens the hit-Build-to-smack-forehead time is a good thing in my book.
-
This includes a bunch of other warnings that have their own flags, but it also includes one that doesn't have a flag of its own: comparing a pointer relationally against zero. There is no good reason to have a comparison such as
myPtr < 0
, and this option tells the compiler to warn you when you do that.I do add
-Wno-unused-parameter
to this one, since-Wextra
includes-Wunused-parameter
and I find that warning to be one of the worst noise offenders. (See “Unused Parameter”, below.) -
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 yourindex
variable, all your references to it will actually refer to theindex
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
. -
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. -
Warn about implicit conversions of constant values that cause the constant value to change, either through a loss of precision, or entirely in its meaning.
-
Warn about implicit conversions between different kinds of
enum
values. For example, this can catch issues when using the wrongenum
flag as an argument to a function or method.With the new strongly-typed enumerations, the compiler can now tell which APIs expect which enumerated types (in most cases—some
enum
s don't yet use the new goodness). This is very helpful when you use the wrong one, which is very possible for certain pairs of similar-looking yet unrelated value names. -
Warn about implicit conversions between pointers and integers. For example, this can catch issues when one incorrectly intermixes using
NSNumber*
's and raw integers.Despite the name in Xcode, the description implies that this covers conversions in both directions.
-
Warn about implicit retains of '
self
' within blocks, which can create a retain-cycle.The obvious case is actually saying
self
, but it's also possible to implicitly refer toself
by accessing an instance variable within the block (such an expression is implicitlyself->_myIvar
). If you can't find a singleself
anywhere in the block, then it's probably that. -
Warn about implicit integer conversions that change the signedness of an integer value.
Every integer type has a range, and that range is partly dependent on the sign of the type. Signed types extend down into negative territory; unsigned types don't, but have twice the reach in the positive direction.
For values in the intersection, a cast from signed to unsigned or vice versa is harmless: the value is still in range, so it remains unchanged; no data loss occurs.
For values in either difference, however, a cast that cuts that section of the number line off is a problem waiting to happen: When you have such a value, and try to put it where it won't fit, the number that comes out of the transmogrifier may make no sense.
Indexes are the most common example: A large enough unsigned number, especially in a 32-bit or smaller type, becomes negative when cast to signed; inversely, a negative number may end up a very large unsigned number.
These tend to be very subtle and hard-to-diagnose bugs, often requiring the interpretation of strange application behavior and eventually the examination of variables. It's much nicer to have the problem pointed out at compile time.
See also: “Suspicious Implicit Conversions”.
-
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 typing9.9f
,1.1f
, or2.2f
instead of0.0f
.) -
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. -
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.
-
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.
-
Another cleanliness warning—this one, about the cleanliness of diffs.
-
Warn if an Objective-C class either subclasses a deprecated class or overrides a method that has been marked deprecated.
Deprecated stuff will go away someday and tends to be a low priority for bug fixes in the meantime. (For example, Apple have explicitly said they're not fixing things in QTKit anymore. If you're still using it, it will break eventually, and then you will be screwed.)
Calls to deprecated methods get warnings anyway. This is a different situation: generally, one where you will be called, according to an interface that has been wholly or partially deprecated.
That deprecation means that those calls may stop coming in someday. If there's a replacement for the class or method(s) you were overriding, it would be nice to have moved to it before then.
-
Warn when a comparison between signed and unsigned values could produce an incorrect result when the signed value is converted to unsigned.
-
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.
-
Warn about various implicit conversions that can lose information or are otherwise suspicious.
The most obvious example being an implicit conversion to a smaller or less-precise type, such as passing an
NSUInteger
(e.g., from an ANDed bit mask) where aBOOL
is expected.See also: “Implicit Signedness Conversions”.
-
Check calls to
printf
andscanf
, 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.Despite the name and description, this option does cover
stringWithFormat:
,NSLog
, and friends nowadays. It's really good for heading off crashes from a conversion specifier that doesn't match the corresponding value, or from fewer values being passed than are described in the string (especially problematic when%@
or%s
are involved).You probably also want to use
-Wformat=2
(in “Other Warning Flags”), which enables at least one extra check. Notably, this will warn if you pass a non-literal string for the format. Among the kinds of bugs that catches: the common rookie mistake of passing user or file or even network input there.If you still have code that pre-dates object literals, then 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 ideal fix, of course, is to replace this with a@[ … ]
literal.) -
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.
-
You might think that having code that simply doesn't get run wouldn't be that major of a problem. You would be wrong.
(I should point out that, as of Xcode 5.0.2 and [2b3ef4], this option doesn't actually catch my reduced version of that bug. I'm not sure if it's my error or Clang's.)
-
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. -
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.)
-
Warn on using a floating point value as a loop counter (CERT: FLP30-C, FLP30-CPP).
Just the fact that Xcode includes a CERT citation in its description should be reason enough to turn this one on. But if it's not, here's what they have to say about it.
Don't miss the examples, one of which “may iterate 9 or 10 times” and the other of which is, “on many implementations”, an infinite loop.
-
Warn on uses of '
rand
', 'random
', and related functions which produce predictable random number sequences. Usearc4random
instead.Or
arc4random_uniform
, when you need a fair random number from 0 to some maximum integer, orarc4random_buf
, when you need to fill a buffer with random bytes (and you don't need to draw on /dev/random, either directly or viaSecRandomCopyBytes
).
Once I have turned on all of these warnings and then eradicated them from my code, I turn on two more build settings:
-
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.
-
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
warningserrors and your analysis results.Whenever possible, I leave this on; if there's a source file that it takes a long time to analyze, then I turn it off, but only then.