[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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.

Now, compilation happens without warnings. 

>From 3d06ced172a76490c13f892d9165d8cf2702eb87 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/lib/Sema/SemaDecl.cpp  |  2 +-
 clang/test/SemaCXX/warn-unused-variables.cpp | 28 ++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 18a5d93ab8e8c6..7f78aef33f1e36 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,7 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct = 
dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d8089355..92032a9c2b8125 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +183,7 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
-  Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+  Foo fooScalar = 1; // no warning
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +297,27 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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/lib/Sema/SemaDecl.cpp  |  3 ++-
 clang/test/SemaCXX/warn-unused-variables.cpp | 28 ++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 18a5d93ab8e8c6..175533b0bd99a9 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,8 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct =
+  dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d8089355..92032a9c2b8125 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +183,7 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
-  Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+  Foo fooScalar = 1; // no warning
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +297,27 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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/docs/ReleaseNotes.rst  |  4 +++
 clang/lib/Sema/SemaDecl.cpp  |  3 ++-
 clang/test/SemaCXX/warn-unused-variables.cpp | 28 ++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e158284aabeab..dea6f040937d6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@ Bug Fixes in This Version
   for logical operators in C23.
   Fixes (`#64356 `_).
 
+- Clang now doesn't produce false-positive warning `-Wunused-variable`
+  for variables created through copy initialization having side-effects.
+  Fixes (`#79518 `_).
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 18a5d93ab8e8c..175533b0bd99a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,8 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct =
+  dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d808935..92032a9c2b812 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +183,7 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
-  Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+  Foo fooScalar = 1; // no warning
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +297,27 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

I think that would give an error when the gnu++11 check is run?

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

> I think that would give an error when the gnu++11 check is run?

It seems to fail on all standards below C++14.

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

That seems fair. How do you think I should proceed? Should I write a new test 
file?

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

Got it! I am on it, please give me some time and I'll report back.

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

OK, the difference in behaviour seems to stem from the fact that in C++-11 mode 
and C++-14 mode, the constructors are marked as not 
elidable(clang/lib/Sema/SemaDecl.cpp, line 2049). This causes it to return 
false when these constructors which are created by copy initialization are 
encountered, specifically while passing in C++ standard modes lower than C++-17

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s

vinayakdsci wrote:

> OK, the difference in behaviour seems to stem from the fact that in C++-11 
> mode and C++-14 mode, the constructors are marked as not 
> elidable(clang/lib/Sema/SemaDecl.cpp, line 2049). This causes it to return 
> false when these constructors which are created by copy initialization are 
> encountered, specifically while passing in C++ standard modes lower than 
> C++-17

This seems to be because Clang at the moment is not accepting converting 
constructors for copy elision at all!
I found ``clang/lib/Sema/SemaDeclCXX.cpp, line 16042, 
Sema::BuildCXXConstructExpr()`` to be the culprit here.
We currently assume that the constructor has the source object currently passed 
as the first argument.
The function is annotated with FIXMEs.

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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/docs/ReleaseNotes.rst  |  4 +++
 clang/lib/Sema/SemaDecl.cpp  |  3 +-
 clang/test/SemaCXX/warn-unused-variables.cpp | 33 
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df3ad20f955eab..5f9f915c97d78e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@ Bug Fixes in This Version
   for logical operators in C23.
   Fixes (`#64356 `_).
 
+- Clang now doesn't produce false-positive warning `-Wunused-variable`
+  for variables created through copy initialization having side-effects.
+  Fixes (`#79518 `_).
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2c526cd0d0e675..898d07e1c32ded 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,8 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct =
+  dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d8089355..c86fb12d0d6298 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -1,5 +1,7 @@
 // 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++14 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +185,12 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
+  // Copy initialzation gives warning before C++17
+#if __cplusplus <= 201402L
   Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+#else
+  Foo fooScalar = 1; // no warning
+#endif
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +304,29 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects (C++17 and later)
+#if __cplusplus >= 201703L
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518
+#endif

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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/docs/ReleaseNotes.rst  |  4 +++
 clang/lib/Sema/SemaDecl.cpp  |  3 +-
 clang/test/SemaCXX/warn-unused-variables.cpp | 33 ++--
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ece6013f672621..007791492cd1b6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@ Bug Fixes in This Version
   for logical operators in C23.
   Fixes (`#64356 `_).
 
+- Clang now doesn't produce false-positive warning `-Wunused-variable`
+  for variables created through copy initialization having side-effects in 
C++17 and later.
+  Fixes (`#79518 `_).
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2c526cd0d0e675..898d07e1c32ded 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,8 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct =
+  dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d8089355..29e8d08d37d8c6 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -1,5 +1,7 @@
 // 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +185,8 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
-  Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+  // Copy initialzation gives warning before C++17
+  Foo fooScalar = 1; // cxx98-14-warning {{unused variable 'fooScalar'}}
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +300,29 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects (C++17 and later)
+#if __cplusplus >= 201703L
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518
+#endif

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via 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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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. Sorry about the pings, please forgive me.

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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 constructors are not 
marked elidable in the code for these standards.

Specifically in the case where the class object is initialized by an '=' sign 
instead of an explicit argument being passed. As the RHS of the '=' is 
different from a `CXXRecordDecl`, `isTemporaryObject()` returns false and thus 
sets the constructor as non-elidable. This is where I am not sure on how to 
proceed, as changing this behavior breaks a number of assertions in the code 
path followed while compiling the source program. Either I could check if the 
RHS of the assignment is an rvalue, and then add an OR condition to set 
Elidable for the constructor to true. However, _whenever_ the constructor is 
elidable, the codegen emits _directly_, and expects an aggregate type to passed 
in, which is not true for a scalar such as an `int`.

Also, this would involve a conversion process in the code?

I would really appreciate pointers on how to solve this problem, and how to 
improve this PR too.

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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, but now emits a 
warning through `-Wpointer-bool-conversion`.

>From 859ce0b2acbeb65543e891e5e5a9b519c0c504b5 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

---
 clang/lib/Sema/SemaChecking.cpp | 21 +
 1 file changed, 21 insertions(+)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..3b03036587baeb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -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 *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+  const unsigned DiagID = diag::warn_impcast_pointer_to_bool;
+  // For lambdas, the pointer type is function, which corresponds to 1.
+  const unsigned FunctionPointerType = 1;
+  // Pretty print the diagnostic for the warning
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), DiagID)
+  << FunctionPointerType << S.str() << E->getSourceRange() << Range
+  << IsEqual;
+  return;
+}
+  }
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

