vinayakdsci wrote:
Great catch! The usage of template instantiation didn't seem very obvious while
adding the tests, and hence the missed test case.
Thanks a lot for the fix!
https://github.com/llvm/llvm-project/pull/83497
___
cfe-commits mailing list
vinayakdsci wrote:
> Aside from some grammatical changes to the release note, the changes LGTM!
Thank you so much! I have made changes to the Release Notes as you suggested.
Thanks for your review!
> Do you need someone to land this on your behalf once those changes are made?
Yes, I don't ha
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/81127
>From d4df9a7bef0f993d0c092af5f5af61202dc3effe Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Thu, 8 Feb 2024 17:29:44 +0530
Subject: [PATCH] [Clang][Sema]: Allow copy constructor side effects
---
clang/d
vinayakdsci wrote:
@AaronBallman C++17 and later mark all of these constructor calls as
_non_-elidable after ignoring the implicit casts, while C++14 and before mark
the constructor that is assigned by `=` as elidable, and hence the warning.
Should the constructor also be marked as elidable in
vinayakdsci wrote:
@shafik I have made the change in the code as you suggested. If everything
seems alright, could you land this for me?
Thanks a lot!
https://github.com/llvm/llvm-project/pull/83152
___
cfe-commits mailing list
cfe-commits@lists.llvm
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From 3d6100ae6fa291db24f26e2ccbce88293810e168 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From 60620d14509b4d097a56d0b61177dfe9c5a72b63 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
vinayakdsci wrote:
@AaronBallman does this PR look good to go, or should I make any changes and
updates? I would really be grateful for any pointers on how this can be
improved.
Thanks a lot!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-c
vinayakdsci wrote:
@AaronBallman sorry for repeating the same mistake twice, I was unable to
correctly interpret what you wanted to say when you meant removing the lambda
body from the diagnostic.
I have made all the suggested changes and updated the tests. I hope I don't
come across as dumb :
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From 640cbf94879217526e27884beefcbffa404fc082 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
https://github.com/vinayakdsci deleted
https://github.com/llvm/llvm-project/pull/83152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -16538,6 +16538,21 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
}
}
+ // Complain if we are converting a lambda expression to a boolean value
+ if (const auto *MCallExpr = dyn_cast(E)) {
+if (const auto *MRecordDecl = MCallExpr->getRecordDecl();
+
vinayakdsci wrote:
Done! I added the diagnostic to the TableGen file, and passed in only the
Record's source range instead of the whole expression's. Now only the lambda's
initializer is highlighted in the diagnostic. I hope this does it!
https://github.com/llvm/llvm-project/pull/83152
___
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From 613e7c0698f16292bb408be832c7ab3647f17195 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
@@ -16538,6 +16538,24 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
}
}
+ // Complain if we are converting a lambda expression to a boolean value
+ if (const auto *MCallExpr = dyn_cast(E)) {
+if (MCallExpr->getObjectType()->isRecordType()) {
+ if (const
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From c65c6a379db75eb4bf38578106ba2aa4e894e3d8 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/83152
>From 0e9d7e7ed96feb8c32be33ca9b1262bba32ab8b4 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Tue, 27 Feb 2024 18:05:29 +0530
Subject: [PATCH] [Clang][Sema]: Diagnose lambda to bool implicit casts
---
cla
@@ -16538,6 +16538,27 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
}
}
+ // Complain if we are converting a lambda expression to a boolean value
+ if (auto *MCallExpr = dyn_cast(E)) {
+if (MCallExpr->getObjectType()->isRecordType()) {
+ if (auto *MRecor
vinayakdsci wrote:
> There are some related test failures caught by precommit CI that need to be
> addressed.
>
> The issue in dr18xx.cpp looks to be a case where we should replace `bool b =
> ` with `auto b = ` (it doesn't change the testing for what's discussed in
> http://wg21.link/cwg1837
@@ -16538,6 +16538,27 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
}
}
+ // Complain if we are converting a lambda expression to a boolean value
+ if (auto *MCallExpr = dyn_cast(E)) {
+if (MCallExpr->getObjectType()->isRecordType()) {
+ if (auto *MRecor
https://github.com/vinayakdsci created
https://github.com/llvm/llvm-project/pull/83152
Fixes #82512
Adds diagnostics for lambda expressions being cast to boolean values, which
results in the expression always evaluating to true.
Earlier, Clang allowed compilation of such erroneous programs, b
vinayakdsci wrote:
@cor3ntin gentle ping. How exactly should I proceed? copy elision was made
mandatory in C++17, so wouldn't a warning for C++ < 17 be allowable?
I am very happy to implement any suggestions you have for this PR, but I am
unable to get the code to work for C++ < 17 because the
vinayakdsci wrote:
OK, it looks like there should not be a warning in C++11 and 14 too, as copy
initialization with side effects appears to be permitted sine C++11.
Could you direct me to the functions that Clang uses internally for implicit
type conversion?
I am sure that using that I will be
vinayakdsci wrote:
@cor3ntin gentle ping. Could you direct on what other changes are required?
Thanks!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
vinayakdsci wrote:
Sorry about the pings, I think I got a little ahead of myself there.
But for C++11, we got to the conclusion that copy elision gives different
behaviour for C++17 and later and thus different diagnostics?
In case I made a mistake, I'd be very happy to fix it.
Thanks
P.S. Sorr
vinayakdsci wrote:
@Endilll sorry for repeated pings, but I think @cor3ntin might be possibly busy.
If this looks good, can we go ahead with this PR?
Thanks!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lis
vinayakdsci wrote:
@cor3ntin is this good to go?
Thanks!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vinayakdsci wrote:
> LGTM Make sure @cor3ntin is happy, too.
Thanks a lot! I have requested a review from @cor3ntin too. Thanks for your
guidance!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vinayakdsci wrote:
I hope this does it!
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/81127
>From 335d2277881af6737ba899417121c321ce4d098f Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Thu, 8 Feb 2024 17:29:44 +0530
Subject: [PATCH] [Clang][Sema]: Allow copy constructor side effects
---
clang/d
vinayakdsci wrote:
@Endilll as you mention on the discourse, I have divided the tests as per the
difference in the standards. The tests seem to be working fine. I hope you can
review once you find time.
Thanks!
https://github.com/llvm/llvm-project/pull/81127
__
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/81127
>From f2f9ba7a5399f259f4970f78803a02dc5689daff Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Thu, 8 Feb 2024 17:29:44 +0530
Subject: [PATCH] [Clang][Sema]: Allow copy constructor side effects
---
clang/d
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
https://github.com/vinayakdsci edited
https://github.com/llvm/llvm-project/pull/81127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
-Wno-c++1y-extensions -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variabl
vinayakdsci wrote:
> This change needs a release note. Please add an entry to
> `clang/docs/ReleaseNotes.rst` in the section the most adapted to the change,
> and referencing any Github issue this change fixes. Thanks!
Done!
https://github.com/llvm/llvm-project/pull/81127
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/81127
>From 2ecea54704910bc4cd47fa8dfb4f8af370dfb398 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Thu, 8 Feb 2024 17:29:44 +0530
Subject: [PATCH] [Clang][Sema]: Allow copy constructor side effects
---
clang/d
https://github.com/vinayakdsci updated
https://github.com/llvm/llvm-project/pull/81127
>From be1f3528d9fa90bf0fe930744f9df054d0b45425 Mon Sep 17 00:00:00 2001
From: Vinayak Dev
Date: Thu, 8 Feb 2024 17:29:44 +0530
Subject: [PATCH] [Clang][Sema]: Allow copy constructor side effects
---
clang/l
https://github.com/vinayakdsci created
https://github.com/llvm/llvm-project/pull/81127
Fixes #79518
Copy constructors can have initialization with side effects, and thus clang
should not emit a warning when -Wunused-variable is used in this context.
Currently however, a warning is emitted.
N
43 matches
Mail list logo
