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.

llvm-svn: 161501

10 files changed, 53 insertions, 22 deletions

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 29e3649a277..89c003c8f66 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/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/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 8811e6aa364..b95a90bd21c 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/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/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 566a8f01fae..7b3cd0fb772 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/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/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9df850d6bd5..d3aff2455ce 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/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/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index bfe72423e1e..6a503ee2d9b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/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/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7fac1c6ec4a..a8744899d70 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/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/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 0b7a6555e06..a3ead79a845 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/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/clang/test/SemaCXX/lambda-expressions.cpp b/clang/test/SemaCXX/lambda-expressions.cpp
index 198f8cf1fef..0fd634502bc 100644
--- a/clang/test/SemaCXX/lambda-expressions.cpp
+++ b/clang/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/clang/test/SemaTemplate/explicit-instantiation.cpp b/clang/test/SemaTemplate/explicit-instantiation.cpp
index 13d76befe28..e3e77d08284 100644
--- a/clang/test/SemaTemplate/explicit-instantiation.cpp
+++ b/clang/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/clang/test/SemaTemplate/instantiate-member-class.cpp b/clang/test/SemaTemplate/instantiate-member-class.cpp
index bb6427670c3..7b42a27d6b2 100644
--- a/clang/test/SemaTemplate/instantiate-member-class.cpp
+++ b/clang/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}}
   }
 }