@@ -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 *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+  const unsigned DiagID = diag::warn_impcast_pointer_to_bool;
+  // For lambdas, the pointer type is function, which corresponds to 1.
+  const unsigned FunctionPointerType = 1;
+  // Pretty print the diagnostic for the warning
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), DiagID)
+  << FunctionPointerType << S.str() << E->getSourceRange() << Range
+  << IsEqual;
+  return;

vinayakdsci wrote:

OK, understood. I will make the required changes, and force push again. Thanks 
for the  review!

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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).

Changing `bool b = ` to `auto b =` does not compile, as the compiler complains 
that `auto` is not allowed in non-static struct members. `b` cannot be declared 
as `static` either, which would cause an invalid use of `this` outside of a 
non-static member function. It looks like type deduction using auto for member 
variables is not allowed.

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

@@ -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 *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+  const unsigned DiagID = diag::warn_impcast_pointer_to_bool;
+  // For lambdas, the pointer type is function, which corresponds to 1.
+  const unsigned FunctionPointerType = 1;
+  // Pretty print the diagnostic for the warning
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), DiagID)
+  << FunctionPointerType << S.str() << E->getSourceRange() << Range
+  << IsEqual;
+  return;

vinayakdsci wrote:

Removing the call to `printPretty` will not populate `S`, so the diagnostic 
will not have the name for the identifier describing the error-causing function 
but instead display `' '`. Is there a workaround for this? Or should I leave it 
be? Thanks!

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst|  3 +++
 clang/lib/Sema/SemaChecking.cpp| 18 ++
 clang/test/CXX/drs/dr18xx.cpp  |  5 +
 .../expr/expr.prim/expr.prim.lambda/blocks.mm  |  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp| 14 ++
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..ea0a739af0cca1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -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 auto *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*FunctionPointerType*/ 1 << S.str() << E->getSourceRange()
+  << Range << IsEqual;
+  return;
+}
+  }
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..6fd063e043517b 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -287,6 +287,11 @@ namespace dr1837 { // dr1837: 3.3
   };
 };
   };
