[clang] [clang] Fix a typo in clangs README (PR #114410)
PhilippRados wrote: @jroelofs can you merge this? https://github.com/llvm/llvm-project/pull/114410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Diagnose for implicit boolean conversions in assignments (PR #114687)
https://github.com/PhilippRados created
https://github.com/llvm/llvm-project/pull/114687
This is an unfinished implementation of #33528. I have a couple of questions
since I'm new to the codebase:
This warning should be issued for both C and C++. For C++ it is pretty
straightforward since I can check this when calling
`PerformImplicitConversion`. However this function isn't called in C and right
now I issue the warning for C in `Sema::ImpCastExprToType`, but I don't know if
this function also might be called in C++ resulting in the same warning issued
twice.
Ideally I would just have to issue this once for both languages but I can't
seem to find the correct `ImplicitCast` function that gets invoked for both
langs.
I also found another already existing warning when running the tests
(warn_condition_is_assignment) that is similar to this warning but only gets
issued on assignments in conditional-expressions, as opposed to all assignment
expressions. In C++ this results in the new and old warning both being
displayed even though they both suggest the same thing.
How should this be handled? Should I manually check if the broader warning has
already been issued and then remove it or maybe just keep the more general
warning as they mean the same thing (-Wparentheses)?
Any feedback is appreciated!
>From b8244b346c7a97c2eb35ccfea23675b8df4f046e Mon Sep 17 00:00:00 2001
From: PhilippR
Date: Sat, 2 Nov 2024 18:51:26 +0100
Subject: [PATCH] [clang] adding diagnose for impl-cast (draft) This is an
unfinished implementation of #33528
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 ++
clang/include/clang/Sema/Sema.h | 2 ++
clang/lib/Sema/Sema.cpp | 14 ++
clang/lib/Sema/SemaExprCXX.cpp | 3 +++
4 files changed, 21 insertions(+)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d697e6d61afa9a..51879c359b3060 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8477,6 +8477,8 @@ def err_incomplete_object_call : Error<
def warn_condition_is_assignment : Warning<"using the result of an "
"assignment as a condition without parentheses">,
InGroup;
+def warn_parens_bool_assign : Warning<"suggest parentheses around assignment
used as truth value">,
+InGroup;
def warn_free_nonheap_object
: Warning<"attempt to call %0 on non-heap %select{object %2|object: block
expression|object: lambda-to-function-pointer conversion}1">,
InGroup;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 93d98e1cbb9c81..30767b11998bb7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7232,6 +7232,8 @@ class Sema final : public SemaBase {
/// being used as a boolean condition, warn if it's an assignment.
void DiagnoseAssignmentAsCondition(Expr *E);
+ void DiagnoseImplicitCastBoolAssignment(Expr *E, QualType ToType);
+
/// Redundant parentheses over an equality comparison can indicate
/// that the user intended an assignment used as condition.
void DiagnoseEqualityWithExtraParens(ParenExpr *ParenE);
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 2b51765e80864a..cc9cb173c01480 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -687,6 +687,18 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind,
const Expr *E) {
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+void Sema::DiagnoseImplicitCastBoolAssignment(Expr* E,QualType Ty) {
+ if (Ty->isBooleanType()) {
+if (BinaryOperator* Op = dyn_cast(E)) {
+ // should only be issued for regular assignment `=`, not for e.g `+=`
+ if (Op->getOpcode() == BO_Assign) {
+SourceLocation Loc = Op->getOperatorLoc();
+Diag(Loc,diag::warn_parens_bool_assign) << E->getSourceRange();
+ }
+}
+ }
+}
+
/// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
/// If there is already an implicit cast, merge into the existing one.
/// The result is of the given category.
@@ -761,6 +773,8 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
}
}
+ DiagnoseImplicitCastBoolAssignment(E,Ty);
+
if (ImplicitCastExpr *ImpCast = dyn_cast(E)) {
if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) {
ImpCast->setType(Ty);
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 50c1b24fce6da7..14ca6a743049e4 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/CharUnits.h"
#include "clang/AST/DeclObjC.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
@@ -4392,6 +4393,8 @@ Sema::PerformImp
[clang] [clang] Diagnose for implicit boolean conversions in assignments (PR #114687)
https://github.com/PhilippRados edited https://github.com/llvm/llvm-project/pull/114687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
PhilippRados wrote: @Endilll Do you have some advice regarding this implementation? Or can you otherwise maybe forward this to someone who can take a look. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix a typo in clangs README (PR #114410)
https://github.com/PhilippRados created https://github.com/llvm/llvm-project/pull/114410 changed "disucss" to "discuss" >From 0932d191b4b260a0b6449124d4fd3735c3fc3291 Mon Sep 17 00:00:00 2001 From: PhilippR Date: Thu, 31 Oct 2024 15:30:10 +0100 Subject: [PATCH] [clang] Fix a typo in clangs README changed "disucss" to "discuss" --- clang/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/README.md b/clang/README.md index b98182d8a3f684..a5e2c9f12e6ac5 100644 --- a/clang/README.md +++ b/clang/README.md @@ -16,7 +16,7 @@ If you're interested in more (including how to build Clang) it is best to read t * Information on the LLVM project:http://llvm.org/ -* If you have questions or comments about Clang, a great place to disucss them is on the Clang forums: +* If you have questions or comments about Clang, a great place to discuss them is on the Clang forums: [Clang Frontend - LLVM Discussion Forums](https://discourse.llvm.org/c/clang/) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
https://github.com/PhilippRados created
https://github.com/llvm/llvm-project/pull/115234
This is a fix for #33528 as I messed up my other PR with unsynced changes.
A couple of things make this less straightforward as initially thought, which
is why I would like some feedback as to how these things should be handled.
This warning should get issued for both C and C++ (I'm not sure about the other
languages). For C the tests pass. For C++ some fail because another similar
warning is expected `warn_condition_is_assignment`. That warning can be covered
by this new warning but it would be better to issue the existing since it is
more specific in cases of condition.
The problem is that in C, conditions do not emit an implicit-cast to booleans
while in C++ they do. So the original warning is needed for C, but would
result in double warning in C++. This requires special handling.
In the code I have left `NOTE:` comments whenever I was unsure about the
implementation of something, which might need some clearing up since this is my
first proper contribution.
>From e819360e6510f12f19b7df9d4eb22c0943776440 Mon Sep 17 00:00:00 2001
From: PhilippR
Date: Thu, 7 Nov 2024 00:06:24 +0100
Subject: [PATCH] [clang] Added warn-assignment-bool-context
This warning is issued if an assignment is used in a context
where a boolean result is required.
---
.../clang/Basic/DiagnosticSemaKinds.td| 3 ++
clang/include/clang/Sema/Sema.h | 2 +
clang/lib/Sema/Sema.cpp | 53 +++
clang/lib/Sema/SemaExpr.cpp | 6 ++-
.../test/Sema/warn-assignment-bool-context.c | 35
.../SemaCXX/warn-assignment-bool-context.cpp | 45
6 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Sema/warn-assignment-bool-context.c
create mode 100644 clang/test/SemaCXX/warn-assignment-bool-context.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ae3e243bdc58bd..4ba4327bde5cfb 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8477,6 +8477,9 @@ def err_incomplete_object_call : Error<
def warn_condition_is_assignment : Warning<"using the result of an "
"assignment as a condition without parentheses">,
InGroup;
+def warn_assignment_bool_context : Warning<"suggest parentheses around
assignment used as truth value">,
+ InGroup;
+
def warn_free_nonheap_object
: Warning<"attempt to call %0 on non-heap %select{object %2|object: block
expression|object: lambda-to-function-pointer conversion}1">,
InGroup;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 93d98e1cbb9c81..6bf73956d3f944 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7232,6 +7232,8 @@ class Sema final : public SemaBase {
/// being used as a boolean condition, warn if it's an assignment.
void DiagnoseAssignmentAsCondition(Expr *E);
+ void DiagnoseAssignmentBoolContext(Expr *E, QualType ToType);
+
/// Redundant parentheses over an equality comparison can indicate
/// that the user intended an assignment used as condition.
void DiagnoseEqualityWithExtraParens(ParenExpr *ParenE);
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 2b51765e80864a..a8e4657ac0b6ec 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -687,6 +687,48 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind,
const Expr *E) {
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+ // Use copy to not alter original expression.
+ Expr *ECopy = E;
+
+ if (Ty->isBooleanType()) {
+// `bool(x=0)` and if (x=0){} emit:
+// - ImplicitCastExpr bool IntegralToBoolean
+// -- ImplicitCastExpr int LValueToRValue
+// --- Assignment ...
+// But should still emit this warning (at least gcc does), even if
bool-cast
+// is not directly followed by assignment.
+// NOTE: Is this robust enough or can there be other semantic expression
+// until the assignment?
+while (ImplicitCastExpr *ICE = dyn_cast(ECopy)) {
+ // If there is another implicit cast to bool then this warning would have
+ // been already emitted.
+ if (ICE->getType()->isBooleanType())
+return;
+ ECopy = ICE->getSubExpr();
+}
+
+if (BinaryOperator *Op = dyn_cast(ECopy)) {
+ // Should only be issued for regular assignment `=`,
+ // not for compound-assign like `+=`.
+ // NOTE: Might make sense to emit for all assignments even if gcc
+ // only does for regular assignment.
+ if (Op->getOpcode() == BO_Assign) {
+SourceLocation Loc = Op->getOperatorLoc();
+Diag(Loc, diag::warn_assignment_bool_context)
+<< ECopy->getSourceRange();
+
+Sou
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -761,6 +803,17 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty, } } + // FIXME: Doesn't include C89, so this warning isn't emitted when passing + // `std=c89`. + auto isC = getLangOpts().C99 || getLangOpts().C11 || getLangOpts().C17 || PhilippRados wrote: > Why doesn't this make sense in ObjC? I thought that this was some kind of common pattern in ObjC which is why the existing `DiagnoseAssignmentAsCondition()` also special cases this. I think I might have also seen some tests that check this, but I could be wrong. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -20287,7 +20287,11 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr
*ParenE) {
ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
bool IsConstexpr) {
- DiagnoseAssignmentAsCondition(E);
+ // This warning is already covered by `warn_assignment_bool_context` in C++.
+ // NOTE: Ideally both warnings would be combined
+ if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
PhilippRados wrote:
> Also, rather than this, I'd prefer we are smarter about the new diagnostic to
> not overlap in the condition case (suppressing THAT instead of htis here).
Yes, I agree. Is there a way to check if a certain other diagnostic has been
issued? Or do I need to pass a flag like `isCond` to `Sema::ImpCastExprToType`
to be able to check if the outer expression is a condition? The latter would
require a lot of function changes which would be a little tedious which is why
I'm asking beforehand.
https://github.com/llvm/llvm-project/pull/115234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
https://github.com/PhilippRados edited https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
https://github.com/PhilippRados ready_for_review https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
PhilippRados wrote: No problem, I know everybody is busy. > The first step is to get the PR out of draft state. Do you just mean to mark it "read for review" or to apply changes so that it can be merged? Because the current implementation is not ready as I still have some questions as to how to implement things in order to pass the failing tests. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
PhilippRados wrote: Yup ok. I thought that was what draft was for to check on not-finished implementations but I just changed it back for review. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -761,6 +803,17 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
}
}
+ // FIXME: Doesn't include C89, so this warning isn't emitted when passing
+ // `std=c89`.
+ auto isC = getLangOpts().C99 || getLangOpts().C11 || getLangOpts().C17 ||
PhilippRados wrote:
It does make sense for C++. It should be issued for both C and C++ (I'm not
proficient in OpenMP/Cuda so I can't really talk for those).
I wanted to clarify that with the `if (getLangOpts().CPlusPlus || isC) &&
!getLangOpts().ObjC) {...}` line, that this would actually just affect C and
C++ and none of the other languages.
Of course I could also just do `!isCuda && !isOpenMP && !isObjC` so that just C
and C++ remain but that seems a little verbose.
https://github.com/llvm/llvm-project/pull/115234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -20287,7 +20287,11 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr
*ParenE) {
ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
bool IsConstexpr) {
- DiagnoseAssignmentAsCondition(E);
+ // This warning is already covered by `warn_assignment_bool_context` in C++.
+ // NOTE: Ideally both warnings would be combined
+ if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
PhilippRados wrote:
Is it not possible to store that info in the expr and then just check it later
on? Or in the context that gets passed around?
https://github.com/llvm/llvm-project/pull/115234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -8477,6 +8477,9 @@ def err_incomplete_object_call : Error< def warn_condition_is_assignment : Warning<"using the result of an " "assignment as a condition without parentheses">, InGroup; +def warn_assignment_bool_context : Warning<"suggest parentheses around assignment used as truth value">, PhilippRados wrote: > Also, emitting note_condition_assign_silence might be sensible. Yes that is already being submitted in `DiagnoseAssignmentBoolContext()` https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -687,6 +687,48 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind,
const Expr *E) {
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+ // Use copy to not alter original expression.
+ Expr *ECopy = E;
+
+ if (Ty->isBooleanType()) {
+// `bool(x=0)` and if (x=0){} emit:
+// - ImplicitCastExpr bool IntegralToBoolean
+// -- ImplicitCastExpr int LValueToRValue
+// --- Assignment ...
+// But should still emit this warning (at least gcc does), even if
bool-cast
PhilippRados wrote:
> ALSO ALSO, if this ends up in C++ anywhere, you'll have some user-defined
> operators to mess with as well.
How can I check for that? Do you have a more robust approach?
https://github.com/llvm/llvm-project/pull/115234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -687,6 +687,48 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind,
const Expr *E) {
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+ // Use copy to not alter original expression.
+ Expr *ECopy = E;
+
+ if (Ty->isBooleanType()) {
+// `bool(x=0)` and if (x=0){} emit:
+// - ImplicitCastExpr bool IntegralToBoolean
+// -- ImplicitCastExpr int LValueToRValue
+// --- Assignment ...
+// But should still emit this warning (at least gcc does), even if
bool-cast
PhilippRados wrote:
> Not sure what the question is here, can you clarify?
Basically just asking if `ImplicitCastExpr` is the only valid expression that
can be in between the outer cast and the assignment expression or if there is
another implicit expression that would need to be matched here.
https://github.com/llvm/llvm-project/pull/115234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
@@ -8477,6 +8477,9 @@ def err_incomplete_object_call : Error< def warn_condition_is_assignment : Warning<"using the result of an " "assignment as a condition without parentheses">, InGroup; +def warn_assignment_bool_context : Warning<"suggest parentheses around assignment used as truth value">, PhilippRados wrote: > The text in warn_condition_is_idiomatic_assignment is pretty close to what we > want, right? Yes, but maybe just having the more generic `warn_assignment_bool_context` with a message like "using the result of an assignment as a truth value without parentheses" which can be emitted during conditions as well. Since this implicit cast can also happen outside of conditions. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
https://github.com/PhilippRados updated
https://github.com/llvm/llvm-project/pull/115234
>From e819360e6510f12f19b7df9d4eb22c0943776440 Mon Sep 17 00:00:00 2001
From: PhilippR
Date: Thu, 7 Nov 2024 00:06:24 +0100
Subject: [PATCH 1/5] [clang] Added warn-assignment-bool-context
This warning is issued if an assignment is used in a context
where a boolean result is required.
---
.../clang/Basic/DiagnosticSemaKinds.td| 3 ++
clang/include/clang/Sema/Sema.h | 2 +
clang/lib/Sema/Sema.cpp | 53 +++
clang/lib/Sema/SemaExpr.cpp | 6 ++-
.../test/Sema/warn-assignment-bool-context.c | 35
.../SemaCXX/warn-assignment-bool-context.cpp | 45
6 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Sema/warn-assignment-bool-context.c
create mode 100644 clang/test/SemaCXX/warn-assignment-bool-context.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ae3e243bdc58b..4ba4327bde5cf 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8477,6 +8477,9 @@ def err_incomplete_object_call : Error<
def warn_condition_is_assignment : Warning<"using the result of an "
"assignment as a condition without parentheses">,
InGroup;
+def warn_assignment_bool_context : Warning<"suggest parentheses around
assignment used as truth value">,
+ InGroup;
+
def warn_free_nonheap_object
: Warning<"attempt to call %0 on non-heap %select{object %2|object: block
expression|object: lambda-to-function-pointer conversion}1">,
InGroup;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 93d98e1cbb9c8..6bf73956d3f94 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7232,6 +7232,8 @@ class Sema final : public SemaBase {
/// being used as a boolean condition, warn if it's an assignment.
void DiagnoseAssignmentAsCondition(Expr *E);
+ void DiagnoseAssignmentBoolContext(Expr *E, QualType ToType);
+
/// Redundant parentheses over an equality comparison can indicate
/// that the user intended an assignment used as condition.
void DiagnoseEqualityWithExtraParens(ParenExpr *ParenE);
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 2b51765e80864..a8e4657ac0b6e 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -687,6 +687,48 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind,
const Expr *E) {
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+ // Use copy to not alter original expression.
+ Expr *ECopy = E;
+
+ if (Ty->isBooleanType()) {
+// `bool(x=0)` and if (x=0){} emit:
+// - ImplicitCastExpr bool IntegralToBoolean
+// -- ImplicitCastExpr int LValueToRValue
+// --- Assignment ...
+// But should still emit this warning (at least gcc does), even if
bool-cast
+// is not directly followed by assignment.
+// NOTE: Is this robust enough or can there be other semantic expression
+// until the assignment?
+while (ImplicitCastExpr *ICE = dyn_cast(ECopy)) {
+ // If there is another implicit cast to bool then this warning would have
+ // been already emitted.
+ if (ICE->getType()->isBooleanType())
+return;
+ ECopy = ICE->getSubExpr();
+}
+
+if (BinaryOperator *Op = dyn_cast(ECopy)) {
+ // Should only be issued for regular assignment `=`,
+ // not for compound-assign like `+=`.
+ // NOTE: Might make sense to emit for all assignments even if gcc
+ // only does for regular assignment.
+ if (Op->getOpcode() == BO_Assign) {
+SourceLocation Loc = Op->getOperatorLoc();
+Diag(Loc, diag::warn_assignment_bool_context)
+<< ECopy->getSourceRange();
+
+SourceLocation Open = ECopy->getBeginLoc();
+SourceLocation Close =
+getLocForEndOfToken(ECopy->getSourceRange().getEnd());
+Diag(Loc, diag::note_condition_assign_silence)
+<< FixItHint::CreateInsertion(Open, "(")
+<< FixItHint::CreateInsertion(Close, ")");
+ }
+}
+ }
+}
+
/// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
/// If there is already an implicit cast, merge into the existing one.
/// The result is of the given category.
@@ -761,6 +803,17 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
}
}
+ // FIXME: Doesn't include C89, so this warning isn't emitted when passing
+ // `std=c89`.
+ auto isC = getLangOpts().C99 || getLangOpts().C11 || getLangOpts().C17 ||
+ getLangOpts().C23;
+ // Do not emit this warning for Objective-C, since it's a common idiom.
+ // NOTE: Are there other languages that this could affect besides C and C++?
+ // Ideally
[clang] [clang] Added warn-assignment-bool-context (PR #115234)
PhilippRados wrote: I applied the mentioned changes from above and merged the two diagnostics into a single one. I also added the field IsInsideCondition to the `Expr` class and everything works. However I’m not sure whether or not this single diagnostic warrants the extra field in the class. I also saw that enum/boolean fields in `Expr` are represented using the `ExprBits` bitfield so if this were a valid approach then adding it to the bitfield would probably be more space efficient. https://github.com/llvm/llvm-project/pull/115234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
