You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
diagnose comparisons between two arrays (P2865), fix for [GH #370] - #387
A comparison between two arrays with ==, !=, <, <=, > or >= was accepted silently in every mode. It is deprecated in C++20 and not allowed in C++26 (P2865).
process_eq_operator and process_rel_operator now call check_for_array_comparison before the array-to-pointer conversion (the same place <=> already rejects two arrays). If both operands have array type:
C++26: error, "comparison between two arrays is not allowed".
C++20 and C++23: warning, "comparison between two arrays is deprecated".
When emulating GCC before 15 or Clang before 20, C++26 only gets the warning. GCC 14 warns and GCC 15 errors with -std=c++26 (checked with the gcc:14 and gcc:15 images); Clang implemented P2865 in Clang 20.
A comparison between an array and a pointer is not affected.
Summary by CodeRabbit
Bug Fixes
Equality and relational comparisons between two arrays are now diagnosed. They produce deprecation warnings in C++20 and C++23; before C++20, they produce a remark, or a warning in Clang 20+ compatibility mode.
In C++26, these comparisons produce errors with newer GCC and Clang compatibility settings; otherwise, they produce deprecation warnings.
Comparisons involving class operands converted to arrays are diagnosed as well.
Comparisons between an array and a pointer remain allowed.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
Walkthrough
The compiler now diagnoses built-in equality and relational comparisons between two arrays. The diagnostic depends on the C++ standard and, in C++26 mode, the emulated compiler version. Tests also cover array-to-pointer comparisons.
Changes: Array comparison diagnostics
Layer / File(s)
Summary
Diagnostic check and operator integration src/error_msg.txt, src/expr.c
The built-in comparison paths check operands before transformations. They report remarks, warnings, or errors according to the C++ standard and emulated compiler versions.
Standard and compiler-version coverage tests/tests/changes/GH370*.sft.cpp, tests/tests/changes/.GH370*.rto/*, src/pending_changes/gh-370-array-comparisons.txt
Tests cover array-to-array and array-to-pointer comparisons across language modes and compiler-version configurations. The pending-change note describes the diagnostics.
sequenceDiagram
participant BuiltinComparisonPath
participant ArrayComparisonCheck
participant CompilerDiagnostics
BuiltinComparisonPath->>ArrayComparisonCheck: Check original operands
ArrayComparisonCheck->>CompilerDiagnostics: Report remark, warning, or error
Loading
Suggested labels:bug
Suggested reviewers:darkarc
Merge Risk: Minimal · up to 8cf11
The change adds standard-aware array-comparison diagnostics. No actionable merge-blocking risk remains in the supplied evidence.
Architecture risk: Low · up to 0d4ea
The change affects 2 systems.
Changed systems:tests, src
Architecture concerns
No architecture-level concerns identified.
Review details
Systems and components
observed — tests (service) was modified; 18 changed files map to changed impact.
observed — src (service) was modified; 3 changed files map to changed impact.
Before / after behavior
observed — Modified behavior in src/error_msg.txt: Added diagnostic entries for deprecated and disallowed comparisons between two arrays.
observed — Modified behavior in src/expr.c: Added check_for_array_comparison, which checks for two array-typed operands in C++20 mode and diagnoses the comparison. In C++26 mode, it emits an error except on GCC 15+ and Clang 20+, where it emits a deprecation warning.
observed — Modified behavior in src/expr.c: The built-in relational-operator path now checks the original operands for an array comparison before operand transformations.
observed — Modified behavior in src/expr.c: The built-in equality-operator path now checks the original operands for an array comparison before operand transformations.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly identifies the main change: diagnosing comparisons between two arrays under P2865. It also references the related issue.
Linked Issues check
Issue #370 requires P2865R6 handling of array comparisons: deprecate them in C++20 and disallow them in C++26. The change adds check_for_array_comparison calls in both non-operator-function comparis…
Out of Scope Changes check
The changes in src/expr.c, src/error_msg.txt, and the pending-change note gh-370-array-comparisons.txt implement the array-comparison diagnostics for #370. The test changes cover the same diagno…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check
Explanation
Issue #370 requires P2865R6 handling of array comparisons: deprecate them in C++20 and disallow them in C++26. The change adds check_for_array_comparison calls in both non-operator-function comparison paths, before operand transformations, so the array check runs before array-to-pointer conversion. The check covers ==, !=, <, <=, > and >=. The summary reports a deprecation warning in C++20 and C++23, and an error in C++26 when the emulated GCC is version 15 or later or the emulated Clang is version 20 or later. Otherwise C++26 gives the deprecation warning. The new and updated tests (GH370.sft.cpp, GH370_a.sft.cpp, .GH370.rto and .GH370_a.rto expectations) cover the language modes, emulated compiler versions, class operands converted to arrays, and the qualification-conversion case. Array-to-pointer comparisons stay allowed. The requirement is implemented and tested. The summary wording for the C++26 error condition uses 'and' between the GCC and Clang checks. The PR description and the test expectations (GCC 150000 and Clang 200000 error; Clang 190000 warns) indicate 'or'. This wording difference does not change the result.
Full details: Out of Scope Changes check
Explanation
The changes in src/expr.c, src/error_msg.txt, and the pending-change note gh-370-array-comparisons.txt implement the array-comparison diagnostics for #370. The test changes cover the same diagnostics in the changes/ and imported Clang SemaCXX expectations. The imported Clang expectation updates record the new diagnostics for existing array comparisons, so they follow from the feature and are not unrelated edits. The pre-C++20 remark and the Clang 20+ warning under emulation belong to the same array-comparison diagnostic family. The current PR description does not mention the pre-C++20 remark, but the reviewer agreed to it and it does not affect unrelated behavior. No unrelated change appears in the reviewable summary. The ignored file tests/expectations/edg_x86_64/imported.log matches the !**/*.log exclusion rule and was not inspected. Its exclusion does not establish an out-of-scope change.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (38 skipped: 37 unsupported, 1 too large.)
Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
LanguageTool
src/error_msg.txt
LanguageTool checks are incomplete because the per-file request limit of 5 was reached. Remaining text was skipped; findings from completed checks are retained.
Comment @coderabbitai help to get the list of available commands.
@nv-cmeerw fixed in ee685d1 (rebased onto main): class operands converted to arrays by a conversion function (c == c, c < c, c == arr) are now diagnosed too, matching GCC 15 and Clang 20, and changes/GH370 covers them. Could you please take another look?
@daveedvdv-nvidia@nv-cmeerw Thanks, good point. I checked this on Godbolt (x86-64), with no extra warning options:
Clang 9 to 19, -std=c++17: no array-comparison warning. The warning in the link is -Wtautological-compare ("array comparison always evaluates to false"), which only fires when the result is known, e.g. for two distinct global arrays. Comparing two array references gives nothing.
Clang 20 and later warn on every array comparison in C++03 to C++17 by default (-Warray-compare: "comparison between two arrays compare their addresses and will be deprecated in c++20").
GCC 12 and later warn before C++20 only with -Wall (-Warray-compare). GCC 11 and earlier do not warn at all.
So my suggestion: before C++20, issue a remark, the way a comma operator in a subscript is handled before C++20, and a warning when emulating Clang 20 or later. It would get its own message, e.g. "comparison between two arrays compares their addresses", since it is not deprecated yet in those modes. C++20 and later stay as in this PR.
Does that sound OK, or would you prefer a warning in all modes before C++20?
There still appears to be an issue with cv-qualifications:
struct C { using A = int[2]; operator A &() const; };
const int a[2] = {};
int b[2] = {};
bool f(C c) { return c == a; } // not diagnosed yet
bool g(C c) { return b == a; } // diagnosed
c == a with a const array is now diagnosed. The check looks through the implicit qualification conversion added by overload resolution, but not through a cast in the source, so (const int *)b == a is still not diagnosed, matching GCC and Clang.
Before C++20 there is now a remark ("comparison between two arrays compares their addresses"), and a warning when emulating Clang 20 or later.
changes/GH370 and GH370_a cover both. Could you please take another look?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
nv-cmeerw
left a comment
daveedvdv-nvidia
left a comment
Fixes #370.
A comparison between two arrays with
==,!=,<,<=,>or>=was accepted silently in every mode. It is deprecated in C++20 and not allowed in C++26 (P2865).process_eq_operatorandprocess_rel_operatornow callcheck_for_array_comparisonbefore the array-to-pointer conversion (the same place<=>already rejects two arrays). If both operands have array type:-std=c++26(checked with the gcc:14 and gcc:15 images); Clang implemented P2865 in Clang 20.A comparison between an array and a pointer is not affected.
Summary by CodeRabbit
Bug Fixes