+  /* since-cxx11-warning@-6{{address of function '[] {
+struct Local {
+static_assert(sizeof (this->f()) == sizeof(int), "");
+};
+}' will always evaluate to 'true'}} */
 #endif
 }
 
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..9041d07e02a5bc 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of function '[]() {\n}' will 
always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of function '[] {\n}' will 
always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of function 'c' will always 
evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of function 'd' will always 
evaluate to 'true'}}
 }
 
 namespace PR13117 {
diff --git a/clang/test/SemaCXX/warn-bool-conversion.cpp 
b/clang/test/SemaCXX/warn-bool-conversion.cpp
index c81d52d864f2d2..f95627b8ac0ca5 100644
--- a/clang/test/SemaCXX/warn-bool-conversion.cpp
+++ b/clang/test/SemaCXX/warn-bool-conversion.cpp
@@ -81,6 +81,20 @@ struct S2 {
 
 bool f5();
 bool f6(int);
+#if __cplusplus >= 201103L
+auto f7 = []{};
+auto f8 = [](){};
+
+void foo() {
+  bool b;
+  b = f7; // expected-warning {{address of function 'f7' will always evaluate 
to 'true'}}
+  b = f8; // expected-warning {{address of function 'f8' will always evaluate 
to 'true'}}
+  bool is_true = [](){ return true; };
+/* expected-cxx11-warning@-1{{address of function '[]() {
+return true;
+}' will always evaluate to 'true'}} */
+}
+#endif
 
 void bar() {
   bool b;

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst|  3 +++
 clang/lib/Sema/SemaChecking.cpp| 18 ++
 clang/test/CXX/drs/dr18xx.cpp  |  5 +
 .../expr/expr.prim/expr.prim.lambda/blocks.mm  |  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp| 14 ++
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..ea0a739af0cca1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -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 auto *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*FunctionPointerType*/ 1 << S.str() << E->getSourceRange()
+  << Range << IsEqual;
+  return;
+}
+  }
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..6fd063e043517b 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -287,6 +287,11 @@ namespace dr1837 { // dr1837: 3.3
   };
 };
   };
+  /* since-cxx11-warning@-6{{address of function '[] {
+struct Local {
+static_assert(sizeof (this->f()) == sizeof(int), "");
+};
+}' will always evaluate to 'true'}} */
 #endif
 }
 
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..9041d07e02a5bc 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of function '[]() {\n}' will 
always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of function '[] {\n}' will 
always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of function 'c' will always 
evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of function 'd' will always 
evaluate to 'true'}}
 }
 
 namespace PR13117 {
diff --git a/clang/test/SemaCXX/warn-bool-conversion.cpp 
b/clang/test/SemaCXX/warn-bool-conversion.cpp
index c81d52d864f2d2..f95627b8ac0ca5 100644
--- a/clang/test/SemaCXX/warn-bool-conversion.cpp
+++ b/clang/test/SemaCXX/warn-bool-conversion.cpp
@@ -81,6 +81,20 @@ struct S2 {
 
 bool f5();
 bool f6(int);
+#if __cplusplus >= 201103L
+auto f7 = []{};
+auto f8 = [](){};
+
+void foo() {
+  bool b;
+  b = f7; // expected-warning {{address of function 'f7' will always evaluate 
to 'true'}}
+  b = f8; // expected-warning {{address of function 'f8' will always evaluate 
to 'true'}}
+  bool is_true = [](){ return true; };
+/* expected-cxx11-warning@-1{{address of function '[]() {
+return true;
+}' will always evaluate to 'true'}} */
+}
+#endif
 
 void bar() {
   bool b;

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

@@ -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 auto *MRecordDecl = MCallExpr->getRecordDecl()) {
+if (MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*FunctionPointerType*/ 1 << S.str() << E->getSourceRange()
+  << Range << IsEqual;
+  return;
+}
+  }
+}
+  }

vinayakdsci wrote:

