Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp @@ -63,33 +63,30 @@ } // end of anonymous namespace void Callback::run(const MatchFinder::MatchResult &Result) { - bool IsPedanticMatch = (Result.Nodes.getNodeAs("pedantic") != nullptr); + bool IsPedanticMatch = + (Result.Nodes.getNodeAs("pedantic") != nullptr); if (IsPedanticMatch && !C->Pedantic) return; - const Stmt *Conv = Result.Nodes.getNodeAs("conv"); - assert(Conv); - const Expr *Osboolean = Result.Nodes.getNodeAs("osboolean"); - const Expr *Nsnumber = Result.Nodes.getNodeAs("nsnumber"); - bool IsObjC = (bool)Nsnumber; - const Expr *Obj = IsObjC ? Nsnumber : Osboolean; - assert(Obj); - ASTContext &ACtx = ADC->getASTContext(); if (const Expr *CheckIfNull = Result.Nodes.getNodeAs("check_if_null")) { - // We consider NULL to be a pointer, even if it is defined as a plain 0. - // FIXME: Introduce a matcher to implement this logic? + // Unless the macro indicates that the intended type is clearly not + // a pointer type, we should avoid warning on comparing pointers + // to zero literals in non-pedantic mode. + // FIXME: Introduce an AST matcher to implement the macro-related logic? + bool MacroIndicatesWeShouldSkipTheCheck = false; SourceLocation Loc = CheckIfNull->getLocStart(); if (Loc.isMacroID()) { StringRef MacroName = Lexer::getImmediateMacroName( Loc, ACtx.getSourceManager(), ACtx.getLangOpts()); - if (MacroName != "YES" && MacroName != "NO") + if (MacroName == "NULL" || MacroName == "nil") return; - } else { - // Otherwise, comparison of pointers to 0 might still be intentional. - // See if this is the case. + if (MacroName == "YES" || MacroName == "NO") + MacroIndicatesWeShouldSkipTheCheck = true; + } + if (!MacroIndicatesWeShouldSkipTheCheck) { llvm::APSInt Result; if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt( Result, ACtx, Expr::SE_AllowSideEffects)) { @@ -102,33 +99,92 @@ } } + const Stmt *Conv = Result.Nodes.getNodeAs("conv"); + assert(Conv); + + const Expr *ConvertedCObject = Result.Nodes.getNodeAs("c_object"); + const Expr *ConvertedCppObject = Result.Nodes.getNodeAs("cpp_object"); + const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs("objc_object"); + bool IsCpp = (ConvertedCppObject != nullptr); + bool IsObjC = (ConvertedObjCObject != nullptr); + const Expr *Obj = IsObjC ? ConvertedObjCObject + : IsCpp ? ConvertedCppObject + : ConvertedCObject; + assert(Obj); + + bool IsComparison = + (Result.Nodes.getNodeAs("comparison") != nullptr); + + bool IsOSNumber = + (Result.Nodes.getNodeAs("osnumber") != nullptr); + + bool IsInteger = + (Result.Nodes.getNodeAs("int_type") != nullptr); + bool IsObjCBool = + (Result.Nodes.getNodeAs("objc_bool_type") != nullptr); + bool IsCppBool = + (Result.Nodes.getNodeAs("cpp_bool_type") != nullptr); + llvm::SmallString<64> Msg; llvm::raw_svector_ostream OS(Msg); - OS << "Converting '" - << Obj->getType().getCanonicalType().getUnqualifiedType().getAsString() - << "' to a plain "; - - if (Result.Nodes.getNodeAs("int_type") != nullptr) - OS << "integer value"; - else if (Result.Nodes.getNodeAs("objc_bool_type") != nullptr) - OS << "BOOL value"; - else if (Result.Nodes.getNodeAs("cpp_bool_type") != nullptr) - OS << "bool value"; + + // Remove ObjC ARC qualifiers. + QualType ObjT = Obj->getType().getUnqualifiedType(); + + // Remove consts from pointers. + if (IsCpp) { + assert(ObjT.getCanonicalType()->isPointerType()); + ObjT = ACtx.getPointerType( + ObjT->getPointeeType().getCanonicalType().getUnqualifiedType()); + } + + if (IsComparison) + OS << "Comparing "; else - OS << "boolean value for branching"; + OS << "Converting "; - if (IsPedanticMatch) { - if (IsObjC) { - OS << "; please compare the pointer to nil instead " - "to suppress this warning"; - } else { - OS << "; please compare the pointer to NULL or nullptr instead " - "to suppress this warning"; - } - } else { - OS << "; pointer value is being used instead"; + OS << "a pointer value of type '" << ObjT.getAsString() << "' to a "; + + std::string EuphemismForPlain = "primitive"; + std::string SuggestedApi = IsObjC ? (IsInteger ? "" : "-boolValue") + : IsCpp ? (IsOSNumber ? "" : "getValue()") + : "CFNumberGetValue()"; + if (SuggestedApi.empty()) { + // A generic message if we're not sure what API should be called. + // FIXME: Pattern-match the integer type to make a better guess? + SuggestedApi = + "a method on '" + ObjT.getAsString() + "' to get the scalar value"; + // "scalar" is not quite correct or common, but some documentation uses it + // when describing object methods we suggest. For consistency, we use + // "scalar" in the whole sentence when we need to use this word in at least + // one place, otherwise we use "primitive". + EuphemismForPlain = "scalar"; } + if (IsInteger) + OS << EuphemismForPlain << " integer value"; + else if (IsObjCBool) + OS << EuphemismForPlain << " BOOL value"; + else if (IsCppBool) + OS << EuphemismForPlain << " bool value"; + else // Branch condition? + OS << EuphemismForPlain << " boolean value"; + + + if (IsPedanticMatch) + OS << "; instead, either compare the pointer to " + << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or "; + else + OS << "; did you mean to "; + + if (IsComparison) + OS << "compare the result of calling " << SuggestedApi; + else + OS << "call " << SuggestedApi; + + if (!IsPedanticMatch) + OS << "?"; + BR.EmitBasicReport( ADC->getDecl(), C, "Suspicious number object conversion", "Logic error", OS.str(), @@ -139,107 +195,132 @@ void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM, BugReporter &BR) const { - MatchFinder F; - Callback CB(this, BR, AM.getAnalysisDeclContext(D)); + // Currently this matches CoreFoundation opaque pointer typedefs. + auto CSuspiciousNumberObjectExprM = + expr(ignoringParenImpCasts( + expr(hasType( + typedefType(hasDeclaration(anyOf( + typedefDecl(hasName("CFNumberRef")), + typedefDecl(hasName("CFBooleanRef"))))))) + .bind("c_object"))); - auto OSBooleanExprM = + // Currently this matches XNU kernel number-object pointers. + auto CppSuspiciousNumberObjectExprM = expr(ignoringParenImpCasts( expr(hasType(hasCanonicalType( pointerType(pointee(hasCanonicalType( recordType(hasDeclaration( - cxxRecordDecl(hasName( - "OSBoolean")))))))))).bind("osboolean"))); - - auto NSNumberExprM = - expr(ignoringParenImpCasts(expr(hasType(hasCanonicalType( - objcObjectPointerType(pointee( - qualType(hasCanonicalType( - qualType(hasDeclaration( - objcInterfaceDecl(hasName( - "NSNumber"))))))))))).bind("nsnumber"))); + anyOf( + cxxRecordDecl(hasName("OSBoolean")), + cxxRecordDecl(hasName("OSNumber")) + .bind("osnumber")))))))))) + .bind("cpp_object"))); - auto SuspiciousExprM = - anyOf(OSBooleanExprM, NSNumberExprM); - - auto AnotherNSNumberExprM = - expr(equalsBoundNode("nsnumber")); + // Currently this matches NeXTSTEP number objects. + auto ObjCSuspiciousNumberObjectExprM = + expr(ignoringParenImpCasts( + expr(hasType(hasCanonicalType( + objcObjectPointerType(pointee( + qualType(hasCanonicalType( + qualType(hasDeclaration( + objcInterfaceDecl(hasName("NSNumber"))))))))))) + .bind("objc_object"))); + + auto SuspiciousNumberObjectExprM = anyOf( + CSuspiciousNumberObjectExprM, + CppSuspiciousNumberObjectExprM, + ObjCSuspiciousNumberObjectExprM); + + // Useful for predicates like "Unless we've seen the same object elsewhere". + auto AnotherSuspiciousNumberObjectExprM = + expr(anyOf( + equalsBoundNode("c_object"), + equalsBoundNode("objc_object"), + equalsBoundNode("cpp_object"))); // The .bind here is in order to compose the error message more accurately. - auto ObjCBooleanTypeM = + auto ObjCSuspiciousScalarBooleanTypeM = qualType(typedefType(hasDeclaration( typedefDecl(hasName("BOOL"))))).bind("objc_bool_type"); // The .bind here is in order to compose the error message more accurately. - auto AnyBooleanTypeM = + auto SuspiciousScalarBooleanTypeM = qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"), - ObjCBooleanTypeM)); - + ObjCSuspiciousScalarBooleanTypeM)); // The .bind here is in order to compose the error message more accurately. - auto AnyNumberTypeM = + // Also avoid intptr_t and uintptr_t because they were specifically created + // for storing pointers. + auto SuspiciousScalarNumberTypeM = qualType(hasCanonicalType(isInteger()), unless(typedefType(hasDeclaration( typedefDecl(matchesName("^::u?intptr_t$")))))) .bind("int_type"); - auto AnyBooleanOrNumberTypeM = - qualType(anyOf(AnyBooleanTypeM, AnyNumberTypeM)); + auto SuspiciousScalarTypeM = + qualType(anyOf(SuspiciousScalarBooleanTypeM, + SuspiciousScalarNumberTypeM)); - auto AnyBooleanOrNumberExprM = - expr(ignoringParenImpCasts(expr(hasType(AnyBooleanOrNumberTypeM)))); + auto SuspiciousScalarExprM = + expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM)))); auto ConversionThroughAssignmentM = binaryOperator(hasOperatorName("="), - hasLHS(AnyBooleanOrNumberExprM), - hasRHS(SuspiciousExprM)); + hasLHS(SuspiciousScalarExprM), + hasRHS(SuspiciousNumberObjectExprM)); auto ConversionThroughBranchingM = - ifStmt(hasCondition(SuspiciousExprM)) + ifStmt(hasCondition(SuspiciousNumberObjectExprM)) .bind("pedantic"); auto ConversionThroughCallM = - callExpr(hasAnyArgument(allOf(hasType(AnyBooleanOrNumberTypeM), - ignoringParenImpCasts(SuspiciousExprM)))); + callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM), + ignoringParenImpCasts( + SuspiciousNumberObjectExprM)))); // We bind "check_if_null" to modify the warning message // in case it was intended to compare a pointer to 0 with a relatively-ok // construct "x == 0" or "x != 0". auto ConversionThroughEquivalenceM = binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), - hasEitherOperand(SuspiciousExprM), - hasEitherOperand(AnyBooleanOrNumberExprM - .bind("check_if_null"))); + hasEitherOperand(SuspiciousNumberObjectExprM), + hasEitherOperand(SuspiciousScalarExprM + .bind("check_if_null"))) + .bind("comparison"); auto ConversionThroughComparisonM = binaryOperator(anyOf(hasOperatorName(">="), hasOperatorName(">"), hasOperatorName("<="), hasOperatorName("<")), - hasEitherOperand(SuspiciousExprM), - hasEitherOperand(AnyBooleanOrNumberExprM)); + hasEitherOperand(SuspiciousNumberObjectExprM), + hasEitherOperand(SuspiciousScalarExprM)) + .bind("comparison"); auto ConversionThroughConditionalOperatorM = conditionalOperator( - hasCondition(SuspiciousExprM), - unless(hasTrueExpression(hasDescendant(AnotherNSNumberExprM))), - unless(hasFalseExpression(hasDescendant(AnotherNSNumberExprM)))) + hasCondition(SuspiciousNumberObjectExprM), + unless(hasTrueExpression( + hasDescendant(AnotherSuspiciousNumberObjectExprM))), + unless(hasFalseExpression( + hasDescendant(AnotherSuspiciousNumberObjectExprM)))) .bind("pedantic"); auto ConversionThroughExclamationMarkM = - unaryOperator(hasOperatorName("!"), has(expr(SuspiciousExprM))) + unaryOperator(hasOperatorName("!"), + has(expr(SuspiciousNumberObjectExprM))) .bind("pedantic"); auto ConversionThroughExplicitBooleanCastM = - explicitCastExpr(hasType(AnyBooleanTypeM), - has(expr(SuspiciousExprM))) - .bind("pedantic"); + explicitCastExpr(hasType(SuspiciousScalarBooleanTypeM), + has(expr(SuspiciousNumberObjectExprM))); auto ConversionThroughExplicitNumberCastM = - explicitCastExpr(hasType(AnyNumberTypeM), - has(expr(SuspiciousExprM))); + explicitCastExpr(hasType(SuspiciousScalarNumberTypeM), + has(expr(SuspiciousNumberObjectExprM))); auto ConversionThroughInitializerM = declStmt(hasSingleDecl( - varDecl(hasType(AnyBooleanOrNumberTypeM), - hasInitializer(SuspiciousExprM)))); + varDecl(hasType(SuspiciousScalarTypeM), + hasInitializer(SuspiciousNumberObjectExprM)))); auto FinalM = stmt(anyOf(ConversionThroughAssignmentM, ConversionThroughBranchingM, @@ -252,16 +333,16 @@ ConversionThroughExplicitNumberCastM, ConversionThroughInitializerM)).bind("conv"); + MatchFinder F; + Callback CB(this, BR, AM.getAnalysisDeclContext(D)); + F.addMatcher(stmt(forEachDescendant(FinalM)), &CB); F.match(*D->getBody(), AM.getASTContext()); } void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { - const LangOptions &LO = Mgr.getLangOpts(); - if (LO.CPlusPlus || LO.ObjC2) { - NumberObjectConversionChecker *Chk = - Mgr.registerChecker(); - Chk->Pedantic = - Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk); - } + NumberObjectConversionChecker *Chk = + Mgr.registerChecker(); + Chk->Pedantic = + Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk); } Index: cfe/trunk/test/Analysis/number-object-conversion.c =================================================================== --- cfe/trunk/test/Analysis/number-object-conversion.c +++ cfe/trunk/test/Analysis/number-object-conversion.c @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -triple i386-apple-darwin10 -w -analyze -analyzer-checker=osx.NumberObjectConversion %s -verify +// RUN: %clang_cc1 -triple i386-apple-darwin10 -w -analyze -analyzer-checker=osx.NumberObjectConversion -analyzer-config osx.NumberObjectConversion:Pedantic=true -DPEDANTIC %s -verify + +#define NULL ((void *)0) + +typedef const struct __CFNumber *CFNumberRef; + +void takes_int(int); + +void bad(CFNumberRef p) { +#ifdef PEDANTIC + if (p) {} // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive boolean value; instead, either compare the pointer to NULL or call CFNumberGetValue()}} + if (!p) {} // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive boolean value; instead, either compare the pointer to NULL or call CFNumberGetValue()}} + p ? 1 : 2; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive boolean value; instead, either compare the pointer to NULL or call CFNumberGetValue()}} + if (p == 0) {} // expected-warning{{Comparing a pointer value of type 'CFNumberRef' to a primitive integer value; instead, either compare the pointer to NULL or compare the result of calling CFNumberGetValue()}} +#else + if (p) {} // no-warning + if (!p) {} // no-warning + p ? 1 : 2; // no-warning + if (p == 0) {} // no-warning +#endif + if (p > 0) {} // expected-warning{{Comparing a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to compare the result of calling CFNumberGetValue()?}} + int x = p; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} + x = p; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} + takes_int(p); // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} + takes_int(x); // no-warning +} + +// Conversion of a pointer to an intptr_t is fine. +typedef long intptr_t; +typedef unsigned long uintptr_t; +typedef long fintptr_t; // Fake, for testing the regex. +void test_intptr_t(CFNumberRef p) { + (long)p; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} + (intptr_t)p; // no-warning + (unsigned long)p; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} + (uintptr_t)p; // no-warning + (fintptr_t)p; // expected-warning{{Converting a pointer value of type 'CFNumberRef' to a primitive integer value; did you mean to call CFNumberGetValue()?}} +} + Index: cfe/trunk/test/Analysis/number-object-conversion.cpp =================================================================== --- cfe/trunk/test/Analysis/number-object-conversion.cpp +++ cfe/trunk/test/Analysis/number-object-conversion.cpp @@ -10,28 +10,57 @@ virtual bool isFalse() const; }; +class OSNumber { +public: + virtual bool isEqualTo(const OSNumber *); + virtual unsigned char unsigned8BitValue() const; + virtual unsigned short unsigned16BitValue() const; + virtual unsigned int unsigned32BitValue() const; + virtual unsigned long long unsigned64BitValue() const; +}; + extern const OSBoolean *const &kOSBooleanFalse; extern const OSBoolean *const &kOSBooleanTrue; void takes_bool(bool); -void bad(const OSBoolean *p) { +void bad_boolean(const OSBoolean *p) { #ifdef PEDANTIC - if (p) {} // expected-warning{{Converting 'const class OSBoolean *' to a plain boolean value for branching; please compare the pointer to NULL or nullptr instead to suppress this warning}} - if (!p) {} // expected-warning{{Converting 'const class OSBoolean *' to a plain boolean value for branching; please compare the pointer to NULL or nullptr instead to suppress this warning}} - p ? 1 : 2; // expected-warning{{Converting 'const class OSBoolean *' to a plain boolean value for branching; please compare the pointer to NULL or nullptr instead to suppress this warning}} - (bool)p; // expected-warning{{Converting 'const class OSBoolean *' to a plain bool value; please compare the pointer to NULL or nullptr instead to suppress this warning}} + if (p) {} // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive boolean value; instead, either compare the pointer to nullptr or call getValue()}} + if (!p) {} // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive boolean value; instead, either compare the pointer to nullptr or call getValue()}} + p ? 1 : 2; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive boolean value; instead, either compare the pointer to nullptr or call getValue()}} +#else + if (p) {} // no-warning + if (!p) {} // no-warning + p ? 1 : 2; // no-warning #endif - bool x = p; // expected-warning{{Converting 'const class OSBoolean *' to a plain bool value; pointer value is being used instead}} - x = p; // expected-warning{{Converting 'const class OSBoolean *' to a plain bool value; pointer value is being used instead}} - takes_bool(p); // expected-warning{{Converting 'const class OSBoolean *' to a plain bool value; pointer value is being used instead}} + (bool)p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive bool value; did you mean to call getValue()?}} + bool x = p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive bool value; did you mean to call getValue()?}} + x = p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive bool value; did you mean to call getValue()?}} + takes_bool(p); // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive bool value; did you mean to call getValue()?}} takes_bool(x); // no-warning } +void bad_number(const OSNumber *p) { +#ifdef PEDANTIC + if (p) {} // expected-warning{{Converting a pointer value of type 'class OSNumber *' to a scalar boolean value; instead, either compare the pointer to nullptr or call a method on 'class OSNumber *' to get the scalar value}} + if (!p) {} // expected-warning{{Converting a pointer value of type 'class OSNumber *' to a scalar boolean value; instead, either compare the pointer to nullptr or call a method on 'class OSNumber *' to get the scalar value}} + p ? 1 : 2; // expected-warning{{Converting a pointer value of type 'class OSNumber *' to a scalar boolean value; instead, either compare the pointer to nullptr or call a method on 'class OSNumber *' to get the scalar value}} + if (p == 0) {} // expected-warning{{Comparing a pointer value of type 'class OSNumber *' to a scalar integer value; instead, either compare the pointer to nullptr or compare the result of calling a method on 'class OSNumber *' to get the scalar value}} +#else + if (p) {} // no-warning + if (!p) {} // no-warning + p ? 1 : 2; // no-warning + if (p == 0) {} // no-warning +#endif + (int)p; // expected-warning{{Converting a pointer value of type 'class OSNumber *' to a scalar integer value; did you mean to call a method on 'class OSNumber *' to get the scalar value?}} + takes_bool(p); // expected-warning{{Converting a pointer value of type 'class OSNumber *' to a scalar bool value; did you mean to call a method on 'class OSNumber *' to get the scalar value?}} +} + typedef bool sugared_bool; typedef const OSBoolean *sugared_OSBoolean; void bad_sugared(sugared_OSBoolean p) { - sugared_bool x = p; // expected-warning{{Converting 'const class OSBoolean *' to a plain bool value; pointer value is being used instead}} + sugared_bool x = p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive bool value; did you mean to call getValue()?}} } void good(const OSBoolean *p) { @@ -50,11 +79,11 @@ typedef unsigned long uintptr_t; typedef long fintptr_t; // Fake, for testing the regex. void test_intptr_t(const OSBoolean *p) { - (long)p; // expected-warning{{Converting 'const class OSBoolean *' to a plain integer value; pointer value is being used instead}} + (long)p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive integer value; did you mean to call getValue()?}} (intptr_t)p; // no-warning - (unsigned long)p; // expected-warning{{Converting 'const class OSBoolean *' to a plain integer value; pointer value is being used instead}} + (unsigned long)p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive integer value; did you mean to call getValue()?}} (uintptr_t)p; // no-warning - (fintptr_t)p; // expected-warning{{Converting 'const class OSBoolean *' to a plain integer value; pointer value is being used instead}} + (fintptr_t)p; // expected-warning{{Converting a pointer value of type 'class OSBoolean *' to a primitive integer value; did you mean to call getValue()?}} } // Test a different definition of NULL. Index: cfe/trunk/test/Analysis/number-object-conversion.m =================================================================== --- cfe/trunk/test/Analysis/number-object-conversion.m +++ cfe/trunk/test/Analysis/number-object-conversion.m @@ -10,30 +10,35 @@ void bad(NSNumber *p) { #ifdef PEDANTIC - if (p) {} // expected-warning{{Converting 'NSNumber *' to a plain boolean value for branching; please compare the pointer to nil instead to suppress this warning}} - if (!p) {} // expected-warning{{Converting 'NSNumber *' to a plain boolean value for branching; please compare the pointer to nil instead to suppress this warning}} - (!p) ? 1 : 2; // expected-warning{{Converting 'NSNumber *' to a plain boolean value for branching; please compare the pointer to nil instead to suppress this warning}} - (BOOL)p; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; please compare the pointer to nil instead to suppress this warning}} - if (p == 0) {} // expected-warning{{Converting 'NSNumber *' to a plain integer value; please compare the pointer to nil instead to suppress this warning}} - if (p > 0) {} // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} + if (p) {} // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive boolean value; instead, either compare the pointer to nil or call -boolValue}} + if (!p) {} // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive boolean value; instead, either compare the pointer to nil or call -boolValue}} + (!p) ? 1 : 2; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive boolean value; instead, either compare the pointer to nil or call -boolValue}} + if (p == 0) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a scalar integer value; instead, either compare the pointer to nil or compare the result of calling a method on 'NSNumber *' to get the scalar value}} +#else + if (p) {} // no-warning + if (!p) {} // no-warning + (!p) ? 1 : 2; // no-warning + if (p == 0) {} // no-warning #endif - if (p == YES) {} // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - if (p == NO) {} // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - BOOL x = p; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - x = p; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - x = (p == YES); // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - if (p == 1) {} // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} - int y = p; // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} - y = p; // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} - takes_boolean(p); // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - takes_integer(p); // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} + (BOOL)p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to call -boolValue?}} + if (p > 0) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to compare the result of calling a method on 'NSNumber *' to get the scalar value?}} + if (p == YES) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} + if (p == NO) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} + BOOL x = p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to call -boolValue?}} + x = p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to call -boolValue?}} + x = (p == YES); // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} + if (p == 1) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to compare the result of calling a method on 'NSNumber *' to get the scalar value?}} + int y = p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} + y = p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} + takes_boolean(p); // expected-warning{{Converting a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to call -boolValue?}} + takes_integer(p); // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} takes_boolean(x); // no-warning takes_integer(y); // no-warning } typedef NSNumber *SugaredNumber; void bad_sugared(SugaredNumber p) { - p == YES; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} + p == YES; // expected-warning{{Comparing a pointer value of type 'SugaredNumber' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} } @interface I : NSObject { @@ -50,9 +55,9 @@ @end void bad_ivar(I *i) { - i->ivar == YES; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - i->prop == YES; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} - [i foo] == YES; // expected-warning{{Converting 'NSNumber *' to a plain BOOL value; pointer value is being used instead}} + i->ivar == YES; // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} + i->prop == YES; // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} + [i foo] == YES; // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a primitive BOOL value; did you mean to compare the result of calling -boolValue?}} } void good(NSNumber *p) { @@ -71,11 +76,32 @@ typedef unsigned long uintptr_t; typedef long fintptr_t; // Fake, for testing the regex. void test_intptr_t(NSNumber *p) { - (long)p; // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} + (long)p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} (intptr_t)p; // no-warning - (unsigned long)p; // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} + (unsigned long)p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} (uintptr_t)p; // no-warning - (fintptr_t)p; // expected-warning{{Converting 'NSNumber *' to a plain integer value; pointer value is being used instead}} + (fintptr_t)p; // expected-warning{{Converting a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to call a method on 'NSNumber *' to get the scalar value?}} +} + +// Test macro suppressions. +#define FOO 0 +#define BAR 1 +void test_macro(NSNumber *p) { + if (p != BAR) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a scalar integer value; did you mean to compare the result of calling a method on 'NSNumber *' to get the scalar value?}} +#ifdef PEDANTIC + if (p != FOO) {} // expected-warning{{Comparing a pointer value of type 'NSNumber *' to a scalar integer value; instead, either compare the pointer to nil or compare the result of calling a method on 'NSNumber *' to get the scalar value}} +#else + if (p != FOO) {} // no-warning +#endif +} + +#define NULL_INSIDE_MACRO NULL +void test_NULL_inside_macro(NSNumber *p) { +#ifdef PEDANTIC + if (p == NULL_INSIDE_MACRO) {} // no-warning +#else + if (p == NULL_INSIDE_MACRO) {} // no-warning +#endif } // Test a different definition of NULL.