[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Shafik Yaghmour via cfe-commits


@@ -36,7 +36,7 @@ namespace InExpr {
 
 // These are valid expressions.
 foo(0);
+foo(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have 
their mathematical meaning}}
 foo(false);

shafik wrote:

It is a shame we don't catch this one but neither does gcc

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Vlad Serebrennikov via cfe-commits


@@ -85,7 +85,7 @@ void f(int x, int y, int z) {
   if ((ayyhttps://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Erich Keane via cfe-commits


@@ -85,7 +85,7 @@ void f(int x, int y, int z) {
   if ((ayy` 
or something).  So something like: `comparison 'a>yhttps://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 closed 
https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 updated 
https://github.com/llvm/llvm-project/pull/92200

>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH 1/5] [clang][Sema] Warn consecutive builtin comparisons in an
 expression

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)

Closes #20456
---
 clang/docs/ReleaseNotes.rst  | 3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 
 clang/lib/Sema/SemaExpr.cpp  | 5 +
 clang/test/Sema/parentheses.cpp  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

>From b3cc457efc40a345d4b67c776edd470e35f73de2 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 13:20:06 -0400
Subject: [PATCH 2/5] Update clang/lib/Sema/SemaExpr.cpp

Co-authored-by: Aaron Ballman 
---
 clang/lib/Sema/SemaExpr.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fab34f4fa3e14..b4c54a3da42c0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,9 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
 
-if (const auto *BI = dyn_cast(LHSExpr))
-  if (BI->isComparisonOp())
-Diag(OpLoc, diag::warn_consecutive_comparison);
+if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
+  Diag(OpLoc, diag::warn_consecutive_comparison);
 
 break;
   case BO_EQ:

>From 8df35dcf7b802ce8e280930224575077a08e0541 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 12:53:38 -0500
Subject: [PATCH 3/5] Remove DefaultIgnore + clang-format

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td  | 2 +-
 clang/lib/Sema/SemaExpr.cpp   | 3 ++-
 clang/test/Sema/bool-compare.c| 4 ++--
 clang/test/SemaCXX/bool-compare.cpp   | 4 ++--
 clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp | 2 +-
 clang/test/SemaTemplate/typo-dependent-name.cpp   

[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Aaron Ballman via cfe-commits


@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.

AaronBallman wrote:

```suggestion
- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
  Fixes #GH20456.
```

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman approved this pull request.

LGTM aside from a small nit with the release notes.

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Aaron Ballman via cfe-commits


@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}

AaronBallman wrote:

> I don't have the capacity to further make clang diagnose case 2 but not case 
> 3.
> I added a TODO note at the bottom of the test lines for now.

SGTM!

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-16 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 edited 
https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-16 Thread Youngsuk Kim via cfe-commits


@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}

JOE1994 wrote:

(I'll refer to each of the 3 cases mentioned in the above comment as case1, 
case2, and case 3)

I pushed a commit that adds 2 `no-warning` test lines for case1.
It's unlikely for one to write `((x < y) >= z)` and believe it to have 
mathematical meaning of `(x < y) AND (y >= z)`.

I limited the scope of this PR to diagnosing consecutive "built-in" comparisons 
(as per the PR title).
Thus, the current implementation does not diagnose case 2.

I don't have the capacity to further work on making clang diagnose case 2 but 
not case 3.
I added a TODO note at the bottom of the test lines for now.

FYI, gcc doesn't diagnose any of case1, case2, or case 3.

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-16 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 updated 
https://github.com/llvm/llvm-project/pull/92200

>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH 1/4] [clang][Sema] Warn consecutive builtin comparisons in an
 expression

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)

Closes #20456
---
 clang/docs/ReleaseNotes.rst  | 3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 
 clang/lib/Sema/SemaExpr.cpp  | 5 +
 clang/test/Sema/parentheses.cpp  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

>From b3cc457efc40a345d4b67c776edd470e35f73de2 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 13:20:06 -0400
Subject: [PATCH 2/4] Update clang/lib/Sema/SemaExpr.cpp

Co-authored-by: Aaron Ballman 
---
 clang/lib/Sema/SemaExpr.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fab34f4fa3e14..b4c54a3da42c0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,9 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
 
-if (const auto *BI = dyn_cast(LHSExpr))
-  if (BI->isComparisonOp())
-Diag(OpLoc, diag::warn_consecutive_comparison);
+if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
+  Diag(OpLoc, diag::warn_consecutive_comparison);
 
 break;
   case BO_EQ:

>From 8df35dcf7b802ce8e280930224575077a08e0541 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 12:53:38 -0500
Subject: [PATCH 3/4] Remove DefaultIgnore + clang-format

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td  | 2 +-
 clang/lib/Sema/SemaExpr.cpp   | 3 ++-
 clang/test/Sema/bool-compare.c| 4 ++--
 clang/test/SemaCXX/bool-compare.cpp   | 4 ++--
 clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp | 2 +-
 clang/test/SemaTemplate/typo-dependent-name.cpp   

[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 edited 
https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 edited 
https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 updated 
https://github.com/llvm/llvm-project/pull/92200

>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH 1/3] [clang][Sema] Warn consecutive builtin comparisons in an
 expression

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)

Closes #20456
---
 clang/docs/ReleaseNotes.rst  | 3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 
 clang/lib/Sema/SemaExpr.cpp  | 5 +
 clang/test/Sema/parentheses.cpp  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

>From b3cc457efc40a345d4b67c776edd470e35f73de2 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 13:20:06 -0400
Subject: [PATCH 2/3] Update clang/lib/Sema/SemaExpr.cpp

Co-authored-by: Aaron Ballman 
---
 clang/lib/Sema/SemaExpr.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fab34f4fa3e14..b4c54a3da42c0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,9 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
 
-if (const auto *BI = dyn_cast(LHSExpr))
-  if (BI->isComparisonOp())
-Diag(OpLoc, diag::warn_consecutive_comparison);
+if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
+  Diag(OpLoc, diag::warn_consecutive_comparison);
 
 break;
   case BO_EQ:

>From 8df35dcf7b802ce8e280930224575077a08e0541 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 12:53:38 -0500
Subject: [PATCH 3/3] Remove DefaultIgnore + clang-format

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td  | 2 +-
 clang/lib/Sema/SemaExpr.cpp   | 3 ++-
 clang/test/Sema/bool-compare.c| 4 ++--
 clang/test/SemaCXX/bool-compare.cpp   | 4 ++--
 clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp | 2 +-
 clang/test/SemaTemplate/typo-dependent-name.cpp   

[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff c5cd049566a795ba5de88dfbb2eb563cad4a9d8a 
b3cc457efc40a345d4b67c776edd470e35f73de2 -- clang/lib/Sema/SemaExpr.cpp 
clang/test/Sema/parentheses.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b4c54a3da4..65c400637a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,7 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
 
-if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
+if (const auto *BI = dyn_cast(LHSExpr);
+BI && BI->isComparisonOp())
   Diag(OpLoc, diag::warn_consecutive_comparison);
 
 break;

``




https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 updated 
https://github.com/llvm/llvm-project/pull/92200

>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH 1/2] [clang][Sema] Warn consecutive builtin comparisons in an
 expression

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)