That is actually a great idea! and then the diagnostic would simply say address 
of lambda function {..} will always evaluate to true. Would actually be more 
informative than printing the entire expression in my opinion. Should I modify 
the code to do this instead??

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td  |  4 ++--
 clang/lib/Sema/SemaChecking.cpp   | 15 +++
 clang/test/CXX/drs/dr18xx.cpp |  7 ++-
 .../CXX/expr/expr.prim/expr.prim.lambda/blocks.mm |  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp   | 14 ++
 6 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c8141fefb8edba..1fd450237e0266 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4127,8 +4127,8 @@ def ext_ms_impcast_fn_obj : ExtWarn<
   "Microsoft extension">, InGroup;
 
 def warn_impcast_pointer_to_bool : Warning<
-"address of%select{| function| array}0 '%1' will always evaluate to "
-"'true'">,
+"address of%select{| function| array| lambda function pointer conversion 
operator}0 '%1' "
+"will always evaluate to 'true'">,
 InGroup;
 def warn_cast_nonnull_to_bool : Warning<
 "nonnull %select{function call|parameter}0 '%1' will evaluate to "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..1ce7f0044103fc 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -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();
+MRecordDecl && MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*LambdaPointerConversionOperatorType*/ 3 << S.str()
+  << MRecordDecl->getSourceRange() << Range << IsEqual;
+  return;
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..d69698c5e27fdb 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -281,12 +281,17 @@ namespace dr1837 { // dr1837: 3.3
 
   struct A {
 int f();
-bool b = [] {
+bool b = [] { // #dr1837-a
   struct Local {
 static_assert(sizeof(this->f()) == sizeof(int), "");
   };
 };
   };
+  /* since-cxx11-warning@#dr1837-a{{address of lambda function pointer 
conversion operator '[] {
+struct Local {
+static_assert(sizeof (this->f()) == sizeof(int), "");
+};
+}' will always evaluate to 'true'}} */
 #endif
 }
 
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..0e27075a2a2597 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of lambda function pointer 
conversion operator '[]() {\n}' will always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of lambda function pointer 
conversion operator '[] {\n}' will always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of lambda function pointer 
conversion operator 'c' will always evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of lambda function pointer 
conversion op

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via 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();
+MRecordDecl && MRecordDecl->isLambda()) {
+  std::string Str;
+  llvm::raw_string_ostream S(Str);
+
+  E->printPretty(S, nullptr, getPrintingPolicy());
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*LambdaPointerConversionOperatorType*/ 3 << S.str()
+  << MRecordDecl->getSourceRange() << Range << IsEqual;

vinayakdsci wrote:

Whoops, I accidentally missed that! Thanks for pointing out the mistakes. Since 
we don't use `S` anyway, can I omit the call to `printPretty()` altogether?

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst  |  3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  4 ++--
 clang/lib/Sema/SemaChecking.cpp  | 11 +++
 clang/test/CXX/drs/dr18xx.cpp|  2 +-
 .../CXX/expr/expr.prim/expr.prim.lambda/blocks.mm|  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp  | 12 
 6 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c8141fefb8edba..72d8a60bec118e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4127,8 +4127,8 @@ def ext_ms_impcast_fn_obj : ExtWarn<
   "Microsoft extension">, InGroup;
 
 def warn_impcast_pointer_to_bool : Warning<
-"address of%select{| function| array}0 '%1' will always evaluate to "
-"'true'">,
+"address of %select{'%1'|function '%1'|array '%1'|lambda function pointer "
+"conversion operator}0 will always evaluate to 'true'">,
 InGroup;
 def warn_cast_nonnull_to_bool : Warning<
 "nonnull %select{function call|parameter}0 '%1' will evaluate to "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..d27f14715a1b20 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16538,6 +16538,17 @@ 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();
+MRecordDecl && MRecordDecl->isLambda()) {
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*LambdaPointerConversionOperatorType*/ 3
+  << MRecordDecl->getSourceRange() << Range << IsEqual;
+  return;
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..52601153eb6312 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -281,7 +281,7 @@ namespace dr1837 { // dr1837: 3.3
 
   struct A {
 int f();
-bool b = [] {
+bool b = [] { // since-cxx11-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
   struct Local {
 static_assert(sizeof(this->f()) == sizeof(int), "");
   };
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..e93c37f3b9ae12 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
 namespace PR13117 {
diff --git a/clang/test/SemaCXX/warn-bool-conversion.cpp 
b/clang/test/SemaCXX/warn-bool-conversion.cpp
index c81d52d864f2d2..9e8cf0e4f8944a 100644
--- a/clang/test/SemaCXX/warn-bool-conversion.cpp
+++ b/clang/test/SemaCXX/warn-bool-conver

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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 :-) ! I will be more mindful of what changes I have made 
before pushing them to the remote in the future.

Thanks!

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst  |  3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  4 ++--
 clang/lib/Sema/SemaChecking.cpp  | 11 +++
 clang/test/CXX/drs/dr18xx.cpp|  1 +
 .../CXX/expr/expr.prim/expr.prim.lambda/blocks.mm|  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp  | 12 
 6 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f726805dc02bd9..60ec9a259a48ff 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4127,8 +4127,8 @@ def ext_ms_impcast_fn_obj : ExtWarn<
   "Microsoft extension">, InGroup;
 
 def warn_impcast_pointer_to_bool : Warning<
-"address of%select{| function| array}0 '%1' will always evaluate to "
-"'true'">,
+"address of %select{'%1'|function '%1'|array '%1'|lambda function pointer "
+"conversion operator}0 will always evaluate to 'true'">,
 InGroup;
 def warn_cast_nonnull_to_bool : Warning<
 "nonnull %select{function call|parameter}0 '%1' will evaluate to "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 979b63884359fc..2926a81fe7b8a6 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16544,6 +16544,17 @@ 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();
+MRecordDecl && MRecordDecl->isLambda()) {
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*LambdaPointerConversionOperatorType=*/ 3
+  << MRecordDecl->getSourceRange() << Range << IsEqual;
+  return;
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..e78730e8992cf8 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -282,6 +282,7 @@ namespace dr1837 { // dr1837: 3.3
   struct A {
 int f();
 bool b = [] {
+  // since-cxx11-warning@-1 {{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
   struct Local {
 static_assert(sizeof(this->f()) == sizeof(int), "");
   };
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..e93c37f3b9ae12 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
 namespace PR13117 {
diff --git a/clang/test/SemaCXX/warn-bool-conversion.cpp 
b/clang/test/SemaCXX/warn-bool-conversion.cpp
index c81d52d864f2d2..9e8cf0e4f8944a 100644
--- a/clang/test/SemaCXX/warn-bool-conversion.cpp
+++ b/clang/test/SemaCXX/warn-bool-conversion.cpp
@

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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

---
 clang/docs/ReleaseNotes.rst  |  3 +++
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  4 ++--
 clang/lib/Sema/SemaChecking.cpp  | 11 +++
 clang/test/CXX/drs/dr18xx.cpp|  1 +
 .../CXX/expr/expr.prim/expr.prim.lambda/blocks.mm|  9 +
 clang/test/SemaCXX/warn-bool-conversion.cpp  | 12 
 6 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..a5c6b80c4e99e1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses declarative nested name specifiers that name alias 
templates.
 
+- Clang now diagnoses lambda function expressions being implicitly cast to 
boolean values, under ``-Wpointer-bool-conversion``.
+  Fixes `#82512 `_.
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f726805dc02bd9..60ec9a259a48ff 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4127,8 +4127,8 @@ def ext_ms_impcast_fn_obj : ExtWarn<
   "Microsoft extension">, InGroup;
 
 def warn_impcast_pointer_to_bool : Warning<
-"address of%select{| function| array}0 '%1' will always evaluate to "
-"'true'">,
+"address of %select{'%1'|function '%1'|array '%1'|lambda function pointer "
+"conversion operator}0 will always evaluate to 'true'">,
 InGroup;
 def warn_cast_nonnull_to_bool : Warning<
 "nonnull %select{function call|parameter}0 '%1' will evaluate to "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 979b63884359fc..35d453e013e84b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16544,6 +16544,17 @@ 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();
+MRecordDecl && MRecordDecl->isLambda()) {
+  Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+  << /*LambdaPointerConversionOperatorType=*/3
+  << MRecordDecl->getSourceRange() << Range << IsEqual;
+  return;
+}
+  }
+
   // Expect to find a single Decl.  Skip anything more complicated.
   ValueDecl *D = nullptr;
   if (DeclRefExpr *R = dyn_cast(E)) {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a7cee4ef8902f9..e78730e8992cf8 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -282,6 +282,7 @@ namespace dr1837 { // dr1837: 3.3
   struct A {
 int f();
 bool b = [] {
+  // since-cxx11-warning@-1 {{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
   struct Local {
 static_assert(sizeof(this->f()) == sizeof(int), "");
   };
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index cb56f6816ad036..e93c37f3b9ae12 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -65,10 +65,10 @@ void nesting() {
 
 namespace overloading {
   void bool_conversion() {
-if ([](){}) {
+if ([](){}) { // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
-bool b = []{};
+bool b = []{}; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 b = (bool)[]{};
   }
 
@@ -108,8 +108,9 @@ void call_with_lambda() {
 using decltype(a)::operator id; // expected-note {{here}}
   } extern d;
 
-  bool r1 = c;
-  bool r2 = d; // expected-error {{private}}
+  bool r1 = c; // expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
+  bool r2 = d; // expected-error {{private}} \
+  expected-warning{{address of lambda function pointer 
conversion operator will always evaluate to 'true'}}
 }
 
 namespace PR13117 {
diff --git a/clang/test/SemaCXX/warn-bool-conversion.cpp 
b/clang/test/SemaCXX/warn-bool-conversion.cpp
index c81d52d864f2d2..9e8cf0e4f8944a 100644
--- a/clang/test/SemaCXX/warn-bool-conversion.cpp
+++ b/clang/test/SemaCXX/warn-bool-conversion.cpp
@@

[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Vinayak Dev via cfe-commits]

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.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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 C++ 17 and later, or not 
marked as elidable in C++14 and earlier?

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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/docs/ReleaseNotes.rst  |  4 +++
 clang/lib/Sema/SemaDecl.cpp  |  3 +-
 clang/test/SemaCXX/warn-unused-variables.cpp | 33 ++--
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f44fef28b9f17f..2aff72733c8b77 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -218,6 +218,10 @@ Bug Fixes in This Version
   for logical operators in C23.
   Fixes (`#64356 `_).
 
+- Clang no longer produces a false-positive `-Wunused-variable` warning
+  for variables created through copy initialization having side-effects in 
C++17 and later.
+  Fixes (`#79518 `_).
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9fdd8eb236d1ee..6289cf75e17413 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2044,7 +2044,8 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions 
&LangOpts,
   return false;
 
 if (Init) {
-  const auto *Construct = dyn_cast(Init);
+  const auto *Construct =
+  dyn_cast(Init->IgnoreImpCasts());
   if (Construct && !Construct->isElidable()) {
 const CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index b649c7d8089355..29e8d08d37d8c6 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -1,5 +1,7 @@
 // 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-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
 template void f() {
   T t;
   t = 17;
@@ -183,7 +185,8 @@ void foo(int size) {
   NonTriviallyDestructible array[2];  // no warning
   NonTriviallyDestructible nestedArray[2][2]; // no warning
 
-  Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
+  // Copy initialzation gives warning before C++17
+  Foo fooScalar = 1; // cxx98-14-warning {{unused variable 'fooScalar'}}
   Foo fooArray[] = {1,2}; // expected-warning {{unused variable 'fooArray'}}
   Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused 
variable 'fooNested'}}
 }
@@ -297,3 +300,29 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+// Ensure that -Wunused-variable does not emit warning
+// on copy constructors with side effects (C++17 and later)
+#if __cplusplus >= 201703L
+namespace gh79518 {
+
+struct S {
+S(int);
+};
+
+// With an initializer list
+struct A {
+  int x;
+  A(int x) : x(x) {}
+};
+
+void foo() {
+S s(0); // no warning
+S s2 = 0; // no warning
+S s3{0}; // no warning
+
+A a = 1; // no warning
+}
+
+} // namespace gh79518
+#endif

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

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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 have commit access at present, so I would really appreciate it if 
you or someone else could land it on my behalf.

Again, a big 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

[clang] [Sema] -Wpointer-bool-conversion: suppress lambda function pointer conversion diagnostic during instantiation (PR #83497)

[Vinayak Dev via cfe-commits]

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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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/listinfo/cfe-commits

[clang] [Clang][Sema]: Allow copy constructor side effects (PR #81127)

[Vinayak Dev via cfe-commits]

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 able to handle the other cases too.

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