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 29e3649a277947ab0f66421b87764d69b286bd01..89c003c8f664b1142219f03e440c5f68c68e8f61 100644 (file)
--- 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 8811e6aa3643a35246d834f21815519a3928dd73..b95a90bd21c3df683b152f23b869c92f5876813b 100644 (file)
--- 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 566a8f01faef95ec1b91bbe17d8002d787bf6d98..7b3cd0fb7720b27f9eac9e8d320c546677d14623 100644 (file)
--- 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<BoolConversion>;
+def warn_non_literal_null_pointer : Warning<
+    "expression which evaluates to zero treated as a null pointer constant of "
+    "type %0">, InGroup<NonLiteralNullConversion>;
 def warn_impcast_null_pointer_to_integer : Warning<
     "implicit conversion of NULL constant to %0">,
     InGroup<NullConversion>;

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 9df850d6bd5f80e595543897f80eba2c17a77c2c..d3aff2455cecbd1d4570f51d6c8722648b8abeac 100644 (file)
--- 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<IntegerLiteral>(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 bfe72423e1e98dd4cf8e48d1ef099d518f92d8b3..6a503ee2d9b50a0d48d6075760e30243e8b0dad6 100644 (file)
--- 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 7fac1c6ec4aeb31ddc19e3c845e1fd92c6daec9f..a8744899d70cfd94ee4c3e577c8a7d9b3c0741be 100644 (file)
--- 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<PointerType>()) {
     if (const PointerType *FromPtrType = FromType->getAs<PointerType>()) {
       QualType FromPointeeType = FromPtrType->getPointeeType(),

diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp
index 0b7a6555e06ec6b3d4b4beb50354378999ae60c9..a3ead79a8456867a53fed6d6104f7e72a79011b3 100644 (file)
--- 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 198f8cf1fef7f25d37c60f89f11040f61a7013b6..0fd634502bc9cd8137ec9a2167f462382a3b8085 100644 (file)
--- 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 13d76befe289aac76da9be1e436b28b6619d0a76..e3e77d0828448ab749388704be951555845dffce 100644 (file)
--- 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<typename U>
   T f0(T, U) { return T(); }
@@ -32,7 +32,7 @@ struct NotDefaultConstructible { // expected-note{{candidate constructor (the im
 template NotDefaultConstructible X0<NotDefaultConstructible>::value; // expected-note{{instantiation}}
 
 template int X0<int>::f0(int);
-template int* X0<int>::f0(int*, int*);
+template int* X0<int>::f0(int*, int*); // expected-note{{in instantiation of member function 'X0<int>::f0' requested here}}
 template int X0<int>::f0(int, float);
 
 template int X0<int>::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 bb6427670c3ee733d649e52513a6bd5a188532ba..7b42a27d6b2cac68e5f8e84b788256c0b0fdc810 100644 (file)
--- 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<int N> void foo()
   {
-    class A<N>::B::C X; // expected-note{{in instantiation of member function}}
+    class A<N>::B::C X; // expected-note 2 {{in instantiation of member function}}
     int A<N+1>::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}}
   }
 }