Closes #20456
---
 clang/docs/ReleaseNotes.rst  | 3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 
 clang/lib/Sema/SemaExpr.cpp  | 5 +
 clang/test/Sema/parentheses.cpp  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

>From b3cc457efc40a345d4b67c776edd470e35f73de2 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Wed, 15 May 2024 13:20:06 -0400
Subject: [PATCH 2/2] Update clang/lib/Sema/SemaExpr.cpp

Co-authored-by: Aaron Ballman 
---
 clang/lib/Sema/SemaExpr.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fab34f4fa3e14..b4c54a3da42c0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,9 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
 
-if (const auto *BI = dyn_cast(LHSExpr))
-  if (BI->isComparisonOp())
-Diag(OpLoc, diag::warn_consecutive_comparison);
+if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
+  Diag(OpLoc, diag::warn_consecutive_comparison);
 
 break;
   case BO_EQ:

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Aaron Ballman via cfe-commits


@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}

AaronBallman wrote:

I'd like an additional test along the lines of:
```
(void)((x < y) >= z);
```
and what about something along these lines?
```
struct S {
  bool operator<(const S &) const;
} s, t;

(void)(s < t >= z);
```
perhaps that should diagnose while this should not?
```
struct S {
  S operator<(const S &) const;
} a, b;

struct T {
  friend bool operator>=(const S &, const T &);
} c;

bool val = (a < b >= c);
```

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Aaron Ballman via cfe-commits


@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+

AaronBallman wrote:

```suggestion
if (const auto *BI = dyn_cast(LHSExpr); BI && 
BI->isComparisonOp())
  Diag(OpLoc, diag::warn_consecutive_comparison);

```

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Youngsuk Kim via cfe-commits

JOE1994 wrote:

> Do we know why GCC elected to go that route? This seems like something that 
> should be enabled by default because the false positive rate should be quite 
> low.

It seems like `gcc` doesn't enable `-Wparentheses` by default, while `clang` 
enables it by default.

I will add a follow-up commit to remove `DefaultIgnore` and update affected 
Clang tests :)

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-15 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> Set the warning to DefaultIgnore, although -Wparentheses is enabled by 
> default 

Do we know why GCC elected to go that route? This seems like something that 
should be enabled by default because the false positive rate should be quite 
low.

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Youngsuk Kim (JOE1994)


Changes

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled (via 
-Wparentheses or -Wall)

Closes #20456

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


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+5) 
- (modified) clang/test/Sema/parentheses.cpp (+7) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

``




https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-14 Thread Youngsuk Kim via cfe-commits

https://github.com/JOE1994 created 
https://github.com/llvm/llvm-project/pull/92200

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled (via 
-Wparentheses or -Wall)

Closes #20456

>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim 
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH] [clang][Sema] Warn consecutive builtin comparisons in an
 expression

Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
  * This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)

Closes #20456
---
 clang/docs/ReleaseNotes.rst  | 3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 
 clang/lib/Sema/SemaExpr.cpp  | 5 +
 clang/test/Sema/parentheses.cpp  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
  };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive 
comparisons like ``x < y < z``.
+  It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup, DefaultIgnore;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   case BO_GT:
 ConvertHalfVec = true;
 ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+if (const auto *BI = dyn_cast(LHSExpr))
+  if (BI->isComparisonOp())
+Diag(OpLoc, diag::warn_consecutive_comparison);
+
 break;
   case BO_EQ:
   case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't 
have their mathematical meaning}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits