[clang] Allow relational comparisons between unequal pointers to `void` in constant expressions #89449

offsetof · 2024-04-19T20:21:45Z

As was recently clarified by CWG2749, the result of applying relational operators to unequal operands of type "pointer to cv void" is not always unspecified under the current wording.

This patch disables the relevant diagnostic in C++17 and later modes.

Fixes #45653

Since C++17, it is no longer the case that applying relational operators to unequal pointers to "void" always produces an unspecified result.

github-actions · 2024-04-19T20:22:04Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-04-19T20:22:34Z

@llvm/pr-subscribers-clang

Author: None (offsetof)

Changes

This patch disables the relevant diagnostic in C++17 and later modes.

Fixes #45653

Full diff: https://github.com/llvm/llvm-project/pull/89449.diff

7 Files Affected:

(modified) clang/lib/AST/ExprConstant.cpp (+4-1)
(modified) clang/test/AST/Interp/literals.cpp (+5-5)
(modified) clang/test/CXX/drs/dr25xx.cpp (+1)
(modified) clang/test/CXX/drs/dr26xx.cpp (+2)
(modified) clang/test/CXX/drs/dr27xx.cpp (+22)
(modified) clang/test/CXX/expr/expr.const/p2-0x.cpp (+13-10)
(modified) clang/www/cxx_dr_status.html (+3-3)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 73ae8d8efb23a2..2d480edab833c8 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13510,7 +13510,10 @@ EvaluateComparisonBinaryOperator(EvalInfo &Info, const BinaryOperator *E,
     //   operator is <= or >= and false otherwise; otherwise the result is
     //   unspecified.
     // We interpret this as applying to pointers to *cv* void.
-    if (LHSTy->isVoidPointerType() && LHSOffset != RHSOffset && IsRelational)
+    // This only applies until C++14; in C++17 a "cv void*" value can "point to
+    // an object" and is not treated specially in [expr.rel].
+    if (!Info.getLangOpts().CPlusPlus17 &&
+        LHSTy->isVoidPointerType() && LHSOffset != RHSOffset && IsRelational)
       Info.CCEDiag(E, diag::note_constexpr_void_comparison);
 
     // C++11 [expr.rel]p2:
diff --git a/clang/test/AST/Interp/literals.cpp b/clang/test/AST/Interp/literals.cpp
index 277438d2e63114..91857228fcddc7 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -Wno-vla -fms-extensions -std=c++11 -verify=expected,both %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -Wno-vla -fms-extensions -std=c++20 -verify=expected,both %s
-// RUN: %clang_cc1 -std=c++11 -fms-extensions -Wno-vla -verify=ref,both %s
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -Wno-vla -verify=ref,ref-cxx11,both %s
 // RUN: %clang_cc1 -std=c++20 -fms-extensions -Wno-vla -verify=ref,both %s
 
 #define INT_MIN (~__INT_MAX__)
@@ -193,10 +193,10 @@ namespace PointerComparison {
 
   /// FIXME: These two are rejected by the current interpreter, but
   ///   accepted by GCC.
-  constexpr bool v5 = qv >= pv; // ref-error {{constant expression}} \
-                                // ref-note {{unequal pointers to void}}
-  constexpr bool v8 = qv > (void*)&s.a; // ref-error {{constant expression}} \
-                                        // ref-note {{unequal pointers to void}}
+  constexpr bool v5 = qv >= pv; // ref-cxx11-error {{constant expression}} \
+                                // ref-cxx11-note {{unequal pointers to void}}
+  constexpr bool v8 = qv > (void*)&s.a; // ref-cxx11-error {{constant expression}} \
+                                        // ref-cxx11-note {{unequal pointers to void}}
   constexpr bool v6 = qv > null; // both-error {{must be initialized by a constant expression}} \
                                  // both-note {{comparison between '&s.b' and 'nullptr' has unspecified value}}
 
diff --git a/clang/test/CXX/drs/dr25xx.cpp b/clang/test/CXX/drs/dr25xx.cpp
index 62b2a0a088cc13..672979c2be8ef8 100644
--- a/clang/test/CXX/drs/dr25xx.cpp
+++ b/clang/test/CXX/drs/dr25xx.cpp
@@ -83,6 +83,7 @@ using ::cwg2521::operator""_div;
 #endif
 } // namespace cwg2521
 
+// cwg2526: sup 2749
 
 #if __cplusplus >= 202302L
 namespace cwg2553 { // cwg2553: 18 review 2023-07-14
diff --git a/clang/test/CXX/drs/dr26xx.cpp b/clang/test/CXX/drs/dr26xx.cpp
index f7a05b9827a235..de18787bcab96b 100644
--- a/clang/test/CXX/drs/dr26xx.cpp
+++ b/clang/test/CXX/drs/dr26xx.cpp
@@ -238,3 +238,5 @@ void test() {
 }
 }
 #endif
+
+// cwg2696: dup 2749
diff --git a/clang/test/CXX/drs/dr27xx.cpp b/clang/test/CXX/drs/dr27xx.cpp
index 0434427d6c92a3..1f5b9e571079c9 100644
--- a/clang/test/CXX/drs/dr27xx.cpp
+++ b/clang/test/CXX/drs/dr27xx.cpp
@@ -10,6 +10,28 @@
 // expected-no-diagnostics
 #endif
 
+namespace cwg2749 { // cwg2749: 19
+#if __cplusplus >= 201703L
+
+constexpr bool f() {
+  int arr[2] {};
+  return (void*)arr < arr + 1;
+}
+static_assert(f());
+
+constexpr bool g() {
+  struct {
+    char c;
+    short s;
+    int i;
+  } s {};
+  return (void*)&s.c < &s.i;
+}
+static_assert(g());
+
+#endif
+}
+
 namespace cwg2759 { // cwg2759: 19
 #if __cplusplus >= 201103L
 
diff --git a/clang/test/CXX/expr/expr.const/p2-0x.cpp b/clang/test/CXX/expr/expr.const/p2-0x.cpp
index e3cd057baba75f..54e163f05e4524 100644
--- a/clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ b/clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -571,18 +571,21 @@ namespace UnspecifiedRelations {
   // [expr.rel]p3: Pointers to void can be compared [...] if both pointers
   // represent the same address or are both the null pointer [...]; otherwise
   // the result is unspecified.
-  struct S { int a, b; } s;
+  struct S { int a, b; static int c; } s;
   constexpr void *null = 0;
-  constexpr void *pv = (void*)&s.a;
-  constexpr void *qv = (void*)&s.b;
+  constexpr void *av = (void*)&s.a;
+  constexpr void *bv = (void*)&s.b;
+  constexpr void *cv = (void*)&s.c;
   constexpr bool v1 = null < (int*)0;
-  constexpr bool v2 = null < pv; // expected-error {{constant expression}} expected-note {{comparison between 'nullptr' and '&s.a' has unspecified value}}
-  constexpr bool v3 = null == pv; // ok
-  constexpr bool v4 = qv == pv; // ok
-  constexpr bool v5 = qv >= pv; // expected-error {{constant expression}} expected-note {{unequal pointers to void}}
-  constexpr bool v6 = qv > null; // expected-error {{constant expression}} expected-note {{comparison between '&s.b' and 'nullptr' has unspecified value}}
-  constexpr bool v7 = qv <= (void*)&s.b; // ok
-  constexpr bool v8 = qv > (void*)&s.a; // expected-error {{constant expression}} expected-note {{unequal pointers to void}}
+  constexpr bool v2 = null < av; // expected-error {{constant expression}} expected-note {{comparison between 'nullptr' and '&s.a' has unspecified value}}
+  constexpr bool v3 = null == av; // ok
+  constexpr bool v4 = bv == av; // ok
+  constexpr bool v5 = bv >= av; // cxx11-error {{constant expression}} cxx11-note {{unequal pointers to void}}
+  constexpr bool v6 = bv > null; // expected-error {{constant expression}} expected-note {{comparison between '&s.b' and 'nullptr' has unspecified value}}
+  constexpr bool v7 = bv <= (void*)&s.b; // ok
+  constexpr bool v8 = bv > (void*)&s.a; // cxx11-error {{constant expression}} cxx11-note {{unequal pointers to void}}
+  constexpr bool v9 = cv > bv; // expected-error {{constant expression}} expected-note {{comparison between '&c' and '&s.b' has unspecified value}}
+  constexpr bool v10 = cv <= (void*)&s; // expected-error {{constant expression}} expected-note {{comparison between '&c' and '&s' has unspecified value}}
 }
 
 // - an assignment or a compound assignment (5.17); or
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 83b71e7c122d1e..12075b5aa0451e 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -14964,7 +14964,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2526.html">2526</a></td>
     <td>C++23</td>
     <td>Relational comparison of <TT>void*</TT> pointers</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Superseded by <a href="#2749">2749</a></td>
   </tr>
   <tr id="2527">
     <td><a href="https://cplusplus.github.io/CWG/issues/2527.html">2527</a></td>
@@ -15984,7 +15984,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2696.html">2696</a></td>
     <td>dup</td>
     <td>Relational comparisons of pointers to <TT>void</TT></td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Duplicate of <a href="#2749">2749</a></td>
   </tr>
   <tr id="2697">
     <td><a href="https://cplusplus.github.io/CWG/issues/2697.html">2697</a></td>
@@ -16302,7 +16302,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2749.html">2749</a></td>
     <td>DR</td>
     <td>Treatment of "pointer to void" for relational comparisons</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 19</td>
   </tr>
   <tr id="2750">
     <td><a href="https://cplusplus.github.io/CWG/issues/2750.html">2750</a></td>

Endilll

Changes to DR tests look good to me, thank you!
Can you send an email to Jens Maurer asking him to mark 2526 as superseded by 2749?
I'm also somewhat suspicious of the fact that we check only LHS for being void*, but that's what we did previously as well, so this might be fine.

cor3ntin · 2024-04-20T10:25:41Z

@Endilll The issues do not supersede one another , despite concerning the same note.
The tests for CWG2526 are in clang/test/CXX/expr/expr.const/p2-0x.cpp,
so we should make that as fixed by copying these tests in dr25xx.cpp

Why do you think this should not apply to C++14 @offsetof ?

[clang] Allow constexpr relational comparison of unequal "void*" values

44e3c7a

Since C++17, it is no longer the case that applying relational operators to unequal pointers to "void" always produces an unspecified result.

offsetof requested review from tbaederr and Endilll as code owners April 19, 2024 20:21

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 19, 2024

Endilll reviewed Apr 20, 2024

View reviewed changes

tbaederr mentioned this pull request Jun 6, 2024

[Clang] CWG2749: relational operators involving pointers to void #93046

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Allow relational comparisons between unequal pointers to `void` in constant expressions #89449

[clang] Allow relational comparisons between unequal pointers to `void` in constant expressions #89449

offsetof commented Apr 19, 2024 •

edited

github-actions bot commented Apr 19, 2024

llvmbot commented Apr 19, 2024

Endilll left a comment

cor3ntin commented Apr 20, 2024

[clang] Allow relational comparisons between unequal pointers to void in constant expressions #89449

Are you sure you want to change the base?

[clang] Allow relational comparisons between unequal pointers to void in constant expressions #89449

Conversation

offsetof commented Apr 19, 2024 • edited

github-actions bot commented Apr 19, 2024

llvmbot commented Apr 19, 2024

Endilll left a comment

Choose a reason for hiding this comment

cor3ntin commented Apr 20, 2024

[clang] Allow relational comparisons between unequal pointers to `void` in constant expressions #89449

[clang] Allow relational comparisons between unequal pointers to `void` in constant expressions #89449

offsetof commented Apr 19, 2024 •

edited