Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,7 @@ def GNUStatementExpressionFromMacroExpansion :
def GNUStatementExpression : DiagGroup<"gnu-statement-expression",
[GNUStatementExpressionFromMacroExpansion]>;
def StringConcatation : DiagGroup<"string-concatenation">;
def ArrayCompare : DiagGroup<"array-compare">;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks

def StringCompare : DiagGroup<"string-compare">;
def StringPlusInt : DiagGroup<"string-plus-int">;
def StringPlusChar : DiagGroup<"string-plus-char">;
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -10264,6 +10264,10 @@ def warn_depr_array_comparison : Warning<
"to compare array addresses, use unary '+' to decay operands to pointers">,
InGroup<DeprecatedArrayCompare>;

def warn_array_comparison : Warning<
"comparison between two arrays compare their addresses and will be deprecated in c++20; "
"to compare array addresses, use unary '+' to decay operands to pointers">;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this lose its group? It still needs one. Also, I thought someone had suggested doing it inline? Diag message seems acceptable enough to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i misunderstood inlining diagnostic here, can you give me example about how inline it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He means to inline the diagnostic group, so instead of InGroup<SomeName> you can do: InGroup<DiagnosticGroup<"Group name">>

(I think you might have to grep here for proper spelling).

Copy link
Member Author

@AmrDeveloper AmrDeveloper Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool i found InGroup<DiagGroup<....>> thanks, for release note i think array-comparion i think it should be in New Compiler Flags section


def warn_stringcompare : Warning<
"result of comparison against %select{a string literal|@encode}0 is "
"unspecified (use an explicit string comparison function instead)">,
Expand Down
15 changes: 11 additions & 4 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11854,14 +11854,21 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
AlwaysEqual, // std::strong_ordering::equal from operator<=>
};

// C++1a [array.comp]:
// Equality and relational comparisons ([expr.eq], [expr.rel]) between two
// operands of array type.
// C++2a [depr.array.comp]:
// Equality and relational comparisons ([expr.eq], [expr.rel]) between two
// operands of array type are deprecated.
if (S.getLangOpts().CPlusPlus20 && LHSStripped->getType()->isArrayType() &&
if (S.getLangOpts().CPlusPlus && LHSStripped->getType()->isArrayType() &&
RHSStripped->getType()->isArrayType()) {
S.Diag(Loc, diag::warn_depr_array_comparison)
<< LHS->getSourceRange() << RHS->getSourceRange()
<< LHSStripped->getType() << RHSStripped->getType();
auto IsDeprArrayComparionIgnored =
S.getDiagnostics().isIgnored(diag::warn_depr_array_comparison, Loc);
auto DiagID = !S.getLangOpts().CPlusPlus20 || IsDeprArrayComparionIgnored
? diag::warn_array_comparison
: diag::warn_depr_array_comparison;
S.Diag(Loc, DiagID) << LHS->getSourceRange() << RHS->getSourceRange()
<< LHSStripped->getType() << RHSStripped->getType();
// Carry on to produce the tautological comparison warning, if this
// expression is potentially-evaluated, we can resolve the array to a
// non-weak declaration, and so on.
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Analysis/reference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ namespace PR13440 {
S s = { a };
S2 s2 = { a };

if (s.x != a) return;
if (s2.x != a) return;
if (s.x != a) return; // expected-warning {{comparison between two arrays}}
if (s2.x != a) return; // expected-warning {{comparison between two arrays}}

a[0] = 42;
clang_analyzer_eval(s.x[0] == 42); // expected-warning{{TRUE}}
Expand Down
10 changes: 5 additions & 5 deletions clang/test/Sema/warn-stringcompare.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cxx %s

#define DELIM "/"
#define DOT "."
Expand All @@ -15,15 +15,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;
return; // cxx-warning@-1 {{comparison between two arrays}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to overlap w/ warn_stringcompare we probably want to do some special handling here since string literals are a special case of arrays and they have their own comparison functions.

We can do this as part of the C++26 feature implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh, thats a great concern! I think you're right, but I think as well that it is ok to do in the followup

if (DELIM == "/") // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
return;
return; // cxx-warning@-1 {{comparison between two arrays}}
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;
return; // cxx-warning@-1 {{comparison between two arrays}}
if (DELIM == DOT) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}}
return;
return; // cxx-warning@-1 {{comparison between two arrays}}
}
12 changes: 7 additions & 5 deletions clang/test/SemaCXX/deprecated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,19 @@ namespace ArithConv {

namespace ArrayComp {
int arr1[3], arr2[4];
bool b1 = arr1 == arr2; // expected-warning {{array comparison always evaluates to false}} cxx20-warning {{comparison between two arrays is deprecated}}
bool b2 = arr1 < arr2; // expected-warning {{array comparison always evaluates to a constant}} cxx20-warning {{comparison between two arrays is deprecated}}
bool b1 = arr1 == arr2; // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
// expected-warning@-1 {{array comparison always evaluates to false}}
bool b2 = arr1 < arr2; // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
// expected-warning@-1 {{array comparison always evaluates to a constant}}
__attribute__((weak)) int arr3[3];
bool b3 = arr1 == arr3; // cxx20-warning {{comparison between two arrays is deprecated}}
bool b4 = arr1 < arr3; // cxx20-warning {{comparison between two arrays is deprecated}}
bool b3 = arr1 == arr3; // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
bool b4 = arr1 < arr3; // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
#if __cplusplus > 201703L
bool b5 = arr1 <=> arr2; // cxx20-error {{invalid operands}}
#endif

int (&f())[3];
bool b6 = arr1 == f(); // cxx20-warning {{comparison between two arrays is deprecated}}
bool b6 = arr1 == f(); // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
bool b7 = arr1 == +f();
}

Expand Down
21 changes: 21 additions & 0 deletions clang/test/SemaCXX/warn-array-comparion.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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

typedef struct {
char str[16];
int id[16];
} Object;

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;
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 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}}
}
6 changes: 3 additions & 3 deletions clang/test/SemaCXX/warn-self-comparisons.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
// expected-no-diagnostics
// 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

void f(int (&array1)[2], int (&array2)[2]) {
if (array1 == array2) { } // no warning
if (array1 == array2) { } // not-cxx20-warning {{comparison between two arrays compare their addresses}} cxx20-warning {{comparison between two arrays is deprecated}}
}
Loading