Fix checking for a null pointer constant when the expression itself is

value-dependent. Audit (and fixed) all calls to
Expr::isNullPointerConstant() to provide the correct behavior with
value-dependent expressions. Fixes PR5041 and a crash in libstdc++
<locale>.

In the same vein, properly compute value- and type-dependence for
ChooseExpr. Fixes PR4996.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@82748 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 39a23a1570bcc1a33bad278a4e5b653d6b9dd2ea..5c594cab35a9ae97029762b50f8148f102573b35 100644 (file)
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -257,10 +257,26 @@ public:
   /// also succeeds on stack based, immutable address lvalues.
   bool EvaluateAsAnyLValue(EvalResult &Result, ASTContext &Ctx) const;
 
+  /// \brief Enumeration used to describe how \c isNullPointerConstant()
+  /// should cope with value-dependent expressions.
+  enum NullPointerConstantValueDependence {
+    /// \brief Specifies that the expression should never be value-dependent.
+    NPC_NeverValueDependent = 0,
+    
+    /// \brief Specifies that a value-dependent expression of integral or
+    /// dependent type should be considered a null pointer constant.
+    NPC_ValueDependentIsNull,
+    
+    /// \brief Specifies that a value-dependent expression should be considered
+    /// to never be a null pointer constant.
+    NPC_ValueDependentIsNotNull
+  };
+  
   /// isNullPointerConstant - C99 6.3.2.3p3 -  Return true if this is either an
   /// integer constant expression with the value zero, or if this is one that is
   /// cast to void*.
-  bool isNullPointerConstant(ASTContext &Ctx) const;
+  bool isNullPointerConstant(ASTContext &Ctx,
+                             NullPointerConstantValueDependence NPC) const;
 
   /// isOBJCGCCandidate - Return true if this expression may be used in a read/
   /// write barrier.
