[C2x] Update 'nullptr' implementation based on CD comments

AaronBallman · AaronBallman · commit 9bb28a18d962 · 2023-05-03T14:50:15.000-04:00
We filed some CD ballot comments which WG14 considered during the ballot comment resolution meetings in Jan and Feb 2023, and this updates our implementation based on the decisions reached. Those decisions were (paraphrased for brevity): US 9-034 (REJECTED) allow (void *)nullptr to be a null pointer constant US 10-035 (ACCEPTED) accept the following code, as in C++: void func(nullptr_t); func(0); US 22-058 (REJECTED) accept the following code, as in C++: nullptr_t val; (void)(1 ? val : 0); (void)(1 ? nullptr : 0); US 23-062 (REJECTED) reject the following code, as in C++: nullptr_t val; bool b1 = val; bool b2 = nullptr; US 24-061 (ACCEPTED) accept the following code, as in C++: nullptr_t val; val = 0; US 21-068 (ACCEPTED) accept the following code, as in C++: (nullptr_t)nullptr; GB-071 (ACCEPTED) accept the following code, as in C++: nullptr_t val; (void)(val == nullptr); This patch updates the implementation as appropriate, but is primarily focused around US 10-035, US 24-061, and US 23-062 in terms of functional changes. Differential Revision: https://reviews.llvm.org/D148800
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -139,6 +139,25 @@ C2x Feature Support
   removed, as this is no longer a GNU extension but a C2x extension. You can
   use ``-Wno-c2x-extensions`` to silence the extension warning instead.
 
+- Updated the implementation of
+  `WG14 N3042 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3042.htm>`_
+  based on decisions reached during the WG14 CD Ballot Resolution meetings held
+  in Jan and Feb 2023. This should complete the implementation of ``nullptr``
+  and ``nullptr_t`` in C. The specific changes are:
+
+  .. code-block:: c
+
+    void func(nullptr_t);
+    func(0); // Previously required to be rejected, is now accepted.
+    func((void *)0); // Previously required to be rejected, is now accepted.
+
+    nullptr_t val;
+    val = 0; // Previously required to be rejected, is now accepted.
+    val = (void *)0; // Previously required to be rejected, is now accepted.
+
+    bool b = nullptr; // Was incorrectly rejected by Clang, is now accepted.
+
+
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 - Clang now saves the address of ABI-indirect function parameters on the stack,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -10118,6 +10118,15 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS,
     return Incompatible;
   }
 
