[PATCH] Remove redundant check for value dependency of null pointer constant

[dyp]

Hi rsmith, doug.gregor,

In SemaOverload.cpp, the isNullPointerConstantForConversion function contains a redundant check for value dependency of a null pointer constant. The same check is performed in Expr::isNullPointerConstant, where the implementation is adapted to various special cases and better maintained. E.g. in C++11 and C++14 modes, the version in SemaOverload.cpp yields wrong results.

As it turned out, the test case for PR20110 already contained a note that this test should actually fail in C++11 mode. I've split this test case into two files for C++03 mode (which reproduces the original behaviour) and C++11 mode (new behaviour).

http://reviews.llvm.org/D10152

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/PR20110.cpp
  test/SemaCXX/cxx11-PR20110.cpp

Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -1950,15 +1950,17 @@
 static bool isNullPointerConstantForConversion(Expr *Expr,
                                                bool InOverloadResolution,
                                                ASTContext &Context) {
-  // Handle value-dependent integral null pointer constants correctly.
-  // http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#903
-  if (Expr->isValueDependent() && !Expr->isTypeDependent() &&
-      Expr->getType()->isIntegerType() && !Expr->getType()->isEnumeralType())
-    return !InOverloadResolution;
-
-  return Expr->isNullPointerConstant(Context,
-                    InOverloadResolution? Expr::NPC_ValueDependentIsNotNull
-                                        : Expr::NPC_ValueDependentIsNull);
+  // Within overload resolution, pre CWG 903, value-dependent integral constant
+  // expressions shall not be considered as null pointer constants.
+  // http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#903
+  // The resolution of CWG 903 requires null pointer constants to be literals,
+  // which has also been implemented in C++11 mode. In those cases, the
+  // @InOverloadResolution parameter is effectively unused.
+  const Expr::NullPointerConstantValueDependence vd = InOverloadResolution
+    ? Expr::NPC_ValueDependentIsNotNull
+    : Expr::NPC_ValueDependentIsNull;
+  
+  return Expr->isNullPointerConstant(Context, vd);
 }
 
 /// IsPointerConversion - Determines whether the conversion of the
Index: test/SemaCXX/PR20110.cpp
===================================================================
--- test/SemaCXX/PR20110.cpp
+++ test/SemaCXX/PR20110.cpp
@@ -1,9 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++03 %s
 // expected-no-diagnostics
 
-// FIXME: These templates should trigger errors in C++11 mode.
-
 template <char const *p>
 class A {
   char const *get_p() { return *p; }
Index: test/SemaCXX/cxx11-PR20110.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/cxx11-PR20110.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+template <char const *p>
+class A {
+  char const *get_p() { return *p; } // expected-error {{cannot initialize}}
+};
+template <int p>
+class B {
+  char const *get_p() { return p; } // expected-error {{cannot initialize}}
+};
+

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D10152.26865.patch
Type: text/x-patch
Size: 2595 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150601/482b5ed2/attachment.bin>