[Clang] Warning as error Array Comparisons from C++26 #118872

AmrDeveloper · 2024-12-05T21:28:36Z

Starting from C++26 the array comparison warning should converted to an error.

Fixes: #117859

Starting from C++26 the array comparison warning should converted to an error.

llvmbot · 2024-12-05T21:29:15Z

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Starting from C++26 the array comparison warning should converted to an error.

Fixes: #117859

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

7 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5)
(modified) clang/lib/Sema/SemaExpr.cpp (+3-1)
(modified) clang/test/Sema/warn-stringcompare.c (+5-4)
(modified) clang/test/SemaCXX/warn-array-comparion.cpp (+4-2)
(modified) clang/test/SemaCXX/warn-self-comparisons.cpp (+2)
(modified) clang/www/cxx_status.html (+1-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ad5ef2119d3654..6719cf42970ba4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -428,6 +428,9 @@ New Compiler Flags
 - The ``-Warray-compare`` warning has been added to warn about array comparison
   on versions older than C++20.
 
+- The ``-Warray-compare-cxx26`` warning has been added to warn about array comparison
+  starting from C++26, this warn is error by default.
+
 Deprecated Compiler Flags
 -------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 447358f0a5f382..0bf9303ba50f45 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10274,6 +10274,11 @@ def warn_array_comparison : Warning<
   "to compare array addresses, use unary '+' to decay operands to pointers">,
   InGroup<DiagGroup<"array-compare">>;
 
+def warn_array_comparison_cxx26 : Warning<
+  "comparison between two arrays compare their addresses not their contents; "
+  "to compare array addresses, use unary '+' to decay operands to pointers">,
+  InGroup<DiagGroup<"array-compare-cxx26">>, DefaultError;
+
 def warn_stringcompare : Warning<
   "result of comparison against %select{a string literal|@encode}0 is "
   "unspecified (use an explicit string comparison function instead)">,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4ffce2c1236610..14564b99de44c5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11843,7 +11843,9 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
       RHSStripped->getType()->isArrayType()) {
     auto IsDeprArrayComparionIgnored =
         S.getDiagnostics().isIgnored(diag::warn_depr_array_comparison, Loc);
-    auto DiagID = !S.getLangOpts().CPlusPlus20 || IsDeprArrayComparionIgnored
+    auto DiagID = S.getLangOpts().CPlusPlus26
+                      ? diag::warn_array_comparison_cxx26
+                  : !S.getLangOpts().CPlusPlus20 || IsDeprArrayComparionIgnored
                       ? diag::warn_array_comparison
                       : diag::warn_depr_array_comparison;
     S.Diag(Loc, DiagID) << LHS->getSourceRange() << RHS->getSourceRange()
diff --git a/clang/test/Sema/warn-stringcompare.c b/clang/test/Sema/warn-stringcompare.c
index 78145cf42578a4..93c2bc31fea0a3 100644
--- a/clang/test/Sema/warn-stringcompare.c
+++ b/clang/test/Sema/warn-stringcompare.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -x c -fsyntax-only -verify %s
 // RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cxx %s
+// RUN: %clang_cc1 -x c++ -std=c++26 -fsyntax-only -verify=expected,cxx-26 %s
 
 #define DELIM "/"
 #define DOT "."
@@ -15,15 +16,15 @@ void test(const char *d) {
   if (NULL == "/")
     return;
   if ("/" != DELIM) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
-    return;         // cxx-warning@-1 {{comparison between two arrays}}
+    return;         // cxx-warning@-1 {{comparison between two arrays}} cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
   if (DELIM == "/") // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
-    return;         // cxx-warning@-1 {{comparison between two arrays}}
+    return;         // cxx-warning@-1 {{comparison between two arrays}} cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
   if (DELIM != NULL)
     return;
   if (NULL == DELIM)
     return;
   if (DOT != DELIM) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
-    return;         // cxx-warning@-1 {{comparison between two arrays}}
+    return;         // cxx-warning@-1 {{comparison between two arrays}} cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
   if (DELIM == DOT) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
-    return;         // cxx-warning@-1 {{comparison between two arrays}}
+    return;         // cxx-warning@-1 {{comparison between two arrays}} cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
 }
diff --git a/clang/test/SemaCXX/warn-array-comparion.cpp b/clang/test/SemaCXX/warn-array-comparion.cpp
index a6eaaab22fc16d..87b5c4fb91e3d7 100644
--- a/clang/test/SemaCXX/warn-array-comparion.cpp
+++ b/clang/test/SemaCXX/warn-array-comparion.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s -verify=expected,not-cxx20
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wno-deprecated-array-compare -verify %s -verify=expected,not-cxx20
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wdeprecated -verify %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++26 -fsyntax-only -Wdeprecated -verify %s -verify=expected,cxx26
 
 typedef struct {
   char str[16];
@@ -9,13 +10,14 @@ typedef struct {
 
 bool object_equal(const Object &obj1, const Object &obj2) {
   if (obj1.str != obj2.str) // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
-    return false;
+    return false;           // cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
   if (obj1.id != obj2.id) // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
-    return false;
+    return false;         // cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
   return true;
 }
 
 
 void foo(int (&array1)[2], int (&array2)[2]) {
   if (array1 == array2) { } // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
+                            // cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
 }
diff --git a/clang/test/SemaCXX/warn-self-comparisons.cpp b/clang/test/SemaCXX/warn-self-comparisons.cpp
index 3847c2d918bf61..c7bb5fcc8fd00d 100644
--- a/clang/test/SemaCXX/warn-self-comparisons.cpp
+++ b/clang/test/SemaCXX/warn-self-comparisons.cpp
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s -verify=expected,not-cxx20
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wdeprecated -verify %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++26 -fsyntax-only -Wdeprecated -verify %s -verify=expected,cxx26
 
 void f(int (&array1)[2], int (&array2)[2]) {
   if (array1 == array2) { } // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
+                            // cxx26-error@-1 {{comparison between two arrays compare their addresses not their contents}}
 }
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index fdb9807b1168c7..839b66e7a082d3 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -239,7 +239,7 @@ <h2 id="cxx26">C++2c implementation status</h2>
  <tr>
   <td>Remove Deprecated Array Comparisons from C++26</td>
   <td><a href="https://wg21.link/P2865R6">P2865R6</a></td>
-  <td class="none" align="center">No</td>
+  <td class="none" align="center">Clang 20</td>
  </tr>
  <tr>
   <td>Structured Bindings can introduce a Pack</td>

erichkeane · 2024-12-05T21:52:45Z

clang/docs/ReleaseNotes.rst

@@ -428,6 +428,9 @@ New Compiler Flags
 - The ``-Warray-compare`` warning has been added to warn about array comparison
  on versions older than C++20.

+- The ``-Warray-compare-cxx26`` warning has been added to warn about array comparison
+  starting from C++26, this warn is error by default.


Suggested change

starting from C++26, this warn is error by default.

starting from C++26, this warning is enabled as an error by default.

erichkeane · 2024-12-05T21:53:33Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10274,6 +10274,11 @@ def warn_array_comparison : Warning<
  "to compare array addresses, use unary '+' to decay operands to pointers">,
  InGroup<DiagGroup<"array-compare">>;

+def warn_array_comparison_cxx26 : Warning<
+  "comparison between two arrays compare their addresses not their contents; "


I wonder if this should just say 'is ill formed in C++26' or something instead of just repeating the same message?

I agree, "comparison between two arrays is ll-formed in C++26; " will be better in this case

I would prefer "not allowed" instead of "ill-formed".

Example:

llvm-project/clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines 665 to 667 in 02ad623

def err_access_decl : Error<

"ISO C++11 does not allow access declarations; "

"use using declarations instead">;

I found that we have diagnostics with 'not allowed' and 'ill-formed' in the codebase, I am okay with both, and we can agree on opinion.

What do you think the best option here?

@erichkeane @cor3ntin @zwuis

I don't think it matters a lot. ill formed conveys it's a standard requirement pretty well

zwuis · 2024-12-06T04:02:48Z

clang/www/cxx_status.html

@@ -239,7 +239,7 @@ <h2 id="cxx26">C++2c implementation status</h2>
 <tr>
  <td>Remove Deprecated Array Comparisons from C++26</td>
  <td><a href="https://wg21.link/P2865R6">P2865R6</a></td>
-  <td class="none" align="center">No</td>
+  <td class="none" align="center">Clang 20</td>


Suggested change

<td class="none" align="center">Clang 20</td>

<td class="unreleased" align="center">Clang 20</td>

Done, thanks

erichkeane

This seems fine to me. Please give folks ~24 hrs to chime in before merging.

[Clang] Warning as error Array Comparisons from C++26

30eb50c

Starting from C++26 the array comparison warning should converted to an error.

AmrDeveloper requested review from cor3ntin, AaronBallman, shafik and erichkeane December 5, 2024 21:28

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

erichkeane reviewed Dec 5, 2024

View reviewed changes

zwuis reviewed Dec 6, 2024

View reviewed changes

AmrDeveloper added 2 commits December 6, 2024 18:39

Modify release note and CXX status

f91c1a3

Update C++26 Diagnostic message

c3b15ca

erichkeane approved these changes Dec 6, 2024

View reviewed changes

AmrDeveloper added 3 commits December 6, 2024 19:29

Merge branch 'llvm:main' into cpp26_dep_array_compare_werr

a36a3c4

Fix the name of verify prefix

14624e0

Disable checking for array-comparison C++26 in clang tidy check

62b4192

llvmbot added clang-tools-extra clang-tidy labels Dec 6, 2024

cor3ntin approved these changes Dec 7, 2024

View reviewed changes

Fix typo in diagnostic message

5280439

AmrDeveloper merged commit 3f458cd into llvm:main Dec 7, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Warning as error Array Comparisons from C++26 #118872

[Clang] Warning as error Array Comparisons from C++26 #118872

Uh oh!

AmrDeveloper commented Dec 5, 2024

Uh oh!

llvmbot commented Dec 5, 2024 •

edited

Loading

Uh oh!

erichkeane Dec 5, 2024

Uh oh!

AmrDeveloper Dec 6, 2024

Uh oh!

erichkeane Dec 5, 2024

Uh oh!

AmrDeveloper Dec 6, 2024

Uh oh!

zwuis Dec 7, 2024

Uh oh!

AmrDeveloper Dec 7, 2024

Uh oh!

cor3ntin Dec 7, 2024

Uh oh!

zwuis Dec 6, 2024

Uh oh!

AmrDeveloper Dec 6, 2024

Uh oh!

erichkeane left a comment

Uh oh!

Uh oh!

Uh oh!

	starting from C++26, this warn is error by default.
	starting from C++26, this warning is enabled as an error by default.

	def err_access_decl : Error<
	"ISO C++11 does not allow access declarations; "
	"use using declarations instead">;

	<td class="none" align="center">Clang 20</td>
	<td class="unreleased" align="center">Clang 20</td>

[Clang] Warning as error Array Comparisons from C++26 #118872

[Clang] Warning as error Array Comparisons from C++26 #118872

Uh oh!

Conversation

AmrDeveloper commented Dec 5, 2024

Uh oh!

llvmbot commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Dec 5, 2024 •

edited

Loading