+  // Conversion to nullptr_t (C2x only)
+  if (getLangOpts().C2x && LHSType->isNullPtrType() &&
+      RHS.get()->isNullPointerConstant(Context,
+                                       Expr::NPC_ValueDependentIsNull)) {
+    // null -> nullptr_t
+    Kind = CK_NullToPointer;
+    return Compatible;
+  }
+
   // Conversions from pointers that are not covered by the above.
   if (isa<PointerType>(RHSType)) {
     // T* -> _Bool
@@ -10335,12 +10344,13 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
   QualType LHSTypeAfterConversion = LHSType.getAtomicUnqualifiedType();
 
   // C99 6.5.16.1p1: the left operand is a pointer and the right is
-  // a null pointer constant.
+  // a null pointer constant <C2x>or its type is nullptr_t;</C2x>.
   if ((LHSTypeAfterConversion->isPointerType() ||
        LHSTypeAfterConversion->isObjCObjectPointerType() ||
        LHSTypeAfterConversion->isBlockPointerType()) &&
-      RHS.get()->isNullPointerConstant(Context,
-                                       Expr::NPC_ValueDependentIsNull)) {
+      ((getLangOpts().C2x && RHS.get()->getType()->isNullPtrType()) ||
+       RHS.get()->isNullPointerConstant(Context,
+                                        Expr::NPC_ValueDependentIsNull))) {
     if (Diagnose || ConvertRHS) {
       CastKind Kind;
       CXXCastPath Path;
@@ -10351,6 +10361,26 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
     }
     return Compatible;
   }
+  // C2x 6.5.16.1p1: the left operand has type atomic, qualified, or
+  // unqualified bool, and the right operand is a pointer or its type is
+  // nullptr_t.
+  if (getLangOpts().C2x && LHSType->isBooleanType() &&
+      RHS.get()->getType()->isNullPtrType()) {
+    // NB: T* -> _Bool is handled in CheckAssignmentConstraints, this only
+    // only handles nullptr -> _Bool due to needing an extra conversion
+    // step.
+    // We model this by converting from nullptr -> void * and then let the
+    // conversion from void * -> _Bool happen naturally.
+    if (Diagnose || ConvertRHS) {
+      CastKind Kind;
+      CXXCastPath Path;
+      CheckPointerConversion(RHS.get(), Context.VoidPtrTy, Kind, Path,
+                             /*IgnoreBaseAccess=*/false, Diagnose);
+      if (ConvertRHS)
+        RHS = ImpCastExprToType(RHS.get(), Context.VoidPtrTy, Kind, VK_PRValue,
+                                &Path);
+    }
+  }
 
   // OpenCL queue_t type assignment.
   if (LHSType->isQueueT() && RHS.get()->isNullPointerConstant(
diff --git a/clang/test/C/C2x/n3042.c b/clang/test/C/C2x/n3042.c
@@ -1,10 +1,7 @@
 // RUN: %clang_cc1 -verify -ffreestanding -Wno-unused -std=c2x %s
 
-/* WG14 N3042: partial
+/* WG14 N3042: full
  * Introduce the nullptr constant
- *
- * Claiming partial support for this feature until the WG14 NB comments can be
- * resolved to know what the correct behavior really should be.
  */
 
 #include <stddef.h>
@@ -21,25 +18,17 @@
 void questionable_behaviors() {
   nullptr_t val;
 
-  // FIXME: This code is intended to be rejected by C and is accepted by C++.
-  // We've filed an NB comment with WG14 about the incompatibility.
+  // This code is intended to be rejected by C and is accepted by C++. We filed
+  // an NB comment asking for this to be changed, but WG14 declined.
   (void)(1 ? val : 0);     // expected-error {{non-pointer operand type 'int' incompatible with nullptr}}
   (void)(1 ? nullptr : 0); // expected-error {{non-pointer operand type 'int' incompatible with nullptr}}
 
-  // FIXME: This code is intended to be accepted by C and is rejected by C++.
-  // We're following the C++ semantics until WG14 has resolved the NB comments
-  // we've filed about the incompatibility.
-  _Bool another = val;    // expected-error {{initializing 'bool' with an expression of incompatible type 'nullptr_t'}}
-  another = val;          // expected-error {{assigning to 'bool' from incompatible type 'nullptr_t'}}
-  _Bool again = nullptr;  // expected-error {{initializing 'bool' with an expression of incompatible type 'nullptr_t'}}
-  again = nullptr;        // expected-error {{assigning to 'bool' from incompatible type 'nullptr_t'}}
-
-  // FIXME: This code is intended to be rejected by C and is accepted by C++.
-  // We've filed an NB comment with WG14 about the incompatibility.
-  val = 0; // expected-error {{assigning to 'nullptr_t' from incompatible type 'int'}}
-
-  // Not accepted in C++ but might want to accept in C as a null pointer constant?
-  val = (void *)0; // expected-error {{assigning to 'nullptr_t' from incompatible type 'void *'}}
+  // This code is intended to be accepted by C and is rejected by C++. We filed
+  // an NB comment asking for this to be changed, but WG14 declined.
+  _Bool another = val;    // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
+  another = val;          // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
+  _Bool again = nullptr;  // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
+  again = nullptr;        // expected-warning {{implicit conversion of nullptr constant to 'bool'}}
 }
 
 void test() {
@@ -67,6 +56,14 @@ void test() {
   // How about the null pointer named constant?
   &nullptr; // expected-error {{cannot take the address of an rvalue of type 'nullptr_t'}}
 
+  // Assignment from a null pointer constant to a nullptr_t is valid.
+  null_val = 0;
+  null_val = (void *)0;
+
+  // Assignment from a nullptr_t to a pointer is also valid.
+  typed_ptr = null_val;
+  void *other_ptr = null_val;
+
   // Can it be used in all the places a scalar can be used?
   if (null_val) {}
   if (!null_val) {}
@@ -162,18 +159,15 @@ void test() {
 }
 
 // Can we use it as a function parameter?
-void null_param(nullptr_t); // expected-note 2 {{passing argument to parameter here}}
+void null_param(nullptr_t);
 
 void other_test() {
   // Can we call the function properly?
   null_param(nullptr);
 
-  // Do we get reasonable diagnostics when we can't call the function?
-  null_param((void *)0); // expected-error {{passing 'void *' to parameter of incompatible type 'nullptr_t'}}
-
-  // FIXME: The paper requires this to be rejected, but it is accepted in C++.
-  // This should be addressed after WG14 has processed national body comments.
-  null_param(0);         // expected-error {{passing 'int' to parameter of incompatible type 'nullptr_t'}}
+  // We can pass any kind of null pointer constant.
+  null_param((void *)0);
+  null_param(0);
 }
 
 
@@ -182,3 +176,7 @@ void format_specifiers() {
   // Don't warn when using nullptr with %p.
   printf("%p", nullptr);
 }
+
+// Ensure that conversion from a null pointer constant to nullptr_t is
+// valid in a constant expression.
+static_assert((nullptr_t){} == 0);
diff --git a/clang/test/CodeGen/nullptr.c b/clang/test/CodeGen/nullptr.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -S %s -std=c2x -emit-llvm -o - | FileCheck %s
+
+// Test that null <-> nullptr_t conversions work as expected.
+typedef typeof(nullptr) nullptr_t;
+
+nullptr_t nullptr_t_val;
+
+void bool_func(bool);
+void nullptr_func(nullptr_t);
+
+void test() {
+  // Test initialization
+  bool bool_from_nullptr_t = nullptr_t_val;
+  nullptr_t nullptr_t_from_nullptr = nullptr;
+  void *vp_from_nullptr_t = nullptr_t_val;
+  nullptr_t nullptr_t_from_vp = (void *)0;
+  nullptr_t nullptr_t_from_int = 0;
+
+  // Test assignment
+  bool_from_nullptr_t = nullptr_t_val;
+  nullptr_t_from_nullptr = nullptr;
+  vp_from_nullptr_t = nullptr_t_val;
+  nullptr_t_from_vp = (void *)0;
+  nullptr_t_from_int = 0;
+
+  // Test calls
+  bool_func(nullptr_t_from_nullptr);
+  nullptr_func(nullptr_t_from_nullptr);
+  nullptr_func(0);
+  nullptr_func((void *)0);
+  nullptr_func(nullptr);
+  nullptr_func(false);
+
+  // Allocation of locals
+  // CHECK: %[[bool_from_nullptr_t:.*]] = alloca i8, align 1
+  // CHECK: %[[nullptr_t_from_nullptr:.*]] = alloca ptr, align 8
+  // CHECK: %[[vp_from_nullptr_t:.*]] = alloca ptr, align 8
+  // CHECK: %[[nullptr_t_from_vp:.*]] = alloca ptr, align 8
+  // CHECK: %[[nullptr_t_from_int:.*]] = alloca ptr, align 8
+
+  // Initialization of locals
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+
+  // Assignment expressions
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+
+  // Calls
+  // CHECK: call void @bool_func(i1 noundef zeroext false)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+}
+
diff --git a/clang/test/Sema/nullptr.c b/clang/test/Sema/nullptr.c
@@ -16,8 +16,8 @@ nullptr_t f(nullptr_t null)
   p = null;
   int *pi = nullptr;
   pi = null;
-  null = 0; // expected-error {{assigning to 'nullptr_t' from incompatible type 'int'}}
-  bool b = nullptr; // expected-error {{initializing 'bool' with an expression of incompatible type 'nullptr_t'}}
+  null = 0;
+  bool b = nullptr;
 
   // Can't convert nullptr to integral implicitly.
   uintptr_t i = nullptr; // expected-error-re {{initializing 'uintptr_t' (aka '{{.*}}') with an expression of incompatible type 'nullptr_t'}}
@@ -77,6 +77,9 @@ void h() {
 
 static_assert(sizeof(nullptr_t) == sizeof(void*), "");
 
+static_assert(!nullptr, "");
+static_assert(!(bool){nullptr}, "");
+
 static_assert(!(nullptr < nullptr), ""); // expected-error {{invalid operands to binary expression}}
 static_assert(!(nullptr > nullptr), ""); // expected-error {{invalid operands to binary expression}}
 static_assert(  nullptr <= nullptr, ""); // expected-error {{invalid operands to binary expression}}
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
@@ -1215,13 +1215,7 @@ <h2 id="c2x">C2x implementation status</h2>
     <tr>
       <td>Introduce the nullptr constant</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3042.htm">N3042</a></td>
-      <td class="partial" align="center">
-        <details><summary>Partial</summary>
-          Parts of the implementation may be incorrect until WG14 has completed NB comment
-          resolution for incompatibilities with C++ that were discovered. The major use cases
-          and usage patterns should work well, though.
-        </details>
-      </td>
+      <td class="unreleased" align="center">Clang 17</td>
     </tr>
     <tr>
       <td>Memory layout of unions</td>
