From: David Blaikie Date: Wed, 8 Aug 2012 17:33:31 +0000 (+0000) Subject: Implement warning for integral null pointer constants other than the literal 0. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=50800fc551ac6b8a95cca662223e7f061bbd169a;p=clang Implement warning for integral null pointer constants other than the literal 0. This is effectively a warning for code that violates core issue 903 & thus will become standard error in the future, hopefully. It catches strange null pointers such as: '\0', 1 - 1, const int null = 0; etc... There's currently a flaw in this warning (& the warning for 'false' as a null pointer literal as well) where it doesn't trigger on comparisons (ptr == '\0' for example). Fix to come in a future patch. Also, due to this only being a warning, not an error, it triggers quite frequently on gtest code which tests expressions for null-pointer-ness in a SFINAE context (so it wouldn't be a problem if this was an error as in an actual implementation of core issue 903). To workaround this for now, the diagnostic does not fire in unevaluated contexts. Review by Sean Silva and Richard Smith. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161501 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 29e3649a27..89c003c8f6 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -541,8 +541,15 @@ public: /// \brief Expression is not a Null pointer constant. NPCK_NotNull = 0, - /// \brief Expression is a Null pointer constant built from a zero integer. - NPCK_ZeroInteger, + /// \brief Expression is a Null pointer constant built from a zero integer + /// expression that is not a simple, possibly parenthesized, zero literal. + /// C++ Core Issue 903 will classify these expressions as "not pointers" + /// once it is adopted. + /// http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#903 + NPCK_ZeroExpression, + + /// \brief Expression is a Null pointer constant built from a literal zero. + NPCK_ZeroLiteral, /// \brief Expression is a C++0X nullptr. NPCK_CXX0X_nullptr, diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 8811e6aa36..b95a90bd21 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -33,6 +33,7 @@ def StringConversion : DiagGroup<"string-conversion">; def SignConversion : DiagGroup<"sign-conversion">; def BoolConversion : DiagGroup<"bool-conversion">; def IntConversion : DiagGroup<"int-conversion">; +def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">; def NullConversion : DiagGroup<"null-conversion">; def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">; def CXXCompat: DiagGroup<"c++-compat">; @@ -314,7 +315,8 @@ def Conversion : DiagGroup<"conversion", StringConversion, SignConversion, BoolConversion, - NullConversion, + NullConversion, // NULL->non-pointer + NonLiteralNullConversion, // (1-1)->pointer (etc) IntConversion]>, DiagCategory<"Value Conversion Issue">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 566a8f01fa..7b3cd0fb77 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1896,6 +1896,9 @@ def warn_impcast_different_enum_types : Warning< def warn_impcast_bool_to_null_pointer : Warning< "initialization of pointer of type %0 to null from a constant boolean " "expression">, InGroup; +def warn_non_literal_null_pointer : Warning< + "expression which evaluates to zero treated as a null pointer constant of " + "type %0">, InGroup; def warn_impcast_null_pointer_to_integer : Warning< "implicit conversion of NULL constant to %0">, InGroup; diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 9df850d6bd..d3aff2455c 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2903,7 +2903,7 @@ Expr::isNullPointerConstant(ASTContext &Ctx, llvm_unreachable("Unexpected value dependent expression!"); case NPC_ValueDependentIsNull: if (isTypeDependent() || getType()->isIntegralType(Ctx)) - return NPCK_ZeroInteger; + return NPCK_ZeroExpression; else return NPCK_NotNull; @@ -2977,7 +2977,12 @@ Expr::isNullPointerConstant(ASTContext &Ctx, return NPCK_NotNull; } - return (EvaluateKnownConstInt(Ctx) == 0) ? NPCK_ZeroInteger : NPCK_NotNull; + if (EvaluateKnownConstInt(Ctx) != 0) + return NPCK_NotNull; + + if (isa(this)) + return NPCK_ZeroLiteral; + return NPCK_ZeroExpression; } /// \brief If this expression is an l-value for an Objective C diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index bfe72423e1..6a503ee2d9 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -4632,7 +4632,10 @@ bool Sema::DiagnoseConditionalForNull(Expr *LHSExpr, Expr *RHSExpr, if (NullKind == Expr::NPCK_NotNull) return false; - if (NullKind == Expr::NPCK_ZeroInteger) { + if (NullKind == Expr::NPCK_ZeroExpression) + return false; + + if (NullKind == Expr::NPCK_ZeroLiteral) { // In this case, check to make sure that we got here from a "NULL" // string in the source code. NullExpr = NullExpr->IgnoreParenImpCasts(); diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 7fac1c6ec4..a8744899d7 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -2562,13 +2562,17 @@ bool Sema::CheckPointerConversion(Expr *From, QualType ToType, Kind = CK_BitCast; - if (!IsCStyleOrFunctionalCast && - Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy) && - From->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNotNull)) - DiagRuntimeBehavior(From->getExprLoc(), From, - PDiag(diag::warn_impcast_bool_to_null_pointer) - << ToType << From->getSourceRange()); - + if (!IsCStyleOrFunctionalCast && !FromType->isAnyPointerType() && + From->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNotNull) == + Expr::NPCK_ZeroExpression) { + if (Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy)) + DiagRuntimeBehavior(From->getExprLoc(), From, + PDiag(diag::warn_impcast_bool_to_null_pointer) + << ToType << From->getSourceRange()); + else if (!isUnevaluatedContext()) + Diag(From->getExprLoc(), diag::warn_non_literal_null_pointer) + << ToType << From->getSourceRange(); + } if (const PointerType *ToPtrType = ToType->getAs()) { if (const PointerType *FromPtrType = FromType->getAs()) { QualType FromPointeeType = FromPtrType->getPointeeType(), diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp index 0b7a6555e0..a3ead79a84 100644 --- a/test/SemaCXX/constant-expression-cxx11.cpp +++ b/test/SemaCXX/constant-expression-cxx11.cpp @@ -710,10 +710,16 @@ static_assert(&ok2 == pok2, ""); static_assert((Base2*)(Derived*)(Base*)pb1 == pok2, ""); static_assert((Derived*)(Base*)pb1 == (Derived*)pok2, ""); -constexpr Base *nullB = 42 - 6 * 7; +constexpr Base *nullB = 42 - 6 * 7; // expected-warning {{expression which evaluates to zero treated as a null pointer constant of type 'Class::Base *const'}} static_assert((Bottom*)nullB == 0, ""); static_assert((Derived*)nullB == 0, ""); static_assert((void*)(Bottom*)nullB == (void*)(Derived*)nullB, ""); +Base * nullB2 = '\0'; // expected-warning {{expression which evaluates to zero treated as a null pointer constant of type 'Class::Base *'}} +Base * nullB3 = (0); +// We suppress the warning in unevaluated contexts to workaround some gtest +// behavior. Once this becomes an error this isn't a problem anymore. +static_assert(nullB == (1 - 1), ""); + namespace ConversionOperators { diff --git a/test/SemaCXX/lambda-expressions.cpp b/test/SemaCXX/lambda-expressions.cpp index 198f8cf1fe..0fd634502b 100644 --- a/test/SemaCXX/lambda-expressions.cpp +++ b/test/SemaCXX/lambda-expressions.cpp @@ -117,16 +117,16 @@ namespace NullPtr { const int m = 0; [=] { - int &k = f(m); // a null pointer constant + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}} } (); [=] () -> bool { - int &k = f(m); // a null pointer constant + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}} return &m == 0; } (); [m] { - int &k = f(m); // a null pointer constant + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}} } (); } } diff --git a/test/SemaTemplate/explicit-instantiation.cpp b/test/SemaTemplate/explicit-instantiation.cpp index 13d76befe2..e3e77d0828 100644 --- a/test/SemaTemplate/explicit-instantiation.cpp +++ b/test/SemaTemplate/explicit-instantiation.cpp @@ -14,7 +14,7 @@ struct X0 { T f0(T x) { return x + 1; // expected-error{{invalid operands}} } - T* f0(T*, T*) { return T(); } + T* f0(T*, T*) { return T(); } // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}} template T f0(T, U) { return T(); } @@ -32,7 +32,7 @@ struct NotDefaultConstructible { // expected-note{{candidate constructor (the im template NotDefaultConstructible X0::value; // expected-note{{instantiation}} template int X0::f0(int); -template int* X0::f0(int*, int*); +template int* X0::f0(int*, int*); // expected-note{{in instantiation of member function 'X0::f0' requested here}} template int X0::f0(int, float); template int X0::f0(int) const; // expected-error{{does not refer}} diff --git a/test/SemaTemplate/instantiate-member-class.cpp b/test/SemaTemplate/instantiate-member-class.cpp index bb6427670c..7b42a27d6b 100644 --- a/test/SemaTemplate/instantiate-member-class.cpp +++ b/test/SemaTemplate/instantiate-member-class.cpp @@ -124,19 +124,20 @@ namespace rdar10397846 { { struct B { - struct C { C() { int *ptr = I; } }; // expected-error{{cannot initialize a variable of type 'int *' with an rvalue of type 'int'}} + struct C { C() { int *ptr = I; } }; // expected-error{{cannot initialize a variable of type 'int *' with an rvalue of type 'int'}} \ + expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}} }; }; template void foo() { - class A::B::C X; // expected-note{{in instantiation of member function}} + class A::B::C X; // expected-note 2 {{in instantiation of member function}} int A::B::C::*member = 0; } void bar() { - foo<0>(); + foo<0>(); // expected-note{{in instantiation of function template}} foo<1>(); // expected-note{{in instantiation of function template}} } }