@@ -2004,8 +2020,8 @@ class ChooseExpr : public Expr {
   SourceLocation BuiltinLoc, RParenLoc;
 public:
   ChooseExpr(SourceLocation BLoc, Expr *cond, Expr *lhs, Expr *rhs, QualType t,
-             SourceLocation RP)
-    : Expr(ChooseExprClass, t),
+             SourceLocation RP, bool TypeDependent, bool ValueDependent)
+    : Expr(ChooseExprClass, t, TypeDependent, ValueDependent),
       BuiltinLoc(BLoc), RParenLoc(RP) {
       SubExprs[COND] = cond;
       SubExprs[LHS] = lhs;

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index dc9cee13a9c7e3d22b0c6fb3ddfd3031d0d8f30b..dcfd1474f16d34dd735c37da6ea826a9cd2fa3d3 100644 (file)
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -1625,9 +1625,21 @@ bool Expr::isIntegerConstantExpr(llvm::APSInt &Result, ASTContext &Ctx,
 /// isNullPointerConstant - C99 6.3.2.3p3 -  Return true if this is either an
 /// integer constant expression with the value zero, or if this is one that is
 /// cast to void*.
-bool Expr::isNullPointerConstant(ASTContext &Ctx) const {
-  // Ignore value dependent expressions.
-  assert(!isValueDependent() && "Unexpect value dependent expression!");
+bool Expr::isNullPointerConstant(ASTContext &Ctx,
+                                 NullPointerConstantValueDependence NPC) const {
+  if (isValueDependent()) {
+    switch (NPC) {
+    case NPC_NeverValueDependent:
+      assert(false && "Unexpected value dependent expression!");
+      // If the unthinkable happens, fall through to the safest alternative.
+        
+    case NPC_ValueDependentIsNull:
+      return isTypeDependent() || getType()->isIntegralType();
+        
+    case NPC_ValueDependentIsNotNull:
+      return false;
+    }
+  }
 
   // Strip off a cast to void*, if it exists. Except in C++.
   if (const ExplicitCastExpr *CE = dyn_cast<ExplicitCastExpr>(this)) {
@@ -1638,20 +1650,20 @@ bool Expr::isNullPointerConstant(ASTContext &Ctx) const {
         if (!Pointee.hasQualifiers() &&
             Pointee->isVoidType() &&                              // to void*
             CE->getSubExpr()->getType()->isIntegerType())         // from int.
-          return CE->getSubExpr()->isNullPointerConstant(Ctx);
+          return CE->getSubExpr()->isNullPointerConstant(Ctx, NPC);
       }
     }
   } else if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(this)) {
     // Ignore the ImplicitCastExpr type entirely.
-    return ICE->getSubExpr()->isNullPointerConstant(Ctx);
+    return ICE->getSubExpr()->isNullPointerConstant(Ctx, NPC);
   } else if (const ParenExpr *PE = dyn_cast<ParenExpr>(this)) {
     // Accept ((void*)0) as a null pointer constant, as many other
     // implementations do.
-    return PE->getSubExpr()->isNullPointerConstant(Ctx);
+    return PE->getSubExpr()->isNullPointerConstant(Ctx, NPC);
   } else if (const CXXDefaultArgExpr *DefaultArg
                = dyn_cast<CXXDefaultArgExpr>(this)) {
     // See through default argument expressions
-    return DefaultArg->getExpr()->isNullPointerConstant(Ctx);
+    return DefaultArg->getExpr()->isNullPointerConstant(Ctx, NPC);
   } else if (isa<GNUNullExpr>(this)) {
     // The GNU __null extension is always a null pointer constant.
     return true;

diff --git a/lib/Analysis/CheckObjCDealloc.cpp b/lib/Analysis/CheckObjCDealloc.cpp
index d89edff2de8839cd5dbb04432c0e70d0cfa5dd1f..92e3e112d9f1ae4f0ddc2d420fe64aa8fb33ba2b 100644 (file)
--- a/lib/Analysis/CheckObjCDealloc.cpp
+++ b/lib/Analysis/CheckObjCDealloc.cpp
@@ -64,7 +64,8 @@ static bool scan_ivar_release(Stmt* S, ObjCIvarDecl* ID,
           if (E->getDecl()->getIdentifier() == SelfII)
             if (ME->getMethodDecl() == PD->getSetterMethodDecl() &&
                 ME->getNumArgs() == 1 &&
-                ME->getArg(0)->isNullPointerConstant(Ctx))
+                ME->getArg(0)->isNullPointerConstant(Ctx, 
+                                              Expr::NPC_ValueDependentIsNull))
               return true;
 
   // self.myIvar = nil;
@@ -73,7 +74,8 @@ static bool scan_ivar_release(Stmt* S, ObjCIvarDecl* ID,
       if (ObjCPropertyRefExpr* PRE =
          dyn_cast<ObjCPropertyRefExpr>(BO->getLHS()->IgnoreParenCasts()))
           if (PRE->getProperty() == PD)
-            if (BO->getRHS()->isNullPointerConstant(Ctx)) {
+            if (BO->getRHS()->isNullPointerConstant(Ctx, 
+                                            Expr::NPC_ValueDependentIsNull)) {
               // This is only a 'release' if the property kind is not
               // 'assign'.
               return PD->getSetterKind() != ObjCPropertyDecl::Assign;;

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 0bb72568b19e601a4e51f7387e7f305aafc00b03..8377d5093919f4d6a987d81c522663224e9662a5 100644 (file)
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -782,6 +782,7 @@ public:
   bool CheckPointerConversion(Expr *From, QualType ToType, 
                               CastExpr::CastKind &Kind);
   bool IsMemberPointerConversion(Expr *From, QualType FromType, QualType ToType,
+                                 bool InOverloadResolution,
                                  QualType &ConvertedType);
   bool CheckMemberPointerConversion(Expr *From, QualType ToType,
                                     CastExpr::CastKind &Kind);

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 18a997998a599bfc88f061a2703c652050112234..92bf83f0830d09dbfe7f1ed867b1bb0b35662ff8 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -93,7 +93,7 @@ bool Sema::CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall) {
     unsigned format_idx = Format->getFormatIdx() - 1;
     if (format_idx < TheCall->getNumArgs()) {
       Expr *Format = TheCall->getArg(format_idx)->IgnoreParenCasts();
-      if (!Format->isNullPointerConstant(Context))
+      if (!Format->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
         return true;
     }
   }
@@ -911,7 +911,8 @@ Sema::CheckNonNullArguments(const NonNullAttr *NonNull,
   for (NonNullAttr::iterator i = NonNull->begin(), e = NonNull->end();
        i != e; ++i) {
     const Expr *ArgExpr = TheCall->getArg(*i);
-    if (ArgExpr->isNullPointerConstant(Context))
+    if (ArgExpr->isNullPointerConstant(Context, 
+                                       Expr::NPC_ValueDependentIsNotNull))
       Diag(TheCall->getCallee()->getLocStart(), diag::warn_null_arg)
         << ArgExpr->getSourceRange();
   }

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 5bc8818dbadf9de9aeedc37a28a6d95a2e4e5f99..2f653b53202f1552f918056f64dfbd8501e62137 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -173,7 +173,8 @@ void Sema::DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
   }
   Expr *sentinelExpr = Args[sentinel];
   if (sentinelExpr && (!sentinelExpr->getType()->isPointerType() ||
-                       !sentinelExpr->isNullPointerConstant(Context))) {
+                       !sentinelExpr->isNullPointerConstant(Context,
+                                            Expr::NPC_ValueDependentIsNull))) {
     Diag(Loc, diag::warn_missing_sentinel) << isMethod;
     Diag(D->getLocation(), diag::note_sentinel_here) << isMethod;
   }
@@ -3395,12 +3396,12 @@ QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   // C99 6.5.15p6 - "if one operand is a null pointer constant, the result has
   // the type of the other operand."
   if ((LHSTy->isAnyPointerType() || LHSTy->isBlockPointerType()) &&
-      RHS->isNullPointerConstant(Context)) {
+      RHS->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(RHS, LHSTy); // promote the null to a pointer.
     return LHSTy;
   }
   if ((RHSTy->isAnyPointerType() || RHSTy->isBlockPointerType()) &&
-      LHS->isNullPointerConstant(Context)) {
+      LHS->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(LHS, RHSTy); // promote the null to a pointer.
     return RHSTy;
   }
@@ -3982,7 +3983,8 @@ Sema::CheckTransparentUnionArgumentConstraints(QualType ArgType, Expr *&rExpr) {
           break;
         }
 
-      if (rExpr->isNullPointerConstant(Context)) {
+      if (rExpr->isNullPointerConstant(Context, 
+                                       Expr::NPC_ValueDependentIsNull)) {
         ImpCastExprToType(rExpr, it->getType());
         InitField = *it;
         break;
@@ -4025,7 +4027,8 @@ Sema::CheckSingleAssignmentConstraints(QualType lhsType, Expr *&rExpr) {
   if ((lhsType->isPointerType() ||
        lhsType->isObjCObjectPointerType() ||
        lhsType->isBlockPointerType())
-      && rExpr->isNullPointerConstant(Context)) {
+      && rExpr->isNullPointerConstant(Context, 
+                                      Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(rExpr, lhsType);
     return Compatible;
   }
@@ -4454,12 +4457,14 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     Expr *literalString = 0;
     Expr *literalStringStripped = 0;
     if ((isa<StringLiteral>(LHSStripped) || isa<ObjCEncodeExpr>(LHSStripped)) &&
-        !RHSStripped->isNullPointerConstant(Context)) {
+        !RHSStripped->isNullPointerConstant(Context, 
+                                            Expr::NPC_ValueDependentIsNull)) {
       literalString = lex;
       literalStringStripped = LHSStripped;
     } else if ((isa<StringLiteral>(RHSStripped) ||
                 isa<ObjCEncodeExpr>(RHSStripped)) &&
-               !LHSStripped->isNullPointerConstant(Context)) {
+               !LHSStripped->isNullPointerConstant(Context, 
+                                            Expr::NPC_ValueDependentIsNull)) {
       literalString = rex;
       literalStringStripped = RHSStripped;
     }
@@ -4504,8 +4509,10 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
       return ResultTy;
   }
 
-  bool LHSIsNull = lex->isNullPointerConstant(Context);
-  bool RHSIsNull = rex->isNullPointerConstant(Context);
+  bool LHSIsNull = lex->isNullPointerConstant(Context, 
+                                              Expr::NPC_ValueDependentIsNull);
+  bool RHSIsNull = rex->isNullPointerConstant(Context, 
+                                              Expr::NPC_ValueDependentIsNull);
 
   // All of the following pointer related warnings are GCC extensions, except
   // when handling null pointer constants. One day, we can consider making them
@@ -5721,8 +5728,10 @@ Sema::OwningExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc,
   assert((CondExpr && LHSExpr && RHSExpr) && "Missing type argument(s)");
 
   QualType resType;
+  bool ValueDependent = false;
   if (CondExpr->isTypeDependent() || CondExpr->isValueDependent()) {
     resType = Context.DependentTy;
+    ValueDependent = true;
   } else {
     // The conditional expression is required to be a constant expression.
     llvm::APSInt condEval(32);
@@ -5734,11 +5743,15 @@ Sema::OwningExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc,
 
     // If the condition is > zero, then the AST type is the same as the LSHExpr.
     resType = condEval.getZExtValue() ? LHSExpr->getType() : RHSExpr->getType();
+    ValueDependent = condEval.getZExtValue() ? LHSExpr->isValueDependent()
+                                             : RHSExpr->isValueDependent();
   }
 
   cond.release(); expr1.release(); expr2.release();
   return Owned(new (Context) ChooseExpr(BuiltinLoc, CondExpr, LHSExpr, RHSExpr,
-                                        resType, RPLoc));
+                                        resType, RPLoc,
+                                        resType->isDependentType(),
+                                        ValueDependent));
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index e93929e94b94ef41a584815b5f6e6726052c9755..a243b2cec6277d5e2ad5c250dd9f21f22b49cfa7 100644 (file)
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -1648,11 +1648,13 @@ QualType Sema::CXXCheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   // frankly, is stupid.)
   const MemberPointerType *LMemPtr = LTy->getAs<MemberPointerType>();
   const MemberPointerType *RMemPtr = RTy->getAs<MemberPointerType>();
-  if (LMemPtr && RHS->isNullPointerConstant(Context)) {
+  if (LMemPtr && 
+      RHS->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(RHS, LTy);
     return LTy;
   }
-  if (RMemPtr && LHS->isNullPointerConstant(Context)) {
+  if (RMemPtr && 
+      LHS->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(LHS, RTy);
     return RTy;
   }
@@ -1743,11 +1745,11 @@ QualType Sema::FindCompositePointerType(Expr *&E1, Expr *&E2) {
   //   pointer operands to bring them to their composite pointer type. If
   //   one operand is a null pointer constant, the composite pointer type is
   //   the type of the other operand.
-  if (E1->isNullPointerConstant(Context)) {
+  if (E1->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(E1, T2);
     return T2;
   }
-  if (E2->isNullPointerConstant(Context)) {
+  if (E2->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)) {
     ImpCastExprToType(E2, T1);
     return T1;
   }

diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 6200beb8630c5f831518d6c17e3a477ebb70bb08..bec61ea63a29f4309cb7e8fd60d87dc13dae9ba5 100644 (file)
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -633,7 +633,8 @@ Sema::IsStandardConversion(Expr* From, QualType ToType,
     // Pointer conversions (C++ 4.10).
     SCS.Second = ICK_Pointer_Conversion;
     SCS.IncompatibleObjC = IncompatibleObjC;
-  } else if (IsMemberPointerConversion(From, FromType, ToType, FromType)) {
+  } else if (IsMemberPointerConversion(From, FromType, ToType, 
+                                       InOverloadResolution, FromType)) {
     // Pointer to member conversions (4.11).
     SCS.Second = ICK_Pointer_Member;
   } else if (ToType->isBooleanType() &&
@@ -883,7 +884,9 @@ static bool isNullPointerConstantForConversion(Expr *Expr,
       Expr->getType()->isIntegralType())
     return !InOverloadResolution;
 
-  return Expr->isNullPointerConstant(Context);
+  return Expr->isNullPointerConstant(Context,
+                    InOverloadResolution? Expr::NPC_ValueDependentIsNotNull
+                                        : Expr::NPC_ValueDependentIsNull);
 }
 
 /// IsPointerConversion - Determines whether the conversion of the
@@ -1188,13 +1191,17 @@ bool Sema::CheckPointerConversion(Expr *From, QualType ToType,
 /// If so, returns true and places the converted type (that might differ from
 /// ToType in its cv-qualifiers at some level) into ConvertedType.
 bool Sema::IsMemberPointerConversion(Expr *From, QualType FromType,
-                                     QualType ToType, QualType &ConvertedType) {
+                                     QualType ToType, 
+                                     bool InOverloadResolution,
+                                     QualType &ConvertedType) {
   const MemberPointerType *ToTypePtr = ToType->getAs<MemberPointerType>();
   if (!ToTypePtr)
     return false;
 
   // A null pointer constant can be converted to a member pointer (C++ 4.11p1)
-  if (From->isNullPointerConstant(Context)) {
+  if (From->isNullPointerConstant(Context,
+                    InOverloadResolution? Expr::NPC_ValueDependentIsNotNull
+                                        : Expr::NPC_ValueDependentIsNull)) {
     ConvertedType = ToType;
     return true;
   }
@@ -1231,7 +1238,8 @@ bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
   const MemberPointerType *FromPtrType = FromType->getAs<MemberPointerType>();
   if (!FromPtrType) {
     // This must be a null pointer to member pointer conversion
-    assert(From->isNullPointerConstant(Context) &&
+    assert(From->isNullPointerConstant(Context, 
+                                       Expr::NPC_ValueDependentIsNull) &&
            "Expr must be null pointer constant!");
     Kind = CastExpr::CK_NullToMemberPointer;
     return false;

diff --git a/test/SemaTemplate/value-dependent-null-pointer-constant.cpp b/test/SemaTemplate/value-dependent-null-pointer-constant.cpp
new file mode 100644 (file)
index 0000000..8bde127
--- /dev/null
+++ b/test/SemaTemplate/value-dependent-null-pointer-constant.cpp
@@ -0,0 +1,29 @@
+// RUN: clang-cc -fsyntax-only %s
+
+template<typename T, int N>
+struct X0 {
+  const char *f0(bool Cond) {
+    return Cond? "honk" : N;
+  }
+
+  const char *f1(bool Cond) {
+    return Cond? N : "honk";
+  }
+  
+  bool f2(const char *str) {
+    return str == N;
+  }
+};
+
+// PR4996
+template<unsigned I> int f0() { 
+  return __builtin_choose_expr(I, 0, 1); 
+}
+
+// PR5041
+struct A { };
+
+template <typename T> void f(T *t)
+{
+  (void)static_cast<void*>(static_cast<A*>(t));
+}
\ No newline at end of file