[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -1383,6 +1394,15 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
 << nextPathEntryRange(Path, I + 1, L);
 break;
   }
+
+  case IndirectLocalPathEntry::DefaultArg: {
+const auto *DAE = cast(Elem.E);
+SemaRef.Diag(DAE->getParam()->getDefaultArgRange().getBegin(),
+ diag::note_init_with_default_argument)
+<< DAE->getParam() << nextPathEntryRange(Path, I + 1, L);

bricknerb wrote:

Seems like we only care about DAE->getParam(), and use it twice, so consider 
introducing a variable for it instead or in addition to DAE.

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


[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -107,6 +107,39 @@ namespace std {
 using std::operator""s;
 using std::operator""sv;
 
+namespace default_args {
+  using IntArray = int[];

bricknerb wrote:

Is this helpful if this is only used once?

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


[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -107,6 +107,39 @@ namespace std {
 using std::operator""s;
 using std::operator""sv;
 
+namespace default_args {
+  using IntArray = int[];
+  const int *defaultparam1(const int &def1 [[clang::lifetimebound]] = 0); // 
#def1
+  const int &defaultparam_array([[clang::lifetimebound]] const int *p = 
IntArray{1, 2, 3}); // #def2
+  struct A {
+A(const char*, const int& def3 [[clang::lifetimebound]] = 0); // #def3
+  };
+  const int &defaultparam2(const int &def4 [[clang::lifetimebound]] = 0); // 
#def4
+  const int &defaultparam3(const int &def5 [[clang::lifetimebound]] = 
defaultparam2()); // #def5
+  std::string_view defaultparam4(std::string_view s [[clang::lifetimebound]] = 
std::string()); // #def6
+
+  const int*test_default_args() {

bricknerb wrote:

Missing space before *

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


[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -107,6 +107,39 @@ namespace std {
 using std::operator""s;
 using std::operator""sv;
 
+namespace default_args {
+  using IntArray = int[];
+  const int *defaultparam1(const int &def1 [[clang::lifetimebound]] = 0); // 
#def1
+  const int &defaultparam_array([[clang::lifetimebound]] const int *p = 
IntArray{1, 2, 3}); // #def2
+  struct A {
+A(const char*, const int& def3 [[clang::lifetimebound]] = 0); // #def3

bricknerb wrote:

Missing space before * and & to be consistent with the rest of the code.

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


[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-14 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/5] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/5] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/5] [clang] Add the breaking change to more correctly check
 covariance when return type doesn't point to a class to release notes.

---
 clang/docs/ReleaseNotes.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+  .. code-block:: c++
+
+  struct A {
+virtual const int *f() const;
+  };
+  struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+  };
+
 ABI Changes in This Version
 ---
 

>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 13:38:39 +
Subject: [PATCH 4/5] [clang] Fix code indentation in release notes for fixing
 how we handle pointers to non classes when calculating covariance.

---
 clang/docs/ReleaseNotes.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
 
   .. code-block:: c++
 
-  struct 

[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -1370,7 +1381,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
 break;
   }
 
-  case IndirectLocalPathEntry::LambdaCaptureInit:
+  case IndirectLocalPathEntry::LambdaCaptureInit: {

bricknerb wrote:

Revert this change as it seems unrelated?

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


[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits

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


[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -1370,7 +1381,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
 break;
   }
 
-  case IndirectLocalPathEntry::LambdaCaptureInit:
+  case IndirectLocalPathEntry::LambdaCaptureInit: {

bricknerb wrote:

Revert this change as it is unrelated?

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted
+
+class AClass {
+public:
+  AClass() {}
+  AClass &f() { return *this; }
+};
+
+#define CALLS1 f
+#define CALLS2 CALLS1().CALLS1
+#define CALLS4 CALLS2().CALLS2
+#define CALLS8 CALLS4().CALLS4
+#define CALLS16 CALLS8().CALLS8
+#define CALLS32 CALLS16().CALLS16
+#define CALLS64 CALLS32().CALLS32
+#define CALLS128 CALLS64().CALLS64
+#define CALLS256 CALLS128().CALLS128
+#define CALLS512 CALLS256().CALLS256
+#define CALLS1024 CALLS512().CALLS512
+#define CALLS2048 CALLS1024().CALLS1024
+#define CALLS4096 CALLS2048().CALLS2048
+#define CALLS8192 CALLS4096().CALLS4096
+#define CALLS16384 CALLS8192().CALLS8192
+#define CALLS32768 CALLS16384().CALLS16384
+
+void test_bar() {
+  AClass a;
+  a.CALLS32768();
+}

bricknerb wrote:

Done.

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,1013 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -verify
+
+class AClass {
+public:
+  AClass() {}
+  AClass &foo() { return *this; }
+};
+
+void test_bar() {
+  AClass a;
+  // expected-warning@* {{stack nearly exhausted; compilation time may suffer, 
and crashes due to stack overflow are likely}}
+  a.foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo()

bricknerb wrote:

Resolving since we decided to remove this test.

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted

bricknerb wrote:

Resolving since we decided to remove this test.

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -1596,6 +1597,19 @@ void CodeGenModule::ErrorUnsupported(const Decl *D, 
const char *Type) {
   getDiags().Report(Context.getFullLoc(D->getLocation()), DiagID) << Msg;
 }
 
+void CodeGenModule::warnStackExhausted(SourceLocation Loc) {
+  // Only warn about this once.
+  if (!WarnedStackExhausted) {
+getDiags().Report(Loc, diag::warn_stack_exhausted);
+WarnedStackExhausted = true;
+  }
+}

bricknerb wrote:

I was thinking that as a follow up I will extract this small logic into a class 
which both Sema and CodeGen would own.
I also have https://github.com/llvm/llvm-project/pull/111799 to make them more 
similar.

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,1013 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -verify
+
+class AClass {
+public:
+  AClass() {}
+  AClass &foo() { return *this; }
+};
+
+void test_bar() {
+  AClass a;
+  // expected-warning@* {{stack nearly exhausted; compilation time may suffer, 
and crashes due to stack overflow are likely}}
+  a.foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo()

bricknerb wrote:

Thanks for the feedback!
I've changed the test to use macros to make it shorter.
I also removed the warning check, since it's platform dependent and hard to 
maintain.

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


[clang] [clang] Move Sema::WarnedStackExhausted from public to private. (PR #111799)

2024-10-14 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111799

>From cd82d15940ca3873d57de2e9f12e14c2544138dc Mon Sep 17 00:00:00 2001
From: bricknerb 
Date: Thu, 10 Oct 2024 07:44:19 +
Subject: [PATCH] Move Sema::WarnedStackExhausted from public to private.

---
 clang/include/clang/Sema/Sema.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e0a5397d8db80d..0ac33adfa1a85d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -872,8 +872,6 @@ class Sema final : public SemaBase {
   /// For example, user-defined classes, built-in "id" type, etc.
   Scope *TUScope;
 
-  bool WarnedStackExhausted = false;
-
   void incrementMSManglingNumber() const {
 return CurScope->incrementMSManglingNumber();
   }
@@ -1185,6 +1183,8 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
+  bool WarnedStackExhausted = false;
+
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 

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


[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112371)

2024-10-15 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112371

>From b4c0afe1b691ce4e48b74884ac771a7038bd5de2 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 15 Oct 2024 14:46:59 +
Subject: [PATCH 1/2] [clang] Dedupliate the logic that only warns once when
 stack is almost full.

Zero diff in behavior.
---
 clang/include/clang/Basic/Stack.h | 27 +++
 clang/include/clang/Sema/Sema.h   |  6 ++---
 clang/include/clang/Serialization/ASTReader.h |  6 +++--
 clang/lib/Basic/Stack.cpp | 20 ++
 clang/lib/CodeGen/CodeGenModule.cpp   | 12 ++---
 clang/lib/CodeGen/CodeGenModule.h |  6 ++---
 clang/lib/Sema/Sema.cpp   | 12 ++---
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  3 +--
 clang/lib/Serialization/ASTReader.cpp | 21 +++
 clang/lib/Serialization/ASTReaderDecl.cpp |  3 +--
 10 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 30ebd94aedd1f7..64367d8519bf84 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -16,6 +16,8 @@
 
 #include 
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -50,6 +52,31 @@ namespace clang {
 Fn();
 #endif
   }
+
+class SingleWarningStackAwareExecutor {
+public:
+  SingleWarningStackAwareExecutor(DiagnosticsEngine &diags)
+  : DiagsRef(diags) {}
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K
+  /// is guaranteed). Produces a warning if we're low on stack space and
+  /// allocates more in that case. Use this in code that may recurse deeply to
+  /// avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
+  /// Check to see if we're low on stack space and produce a warning if we're
+  /// low on stack space (Currently, at least 256Kis guaranteed).
+  void warnOnStackNearlyExhausted(SourceLocation Loc);
+
+private:
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  DiagnosticsEngine &DiagsRef;
+  bool WarnedStackExhausted = false;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_STACK_H
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..9c846f2b6b12f0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -49,6 +49,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -546,9 +547,6 @@ class Sema final : public SemaBase {
   /// Print out statistics about the semantic analysis.
   void PrintStats() const;
 
-  /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
-
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ee4e897b248882..2c6251ce0190c6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -445,7 +446,7 @@ class ASTReader
   DiagnosticsEngine &Diags;
   // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
   // has its own version.
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   /// The semantic analysis object that will be processing the
   /// AST files and the translation unit that uses it.
@@ -2180,7 +2181,8 @@ class ASTReader
   /// Report a diagnostic.
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
 
-  void warnStackExhausted(SourceLocation Loc);
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
 
   IdentifierInfo *DecodeIdentifierInf

[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112371)

2024-10-15 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/112371

Zero diff in behavior.

>From b4c0afe1b691ce4e48b74884ac771a7038bd5de2 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 15 Oct 2024 14:46:59 +
Subject: [PATCH] [clang] Dedupliate the logic that only warns once when stack
 is almost full.

Zero diff in behavior.
---
 clang/include/clang/Basic/Stack.h | 27 +++
 clang/include/clang/Sema/Sema.h   |  6 ++---
 clang/include/clang/Serialization/ASTReader.h |  6 +++--
 clang/lib/Basic/Stack.cpp | 20 ++
 clang/lib/CodeGen/CodeGenModule.cpp   | 12 ++---
 clang/lib/CodeGen/CodeGenModule.h |  6 ++---
 clang/lib/Sema/Sema.cpp   | 12 ++---
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  3 +--
 clang/lib/Serialization/ASTReader.cpp | 21 +++
 clang/lib/Serialization/ASTReaderDecl.cpp |  3 +--
 10 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 30ebd94aedd1f7..64367d8519bf84 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -16,6 +16,8 @@
 
 #include 
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -50,6 +52,31 @@ namespace clang {
 Fn();
 #endif
   }
+
+class SingleWarningStackAwareExecutor {
+public:
+  SingleWarningStackAwareExecutor(DiagnosticsEngine &diags)
+  : DiagsRef(diags) {}
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K
+  /// is guaranteed). Produces a warning if we're low on stack space and
+  /// allocates more in that case. Use this in code that may recurse deeply to
+  /// avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
+  /// Check to see if we're low on stack space and produce a warning if we're
+  /// low on stack space (Currently, at least 256Kis guaranteed).
+  void warnOnStackNearlyExhausted(SourceLocation Loc);
+
+private:
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  DiagnosticsEngine &DiagsRef;
+  bool WarnedStackExhausted = false;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_STACK_H
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..9c846f2b6b12f0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -49,6 +49,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -546,9 +547,6 @@ class Sema final : public SemaBase {
   /// Print out statistics about the semantic analysis.
   void PrintStats() const;
 
-  /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
-
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ee4e897b248882..2c6251ce0190c6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -445,7 +446,7 @@ class ASTReader
   DiagnosticsEngine &Diags;
   // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
   // has its own version.
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   /// The semantic analysis object that will be processing the
   /// AST files and the translation unit that uses it.
@@ -2180,7 +2181,8 @@ class ASTReader
   /// Report a diagnostic.
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
 
-  void warnStackExhausted(SourceLocation Loc);
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
 
   IdentifierInfo 

[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nes… (PR #111701)

2024-10-09 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/111701

Done by calling clang::runWithSufficientStackSpace().
Added CodeGenModule::runWithSufficientStackSpace() method similar to the one in 
Sema to provide a single warning when this triggers
Fixes: #111699 

>From 1a63281b6c240352653fd2e4299755c1f32a76f4 Mon Sep 17 00:00:00 2001
From: bricknerb 
Date: Wed, 9 Oct 2024 15:05:34 +
Subject: [PATCH] [clang] Fix segmentation fault caused by stack overflow on
 deeply nested expressions. This is done by calling
 clang::runWithSufficientStackSpace(). Added
 CodeGenModule::runWithSufficientStackSpace() method similar to the one in
 Sema to provide a single warning when this triggers.

---
 clang/lib/CodeGen/CGExpr.cpp  |5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   14 +
 clang/lib/CodeGen/CodeGenModule.h |   11 +
 .../CodeGen/deeply-nested-expressions.cpp | 1013 +
 4 files changed, 1042 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/deeply-nested-expressions.cpp

diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 451442765620f7..5ececf7d940520 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5817,7 +5817,10 @@ LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const 
BinaryOperator *E) {
 
 LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
llvm::CallBase **CallOrInvoke) {
-  RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
+  RValue RV;
+  CGM.runWithSufficientStackSpace(E->getExprLoc(), [&] {
+RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
+  });
 
   if (!RV.isScalar())
 return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 5ba098144a74e7..424455cbf4da39 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/CodeGen/BackendUtil.h"
@@ -1596,6 +1597,19 @@ void CodeGenModule::ErrorUnsupported(const Decl *D, 
const char *Type) {
   getDiags().Report(Context.getFullLoc(D->getLocation()), DiagID) << Msg;
 }
 
+void CodeGenModule::warnStackExhausted(SourceLocation Loc) {
+  // Only warn about this once.
+  if (!WarnedStackExhausted) {
+getDiags().Report(Loc, diag::warn_stack_exhausted);
+WarnedStackExhausted = true;
+  }
+}
+
+void CodeGenModule::runWithSufficientStackSpace(SourceLocation Loc,
+llvm::function_ref Fn) 
{
+  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+}
+
 llvm::ConstantInt *CodeGenModule::getSize(CharUnits size) {
   return llvm::ConstantInt::get(SizeTy, size.getQuantity());
 }
diff --git a/clang/lib/CodeGen/CodeGenModule.h 
b/clang/lib/CodeGen/CodeGenModule.h
index c58bb88035ca8a..57e06cbfac13a9 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -336,6 +336,7 @@ class CodeGenModule : public CodeGenTypeCache {
   std::unique_ptr PGOReader;
   InstrProfStats PGOStats;
   std::unique_ptr SanStats;
+  bool WarnedStackExhausted = false;
 
   // A set of references that have only been seen via a weakref so far. This is
   // used to remove the weak of the reference if we ever see a direct reference
@@ -1297,6 +1298,16 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Print out an error that codegen doesn't support the specified decl yet.
   void ErrorUnsupported(const Decl *D, const char *Type);
 
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K is
+  /// guaranteed). Produces a warning if we're low on stack space and allocates
+  /// more in that case. Use this in code that may recurse deeply to avoid 
stack
+  /// overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
   /// Set the attributes on the LLVM function for the given decl and function
   /// info. This applies attributes necessary for handling the ABI as well as
   /// user specified attributes like section.
diff --git a/clang/test/CodeGen/deeply-nested-expressions.cpp 
b/clang/test/CodeGen/deeply-nested-expressions.cpp
new file mode 100644
index 00..3f7b55d35ed76d
--- /dev/null
+++ b/clang/test/CodeGen/deeply-nested-expressions.cpp
@@ -0,0 +1,1013 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -verify
+
+class AClass {
+public:
+  AClass() {}
+  AClass &foo() { return *this; }
+};
+
+void test_bar() {
+  AClass a;
+  // expected-warning@* {{stack nearly e

[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-09 Thread Boaz Brickner via cfe-commits

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


[clang] Format clang/lib/Sema/Sema.cpp (PR #111518)

2024-10-08 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/111518

None

>From ac3936f80c139adcbd9872d1f56ba3b24162dc9e Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 8 Oct 2024 11:44:26 +0200
Subject: [PATCH 1/2] Fix Sema::makeUnavailableInSystemHeader() indentation

---
 clang/lib/Sema/Sema.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index f05760428458b1..e38fbf5ef50b84 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -575,8 +575,8 @@ void Sema::runWithSufficientStackSpace(SourceLocation Loc,
   clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
 }
 
-bool Sema::makeUnavailableInSystemHeader(SourceLocation loc,
-  UnavailableAttr::ImplicitReason reason) {
+bool Sema::makeUnavailableInSystemHeader(
+SourceLocation loc, UnavailableAttr::ImplicitReason reason) {
   // If we're not in a function, it's an error.
   FunctionDecl *fn = dyn_cast(CurContext);
   if (!fn) return false;

>From 474b8befa9bfeae3cd728112e6958eaa33e593f4 Mon Sep 17 00:00:00 2001
From: bricknerb 
Date: Tue, 8 Oct 2024 10:18:48 +
Subject: [PATCH 2/2] Format file.

---
 clang/lib/Sema/Sema.cpp | 209 +---
 1 file changed, 111 insertions(+), 98 deletions(-)

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index e38fbf5ef50b84..8b2e246aa128cc 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -321,8 +321,8 @@ void Sema::Initialize() {
 SC->InitializeSema(*this);
 
   // Tell the external Sema source about this Sema object.
-  if (ExternalSemaSource *ExternalSema
-  = dyn_cast_or_null(Context.getExternalSource()))
+  if (ExternalSemaSource *ExternalSema =
+  dyn_cast_or_null(Context.getExternalSource()))
 ExternalSema->InitializeSema(*this);
 
   // This needs to happen after ExternalSemaSource::InitializeSema(this) or we
@@ -347,7 +347,6 @@ void Sema::Initialize() {
   PushOnScopeChains(Context.getUInt128Decl(), TUScope);
   }
 
-
   // Initialize predefined Objective-C types:
   if (getLangOpts().ObjC) {
 // If 'SEL' does not yet refer to any declarations, make it refer to the
@@ -413,7 +412,6 @@ void Sema::Initialize() {
   // 32-bit integer and OpenCLC v2.0, s6.1.1 int is always 32-bit wide.
   addImplicitTypedef("atomic_flag", Context.getAtomicType(Context.IntTy));
 
-
   // OpenCL v2.0 s6.13.11.6:
   // - The atomic_long and atomic_ulong types are supported if the
   //   cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics
@@ -462,7 +460,6 @@ void Sema::Initialize() {
 addImplicitTypedef("atomic_long", AtomicLongT);
 addImplicitTypedef("atomic_ulong", AtomicULongT);
 
-
 if (Context.getTypeSize(Context.getSizeType()) == 64) {
   AddPointerSizeDependentTypes();
 }
@@ -479,17 +476,17 @@ void Sema::Initialize() {
   if (Context.getTargetInfo().hasAArch64SVETypes() ||
   (Context.getAuxTargetInfo() &&
Context.getAuxTargetInfo()->hasAArch64SVETypes())) {
-#define SVE_TYPE(Name, Id, SingletonId) \
-addImplicitTypedef(Name, Context.SingletonId);
+#define SVE_TYPE(Name, Id, SingletonId)
\
+  addImplicitTypedef(Name, Context.SingletonId);
 #include "clang/Basic/AArch64SVEACLETypes.def"
   }
 
   if (Context.getTargetInfo().getTriple().isPPC64()) {
-#define PPC_VECTOR_MMA_TYPE(Name, Id, Size) \
-  addImplicitTypedef(#Name, Context.Id##Ty);
+#define PPC_VECTOR_MMA_TYPE(Name, Id, Size)
\
+  addImplicitTypedef(#Name, Context.Id##Ty);
 #include "clang/Basic/PPCTypes.def"
-#define PPC_VECTOR_VSX_TYPE(Name, Id, Size) \
-addImplicitTypedef(#Name, Context.Id##Ty);
+#define PPC_VECTOR_VSX_TYPE(Name, Id, Size)
\
+  addImplicitTypedef(#Name, Context.Id##Ty);
 #include "clang/Basic/PPCTypes.def"
   }
 
@@ -529,7 +526,8 @@ Sema::~Sema() {
   assert(InstantiatingSpecializations.empty() &&
  "failed to clean up an InstantiatingTemplate?");
 
-  if (VisContext) FreeVisContext();
+  if (VisContext)
+FreeVisContext();
 
   // Kill all the active scopes.
   for (sema::FunctionScopeInfo *FSI : FunctionScopes)
@@ -540,8 +538,8 @@ Sema::~Sema() {
 SC->ForgetSema();
 
   // Detach from the external Sema source.
-  if (ExternalSemaSource *ExternalSema
-= dyn_cast_or_null(Context.getExternalSource()))
+  if (ExternalSemaSource *ExternalSema =
+  dyn_cast_or_null(Context.getExternalSource()))
 ExternalSema->ForgetSema();
 
   // Delete cached satisfactions.
@@ -579,7 +577,8 @@ bool Sema::makeUnavailableInSystemHeader(
 SourceLocation loc, UnavailableAttr::ImplicitReason reason) {
   // If we're not in a function, it's an error.
   FunctionDecl *fn = dyn_cast(CurContext);
-  if (!fn) return false;
+  if (!fn)
+return false;
 
   // If 

[clang] Format clang/lib/Sema/Sema.cpp (PR #111518)

2024-10-09 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

Thanks for the feedback!
This all makes sense.
For context, I was going through some of the code and saw some formatting and 
thought it would be more readable to fix those.
I do understand the tradeoff here, so I'll close this pull request.

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


[clang] Format clang/lib/Sema/Sema.cpp (PR #111518)

2024-10-09 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] When checking for covariant return types, make sure the poiners or references are to *classes* (PR #111856)

2024-10-10 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/2] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/2] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

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


[clang] [clang] When checking for covariant return types, make sure the poiners or references are to *classes* (PR #111856)

2024-10-10 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/2] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/2] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

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


[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-11 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111701

>From 1a63281b6c240352653fd2e4299755c1f32a76f4 Mon Sep 17 00:00:00 2001
From: bricknerb 
Date: Wed, 9 Oct 2024 15:05:34 +
Subject: [PATCH 1/2] [clang] Fix segmentation fault caused by stack overflow
 on deeply nested expressions. This is done by calling
 clang::runWithSufficientStackSpace(). Added
 CodeGenModule::runWithSufficientStackSpace() method similar to the one in
 Sema to provide a single warning when this triggers.

---
 clang/lib/CodeGen/CGExpr.cpp  |5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   14 +
 clang/lib/CodeGen/CodeGenModule.h |   11 +
 .../CodeGen/deeply-nested-expressions.cpp | 1013 +
 4 files changed, 1042 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/deeply-nested-expressions.cpp

diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 451442765620f7..5ececf7d940520 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5817,7 +5817,10 @@ LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const 
BinaryOperator *E) {
 
 LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
llvm::CallBase **CallOrInvoke) {
-  RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
+  RValue RV;
+  CGM.runWithSufficientStackSpace(E->getExprLoc(), [&] {
+RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
+  });
 
   if (!RV.isScalar())
 return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 5ba098144a74e7..424455cbf4da39 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/CodeGen/BackendUtil.h"
@@ -1596,6 +1597,19 @@ void CodeGenModule::ErrorUnsupported(const Decl *D, 
const char *Type) {
   getDiags().Report(Context.getFullLoc(D->getLocation()), DiagID) << Msg;
 }
 
+void CodeGenModule::warnStackExhausted(SourceLocation Loc) {
+  // Only warn about this once.
+  if (!WarnedStackExhausted) {
+getDiags().Report(Loc, diag::warn_stack_exhausted);
+WarnedStackExhausted = true;
+  }
+}
+
+void CodeGenModule::runWithSufficientStackSpace(SourceLocation Loc,
+llvm::function_ref Fn) 
{
+  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+}
+
 llvm::ConstantInt *CodeGenModule::getSize(CharUnits size) {
   return llvm::ConstantInt::get(SizeTy, size.getQuantity());
 }
diff --git a/clang/lib/CodeGen/CodeGenModule.h 
b/clang/lib/CodeGen/CodeGenModule.h
index c58bb88035ca8a..57e06cbfac13a9 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -336,6 +336,7 @@ class CodeGenModule : public CodeGenTypeCache {
   std::unique_ptr PGOReader;
   InstrProfStats PGOStats;
   std::unique_ptr SanStats;
+  bool WarnedStackExhausted = false;
 
   // A set of references that have only been seen via a weakref so far. This is
   // used to remove the weak of the reference if we ever see a direct reference
@@ -1297,6 +1298,16 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Print out an error that codegen doesn't support the specified decl yet.
   void ErrorUnsupported(const Decl *D, const char *Type);
 
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K is
+  /// guaranteed). Produces a warning if we're low on stack space and allocates
+  /// more in that case. Use this in code that may recurse deeply to avoid 
stack
+  /// overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
   /// Set the attributes on the LLVM function for the given decl and function
   /// info. This applies attributes necessary for handling the ABI as well as
   /// user specified attributes like section.
diff --git a/clang/test/CodeGen/deeply-nested-expressions.cpp 
b/clang/test/CodeGen/deeply-nested-expressions.cpp
new file mode 100644
index 00..3f7b55d35ed76d
--- /dev/null
+++ b/clang/test/CodeGen/deeply-nested-expressions.cpp
@@ -0,0 +1,1013 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -verify
+
+class AClass {
+public:
+  AClass() {}
+  AClass &foo() { return *this; }
+};
+
+void test_bar() {
+  AClass a;
+  // expected-warning@* {{stack nearly exhausted; compilation time may suffer, 
and crashes due to stack overflow are likely}}
+  a.foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo().foo()
+  .foo().foo().foo().foo().foo(

[clang] [clang] Fix segmentation fault caused by stack overflow on deeply nested expressions (PR #111701)

2024-10-11 Thread Boaz Brickner via cfe-commits


@@ -5817,7 +5817,10 @@ LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const 
BinaryOperator *E) {
 
 LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
llvm::CallBase **CallOrInvoke) {
-  RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);

bricknerb wrote:

Done.

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


[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-11 Thread Boaz Brickner via cfe-commits


@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

bricknerb wrote:

Do you mean that base return type A (`void*`, `int*` or some class type) and 
derived returns type B != A (`void*`, `int*` or some class type)?
Or do you mean they both return the same type but it's qualified differently?

For different types we have T1 and T2.
For different qualification of class types, we have T6.
Let me know what use case you think is uncovered.

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


[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-11 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] When checking for covariant return types, make sure the poiners or references are to *classes* (PR #111856)

2024-10-11 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/4] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/4] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/4] [clang] Add the breaking change to more correctly check
 covariance when return type doesn't point to a class to release notes.

---
 clang/docs/ReleaseNotes.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+  .. code-block:: c++
+
+  struct A {
+virtual const int *f() const;
+  };
+  struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+  };
+
 ABI Changes in This Version
 ---
 

>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 13:38:39 +
Subject: [PATCH 4/4] [clang] Fix code indentation in release notes for fixing
 how we handle pointers to non classes when calculating covariance.

---
 clang/docs/ReleaseNotes.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
 
   .. code-block:: c++
 
-  struct 

[clang] [clang] When checking for covariant return types, make sure the poiners or references are to *classes* (PR #111856)

2024-10-11 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/3] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/3] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/3] [clang] Add the breaking change to more correctly check
 covariance when return type doesn't point to a class to release notes.

---
 clang/docs/ReleaseNotes.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+  .. code-block:: c++
+
+  struct A {
+virtual const int *f() const;
+  };
+  struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+  };
+
 ABI Changes in This Version
 ---
 

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


[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-16 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/5] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/5] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/5] [clang] Add the breaking change to more correctly check
 covariance when return type doesn't point to a class to release notes.

---
 clang/docs/ReleaseNotes.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+  .. code-block:: c++
+
+  struct A {
+virtual const int *f() const;
+  };
+  struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+  };
+
 ABI Changes in This Version
 ---
 

>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 13:38:39 +
Subject: [PATCH 4/5] [clang] Fix code indentation in release notes for fixing
 how we handle pointers to non classes when calculating covariance.

---
 clang/docs/ReleaseNotes.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
 
   .. code-block:: c++
 
-  struct 

[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112371)

2024-10-16 Thread Boaz Brickner via cfe-commits


@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;

bricknerb wrote:

Yes, I considered that and it seems somewhat to add some complexity.
Assuming the same code might hit this in both Sema and CodeGen, I don't see a 
reason the user should see this warning twice.
If Sema and CodeGen would typically be hitting different use cases, it might 
make sense, but I don't think that's the case.
It's anyways beyond the scope of this change, and we could consider it as a 
future improvement, though I doubt it should be high priority.

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


[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-16 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/112552

Zero diff in behavior.

>From 8ff7b52319c5525a4ff26c324c283c6b0d249de3 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 15 Oct 2024 14:46:59 +
Subject: [PATCH 1/3] [clang] Dedupliate the logic that only warns once when
 stack is almost full.

Zero diff in behavior.
---
 clang/include/clang/Basic/Stack.h | 27 +++
 clang/include/clang/Sema/Sema.h   |  6 ++---
 clang/include/clang/Serialization/ASTReader.h |  6 +++--
 clang/lib/Basic/Stack.cpp | 20 ++
 clang/lib/CodeGen/CodeGenModule.cpp   | 12 ++---
 clang/lib/CodeGen/CodeGenModule.h |  6 ++---
 clang/lib/Sema/Sema.cpp   | 12 ++---
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  3 +--
 clang/lib/Serialization/ASTReader.cpp | 21 +++
 clang/lib/Serialization/ASTReaderDecl.cpp |  3 +--
 10 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 30ebd94aedd1f7..64367d8519bf84 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -16,6 +16,8 @@
 
 #include 
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -50,6 +52,31 @@ namespace clang {
 Fn();
 #endif
   }
+
+class SingleWarningStackAwareExecutor {
+public:
+  SingleWarningStackAwareExecutor(DiagnosticsEngine &diags)
+  : DiagsRef(diags) {}
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K
+  /// is guaranteed). Produces a warning if we're low on stack space and
+  /// allocates more in that case. Use this in code that may recurse deeply to
+  /// avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
+  /// Check to see if we're low on stack space and produce a warning if we're
+  /// low on stack space (Currently, at least 256Kis guaranteed).
+  void warnOnStackNearlyExhausted(SourceLocation Loc);
+
+private:
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  DiagnosticsEngine &DiagsRef;
+  bool WarnedStackExhausted = false;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_STACK_H
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..9c846f2b6b12f0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -49,6 +49,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -546,9 +547,6 @@ class Sema final : public SemaBase {
   /// Print out statistics about the semantic analysis.
   void PrintStats() const;
 
-  /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
-
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ee4e897b248882..2c6251ce0190c6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -445,7 +446,7 @@ class ASTReader
   DiagnosticsEngine &Diags;
   // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
   // has its own version.
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   /// The semantic analysis object that will be processing the
   /// AST files and the translation unit that uses it.
@@ -2180,7 +2181,8 @@ class ASTReader
   /// Report a diagnostic.
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
 
-  void warnStackExhausted(SourceLocation Loc);
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
 
   IdentifierI

[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-16 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112552

>From 8ff7b52319c5525a4ff26c324c283c6b0d249de3 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 15 Oct 2024 14:46:59 +
Subject: [PATCH 1/4] [clang] Dedupliate the logic that only warns once when
 stack is almost full.

Zero diff in behavior.
---
 clang/include/clang/Basic/Stack.h | 27 +++
 clang/include/clang/Sema/Sema.h   |  6 ++---
 clang/include/clang/Serialization/ASTReader.h |  6 +++--
 clang/lib/Basic/Stack.cpp | 20 ++
 clang/lib/CodeGen/CodeGenModule.cpp   | 12 ++---
 clang/lib/CodeGen/CodeGenModule.h |  6 ++---
 clang/lib/Sema/Sema.cpp   | 12 ++---
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  3 +--
 clang/lib/Serialization/ASTReader.cpp | 21 +++
 clang/lib/Serialization/ASTReaderDecl.cpp |  3 +--
 10 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 30ebd94aedd1f7..64367d8519bf84 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -16,6 +16,8 @@
 
 #include 
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -50,6 +52,31 @@ namespace clang {
 Fn();
 #endif
   }
+
+class SingleWarningStackAwareExecutor {
+public:
+  SingleWarningStackAwareExecutor(DiagnosticsEngine &diags)
+  : DiagsRef(diags) {}
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K
+  /// is guaranteed). Produces a warning if we're low on stack space and
+  /// allocates more in that case. Use this in code that may recurse deeply to
+  /// avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
+  /// Check to see if we're low on stack space and produce a warning if we're
+  /// low on stack space (Currently, at least 256Kis guaranteed).
+  void warnOnStackNearlyExhausted(SourceLocation Loc);
+
+private:
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  DiagnosticsEngine &DiagsRef;
+  bool WarnedStackExhausted = false;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_STACK_H
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..9c846f2b6b12f0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -49,6 +49,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -546,9 +547,6 @@ class Sema final : public SemaBase {
   /// Print out statistics about the semantic analysis.
   void PrintStats() const;
 
-  /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
-
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ee4e897b248882..2c6251ce0190c6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -445,7 +446,7 @@ class ASTReader
   DiagnosticsEngine &Diags;
   // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
   // has its own version.
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   /// The semantic analysis object that will be processing the
   /// AST files and the translation unit that uses it.
@@ -2180,7 +2181,8 @@ class ASTReader
   /// Report a diagnostic.
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
 
-  void warnStackExhausted(SourceLocation Loc);
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
 
   IdentifierInfo *DecodeIdentifierInf

[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

>From what I can tell, buildkite failures seem unrelated.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

> Can you merge `main` into your branch? I think that should fix the CI 
> failures.

Done.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112853

>From b1ffbf6b7a59d1e57dccf8b9fab32c2c7d599058 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Mon, 21 Oct 2024 13:07:37 +0200
Subject: [PATCH] [clang] Add covariance tests that make sure we return an
 error when return value is different in pointer / lvalue ref / rvalue ref

Per https://cplusplus.github.io/CWG/issues/960.html.
---
 clang/test/SemaCXX/virtual-override.cpp | 28 +
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index ce6dd35e0b56fa..fc2a36937d4b5d 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -83,7 +83,7 @@ namespace T6 {
 struct a { };
 
 class A {
-  // Classes.
+  // Check cv-qualification.
   virtual const a* const_vs_unqualified_class();
   virtual a* unqualified_vs_const_class(); // expected-note{{overridden 
virtual function is here}}
 
@@ -102,13 +102,23 @@ class A {
   virtual const volatile a* const_volatile_vs_unualified_class();
   virtual a* unqualified_vs_const_volatile_class(); // 
expected-note{{overridden virtual function is here}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a& rvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+
+  // Check non class.
   virtual const int* const_vs_unqualified_non_class(); // 
expected-note{{overridden virtual function is here}}
   virtual int* unqualified_vs_const_non_class(); // expected-note{{overridden 
virtual function is here}}
 };
 
 class B : A {
-  // Classes.
+  // Check cv-qualification.
   a* const_vs_unqualified_class() override;
   const a* unqualified_vs_const_class() override; // expected-error{{return 
type of virtual function 'unqualified_vs_const_class' is not covariant with the 
return type of the function it overrides (class type 'const a *' does not have 
the same cv-qualification as or less cv-qualification than class type 'a *')}}
 
@@ -127,7 +137,17 @@ class B : A {
   a* const_volatile_vs_unualified_class() override;
   const volatile a* unqualified_vs_const_volatile_class() override; // 
expected-error{{return type of virtual function 
'unqualified_vs_const_volatile_class' is not covariant with the return type of 
the function it overrides (class type 'const volatile a *' does not have the 
same cv-qualification as or less cv-qualification than class type 'a *')}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  a& rvalue_ref() override;
+  a&& lvalue_ref() override;
+  a&& rvalue_vs_lvalue_ref() override; // expected-error{{virtual function 
'rvalue_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a &')}}
+  a& lvalue_vs_rvalue_ref() override; // expected-error{{virtual function 
'lvalue_vs_rvalue_ref' has a different return type ('a &') than the function it 
overrides (which has return type 'a &&')}}
+  a* rvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'rvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &')}}
+  a& pointer_vs_rvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_rvalue_ref' has a different return type ('a &') than the function 
it overrides (which has return type 'a *')}}
+  a* lvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'lvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &&')}}
+  a&& pointer_vs_lvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a *')}}
+
+  // Check non class.
   int* const_vs_unqualified_non_class() override; // expected-error{{virtual 
function 'const_vs_unqualified_non_class' has a different return type ('int *') 
than the function it overrides (which has return type 'const int *')}}
   const int* unqualified_vs_const_non_class() override; // 
expected-error{{virtual function 'unqualified_vs_const_non_class' has a 
different return type ('const int *') than the function it overrides (which has 
return type 'int *')}}
 };

___
cfe-commits mailing list
cfe-commits@lists.

[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112853

>From b1ffbf6b7a59d1e57dccf8b9fab32c2c7d599058 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Mon, 21 Oct 2024 13:07:37 +0200
Subject: [PATCH 1/2] [clang] Add covariance tests that make sure we return an
 error when return value is different in pointer / lvalue ref / rvalue ref

Per https://cplusplus.github.io/CWG/issues/960.html.
---
 clang/test/SemaCXX/virtual-override.cpp | 28 +
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index ce6dd35e0b56fa..fc2a36937d4b5d 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -83,7 +83,7 @@ namespace T6 {
 struct a { };
 
 class A {
-  // Classes.
+  // Check cv-qualification.
   virtual const a* const_vs_unqualified_class();
   virtual a* unqualified_vs_const_class(); // expected-note{{overridden 
virtual function is here}}
 
@@ -102,13 +102,23 @@ class A {
   virtual const volatile a* const_volatile_vs_unualified_class();
   virtual a* unqualified_vs_const_volatile_class(); // 
expected-note{{overridden virtual function is here}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a& rvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+
+  // Check non class.
   virtual const int* const_vs_unqualified_non_class(); // 
expected-note{{overridden virtual function is here}}
   virtual int* unqualified_vs_const_non_class(); // expected-note{{overridden 
virtual function is here}}
 };
 
 class B : A {
-  // Classes.
+  // Check cv-qualification.
   a* const_vs_unqualified_class() override;
   const a* unqualified_vs_const_class() override; // expected-error{{return 
type of virtual function 'unqualified_vs_const_class' is not covariant with the 
return type of the function it overrides (class type 'const a *' does not have 
the same cv-qualification as or less cv-qualification than class type 'a *')}}
 
@@ -127,7 +137,17 @@ class B : A {
   a* const_volatile_vs_unualified_class() override;
   const volatile a* unqualified_vs_const_volatile_class() override; // 
expected-error{{return type of virtual function 
'unqualified_vs_const_volatile_class' is not covariant with the return type of 
the function it overrides (class type 'const volatile a *' does not have the 
same cv-qualification as or less cv-qualification than class type 'a *')}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  a& rvalue_ref() override;
+  a&& lvalue_ref() override;
+  a&& rvalue_vs_lvalue_ref() override; // expected-error{{virtual function 
'rvalue_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a &')}}
+  a& lvalue_vs_rvalue_ref() override; // expected-error{{virtual function 
'lvalue_vs_rvalue_ref' has a different return type ('a &') than the function it 
overrides (which has return type 'a &&')}}
+  a* rvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'rvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &')}}
+  a& pointer_vs_rvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_rvalue_ref' has a different return type ('a &') than the function 
it overrides (which has return type 'a *')}}
+  a* lvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'lvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &&')}}
+  a&& pointer_vs_lvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a *')}}
+
+  // Check non class.
   int* const_vs_unqualified_non_class() override; // expected-error{{virtual 
function 'const_vs_unqualified_non_class' has a different return type ('int *') 
than the function it overrides (which has return type 'const int *')}}
   const int* unqualified_vs_const_non_class() override; // 
expected-error{{virtual function 'unqualified_vs_const_non_class' has a 
different return type ('const int *') than the function it overrides (which has 
return type 'int *')}}
 };

>From b4221ce300697bf429278222a3f61f9498b5f5ae Mon Sep 17 00:00:00 2001
From: Boaz Bric

[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

> You should write a test in `clang/test/CXX/drs/cwg0xx.cpp`, possibly by 
> moving the test you've written there. While doing that, you should follow the 
> example of other tests in that file, which includes the way `expected` 
> directives are written, and the presence of `// cwg960: NN` comment that 
> describes what our support for CWG960 is. Then you need to run 
> `clang/www/make_cxx_dr_status` script which generated our "C++ DR Status" 
> page in the documentation: https://clang.llvm.org/cxx_dr_status.html

Done.
`clang/www/make_cxx_dr_status` seems to generate a lot of unrelated changes, so 
perhaps a separate pull request would make sense if we want to update 
cxx_dr_status.html.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112853

>From b1ffbf6b7a59d1e57dccf8b9fab32c2c7d599058 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Mon, 21 Oct 2024 13:07:37 +0200
Subject: [PATCH 1/6] [clang] Add covariance tests that make sure we return an
 error when return value is different in pointer / lvalue ref / rvalue ref

Per https://cplusplus.github.io/CWG/issues/960.html.
---
 clang/test/SemaCXX/virtual-override.cpp | 28 +
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index ce6dd35e0b56fa..fc2a36937d4b5d 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -83,7 +83,7 @@ namespace T6 {
 struct a { };
 
 class A {
-  // Classes.
+  // Check cv-qualification.
   virtual const a* const_vs_unqualified_class();
   virtual a* unqualified_vs_const_class(); // expected-note{{overridden 
virtual function is here}}
 
@@ -102,13 +102,23 @@ class A {
   virtual const volatile a* const_volatile_vs_unualified_class();
   virtual a* unqualified_vs_const_volatile_class(); // 
expected-note{{overridden virtual function is here}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a& rvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+
+  // Check non class.
   virtual const int* const_vs_unqualified_non_class(); // 
expected-note{{overridden virtual function is here}}
   virtual int* unqualified_vs_const_non_class(); // expected-note{{overridden 
virtual function is here}}
 };
 
 class B : A {
-  // Classes.
+  // Check cv-qualification.
   a* const_vs_unqualified_class() override;
   const a* unqualified_vs_const_class() override; // expected-error{{return 
type of virtual function 'unqualified_vs_const_class' is not covariant with the 
return type of the function it overrides (class type 'const a *' does not have 
the same cv-qualification as or less cv-qualification than class type 'a *')}}
 
@@ -127,7 +137,17 @@ class B : A {
   a* const_volatile_vs_unualified_class() override;
   const volatile a* unqualified_vs_const_volatile_class() override; // 
expected-error{{return type of virtual function 
'unqualified_vs_const_volatile_class' is not covariant with the return type of 
the function it overrides (class type 'const volatile a *' does not have the 
same cv-qualification as or less cv-qualification than class type 'a *')}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  a& rvalue_ref() override;
+  a&& lvalue_ref() override;
+  a&& rvalue_vs_lvalue_ref() override; // expected-error{{virtual function 
'rvalue_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a &')}}
+  a& lvalue_vs_rvalue_ref() override; // expected-error{{virtual function 
'lvalue_vs_rvalue_ref' has a different return type ('a &') than the function it 
overrides (which has return type 'a &&')}}
+  a* rvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'rvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &')}}
+  a& pointer_vs_rvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_rvalue_ref' has a different return type ('a &') than the function 
it overrides (which has return type 'a *')}}
+  a* lvalue_ref_vs_pointer() override; // expected-error{{virtual function 
'lvalue_ref_vs_pointer' has a different return type ('a *') than the function 
it overrides (which has return type 'a &&')}}
+  a&& pointer_vs_lvalue_ref() override; // expected-error{{virtual function 
'pointer_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a *')}}
+
+  // Check non class.
   int* const_vs_unqualified_non_class() override; // expected-error{{virtual 
function 'const_vs_unqualified_non_class' has a different return type ('int *') 
than the function it overrides (which has return type 'const int *')}}
   const int* unqualified_vs_const_non_class() override; // 
expected-error{{virtual function 'unqualified_vs_const_non_class' has a 
different return type ('const int *') than the function it overrides (which has 
return type 'int *')}}
 };

>From b4221ce300697bf429278222a3f61f9498b5f5ae Mon Sep 17 00:00:00 2001
From: Boaz Bric

[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits


@@ -93,6 +93,38 @@ struct B : A {
 } // namespace example2
 } // namespace cwg952
 
+namespace cwg960 { // cwg960: 2.8
+struct a {};
+class A {
+#if __cplusplus >= 201103L
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}

bricknerb wrote:

Done.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits


@@ -93,6 +93,38 @@ struct B : A {
 } // namespace example2
 } // namespace cwg952
 
+namespace cwg960 { // cwg960: 2.8
+struct a {};
+class A {
+#if __cplusplus >= 201103L
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a& rvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_rvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+  virtual a&& lvalue_ref_vs_pointer(); // expected-note{{overridden virtual 
function is here}}
+  virtual a* pointer_vs_lvalue_ref(); // expected-note{{overridden virtual 
function is here}}
+#endif
+};
+
+class B : A {
+#if __cplusplus >= 201103L
+  // Check lvalue ref vs rvalue ref vs pointer.
+  a& rvalue_ref() override;
+  a&& lvalue_ref() override;
+  a&& rvalue_vs_lvalue_ref() override; // expected-error{{virtual function 
'rvalue_vs_lvalue_ref' has a different return type ('a &&') than the function 
it overrides (which has return type 'a &')}}

bricknerb wrote:

Thanks!
Done.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

> > clang/www/make_cxx_dr_status seems to generate a lot of unrelated changes, 
> > so perhaps a separate pull request would make sense if we want to update 
> > cxx_dr_status.html.
> 
> Ignore other changes; just include the one that updates CWG960 status. I'll 
> update the rest as an NFC change.

Done.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits


@@ -93,6 +93,38 @@ struct B : A {
 } // namespace example2
 } // namespace cwg952
 
+namespace cwg960 { // cwg960: 2.8

bricknerb wrote:

Thanks!
Done.

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref (PR #112853)

2024-10-21 Thread Boaz Brickner via cfe-commits




bricknerb wrote:

Reverted.

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -45,10 +48,14 @@ enum LifetimeKind {
   /// a default member initializer), the program is ill-formed.
   LK_MemInitializer,
 
-  /// The lifetime of a temporary bound to this entity probably ends too soon,
+  /// The lifetime of a temporary bound to this entity may end too soon,

bricknerb wrote:

Move this change outside this pull request?

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -1091,21 +1104,22 @@ static bool 
isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
 }
 
 static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
-   const AssignedEntity &Entity) {
+   const CapturingEntity &Entity) {
   bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored(
   diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
   return (EnableGSLAssignmentWarnings &&
-  (isRecordWithAttr(Entity.LHS->getType()) ||
+  (isRecordWithAttr(Entity.Expression->getType()) ||
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
 }
 
 static void checkExprLifetimeImpl(Sema &SemaRef,
   const InitializedEntity *InitEntity,
   const InitializedEntity *ExtendingEntity,
   LifetimeKind LK,
-  const AssignedEntity *AEntity, Expr *Init) {
-  assert((AEntity && LK == LK_Assignment) ||
- (InitEntity && LK != LK_Assignment));
+  const CapturingEntity *CEntity, Expr *Init) {

bricknerb wrote:

Can we name CEntity with something more readable?
Perhaps even CaptEntity?

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -18,29 +18,42 @@
 
 namespace clang::sema {
 
-/// Describes an entity that is being assigned.
-struct AssignedEntity {
-  // The left-hand side expression of the assignment.
-  Expr *LHS = nullptr;
+struct CapturingEntity {

bricknerb wrote:

I think that using CapturingEntity for both use cases, with one of them keeping 
a field null makes it much harder to follow.
Is it possible to use a more specific type?

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -269,6 +271,40 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
   }
 }
 
+static bool IsPointerLikeType(QualType QT) {
+  QT = QT.getNonReferenceType();
+  // if (QT->isPointerType())
+  //   return true;
+  auto *RD = QT->getAsCXXRecordDecl();
+  if (!RD)
+return false;
+  RD = RD->getCanonicalDecl();
+  if (auto *CTSD = dyn_cast(RD))
+RD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
+  return RD->hasAttr();
+}
+
+void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
+  if (!FD)
+return;
+  auto *MD = dyn_cast(FD);
+  if (!MD || !MD->getIdentifier() || !MD->getParent()->isInStdNamespace())
+return;
+  static const llvm::StringSet<> CapturingMethods{"insert", "push",
+  "push_front", "push_back"};

bricknerb wrote:

Do we not have emplace* methods on purpose?

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -3223,6 +3225,49 @@ void Sema::CheckArgAlignment(SourceLocation Loc, 
NamedDecl *FDecl,
 << ParamName << (FDecl != nullptr) << FDecl;
 }
 
+void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
+  const Expr *ThisArg,
+  ArrayRef Args) {
+  auto GetArgAt = [&](int Idx) {
+if (IsMemberFunction && Idx == 0)
+  return const_cast(ThisArg);
+return const_cast(Args[Idx - int(IsMemberFunction)]);
+  };
+  for (unsigned I = 0; I < FD->getNumParams(); ++I) {
+auto *CapturedByAttr =
+FD->getParamDecl(I)->getAttr();
+if (!CapturedByAttr)
+  continue;
+for (int CapturingParamIdx : CapturedByAttr->params()) {
+  Expr *Capturing = GetArgAt(CapturingParamIdx);
+  Expr *Captured = GetArgAt(I + IsMemberFunction);
+  CapturingEntity CE{Capturing};

bricknerb wrote:

Consider making this initialization clearer by writing .Expression = Capturing

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -18,29 +18,42 @@
 
 namespace clang::sema {
 
-/// Describes an entity that is being assigned.
-struct AssignedEntity {
-  // The left-hand side expression of the assignment.
-  Expr *LHS = nullptr;
+struct CapturingEntity {
+  // The expression of the entity which captures another entity.
+  // For example:
+  //  1. In an assignment, this would be the left-hand side expression.
+  //std::string_view sv;
+  //sv = std::string(); // Here 'sv' is the 'Entity'.
+  //
+  //  2. In an function call involving a lifetime capture, this would be the
+  //  argument capturing the lifetime of another argument.
+  //void addToSet(std::string_view s [[clang::lifetime_capture_by(sv)]],

bricknerb wrote:

Is sv here referring to sv in (1) or a typo?

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


[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-17 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

This pull request is a replacement for 
https://github.com/llvm/llvm-project/pull/112371.

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


[clang] [clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #111499)

2024-10-17 Thread Boaz Brickner via cfe-commits


@@ -1091,21 +1104,22 @@ static bool 
isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
 }
 
 static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
-   const AssignedEntity &Entity) {
+   const CapturingEntity &Entity) {
   bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored(
   diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
   return (EnableGSLAssignmentWarnings &&
-  (isRecordWithAttr(Entity.LHS->getType()) ||
+  (isRecordWithAttr(Entity.Expression->getType()) ||
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
 }
 
 static void checkExprLifetimeImpl(Sema &SemaRef,

bricknerb wrote:

IIUC, this function handles different use cases and for different use cases it 
needs different args to be set.
Since it's becoming more complex with this change, would it make sense to split 
it to different functions and only reuse the shared logic between the different 
use cases?
I think this could simplify the asserts and make the calling this method much 
clearer and safer.

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


[clang] [clang] Fix covariant cv-qualification check to require the override function return type to have the same or less cv-qualification (PR #112713)

2024-10-17 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/112713

This prevents changing cv-qualification from const to volatile or vice versa, 
for example.

https://eel.is/c++draft/class.virtual#8.3

Previously, we checked that the new type is the same or more qualified to 
return an error, but the standard requires the new type to be the same or less 
qualified and since the cv-qualification is only partially ordered, we cannot 
rely on a check on whether it is more qualified to return an error. Now, we 
reversed the condition to check whether the old is at least as qualified, and 
return an error if it is not.

Also, adjusted the error name and message to clarify the requirement and added 
a missing closing parenthesis.

Added tests to cover different use cases for classes with different 
qualifications and also refactored them to make them easier to follow:
1. Use override to make sure the function names actually match.
2. Named the function in a more descriptive way to clarify what each use case 
is checking.

Fixes: #111742

>From f1253057fe99e7bd161f7e25f7cdfa640d7ed446 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 17 Oct 2024 16:02:28 +0200
Subject: [PATCH] [clang] Fix covariant cv-qualification check to require the
 override function return type to have the same or less cv-qualification

This prevents changing cv-qualification from const to volatile or vice versa, 
for example.

https://eel.is/c++draft/class.virtual#8.3

Previously, we checked that the new type is the same or more qualified to 
return an error, but the standard requires the new type to be the same or less 
qualified and since the cv-qualification is only partially ordered, we cannot 
rely on a check on whether it is more qualified to return an error.
Now, we reversed the condition to check whether the old is at least as 
qualifiied, and return an error if it is not.

Also, adjusted the error name and message to clarify the requirement and added 
a missing closing parenthesis.

Added tests to cover different use cases for classes with different 
qualifications and also refactored them to make them easier ro follow:
1. Use override to make sure the function names actually match.
2. Named the function in a more descriptive way to clarify what each use case 
is checking.

Fixes: #111742
---
 clang/docs/ReleaseNotes.rst   |  9 +++-
 .../clang/Basic/DiagnosticSemaKinds.td|  6 +--
 clang/lib/Sema/SemaDeclCXX.cpp|  4 +-
 clang/test/SemaCXX/virtual-override.cpp   | 52 ---
 4 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc5564b6db119f..1d630335f8d213 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,17 +99,24 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
-- Clang will now correctly not consider pointers to non classes for covariance.
+- Clang will now correctly not consider pointers to non classes for covariance
+  and disallow changing return type to a type that doesn't have the same or 
less cv-qualifications.
 
   .. code-block:: c++
 
 struct A {
   virtual const int *f() const;
+  virtual const std::string *g() const;
 };
 struct B : A {
   // Return type has less cv-qualification but doesn't point to a class.
   // Error will be generated.
   int *f() const override;
+
+  // Return type doesn't have more cv-qualification also not the same or
+  // less cv-qualification.
+  // Error will be generated.
+  volatile std::string *g() const override;
 };
 
 - The warning ``-Wdeprecated-literal-operator`` is now on by default, as this 
is
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..487dd8990d88e9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2182,10 +2182,10 @@ def err_covariant_return_incomplete : Error<
 def err_covariant_return_type_different_qualifications : Error<
   "return type of virtual function %0 is not covariant with the return type of 
"
   "the function it overrides (%1 has different qualifiers than %2)">;
-def err_covariant_return_type_class_type_more_qualified : Error<
+def err_covariant_return_type_class_type_not_same_or_less_qualified : Error<
   "return type of virtual function %0 is not covariant with the return type of 
"
-  "the function it overrides (class type %1 is more qualified than class "
-  "type %2">;
+  "the function it overrides (class type %1 does not have the same "
+  "cv-qualification as or less cv-qualification than class type %2)">;
 
 // C++ implicit special member functions
 def note_in_declaration_of_implicit_special_member : Note<
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCX

[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-18 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/112552

>From 8ff7b52319c5525a4ff26c324c283c6b0d249de3 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 15 Oct 2024 14:46:59 +
Subject: [PATCH 1/5] [clang] Dedupliate the logic that only warns once when
 stack is almost full.

Zero diff in behavior.
---
 clang/include/clang/Basic/Stack.h | 27 +++
 clang/include/clang/Sema/Sema.h   |  6 ++---
 clang/include/clang/Serialization/ASTReader.h |  6 +++--
 clang/lib/Basic/Stack.cpp | 20 ++
 clang/lib/CodeGen/CodeGenModule.cpp   | 12 ++---
 clang/lib/CodeGen/CodeGenModule.h |  6 ++---
 clang/lib/Sema/Sema.cpp   | 12 ++---
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  3 +--
 clang/lib/Serialization/ASTReader.cpp | 21 +++
 clang/lib/Serialization/ASTReaderDecl.cpp |  3 +--
 10 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 30ebd94aedd1f7..64367d8519bf84 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -16,6 +16,8 @@
 
 #include 
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -50,6 +52,31 @@ namespace clang {
 Fn();
 #endif
   }
+
+class SingleWarningStackAwareExecutor {
+public:
+  SingleWarningStackAwareExecutor(DiagnosticsEngine &diags)
+  : DiagsRef(diags) {}
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K
+  /// is guaranteed). Produces a warning if we're low on stack space and
+  /// allocates more in that case. Use this in code that may recurse deeply to
+  /// avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
+  /// Check to see if we're low on stack space and produce a warning if we're
+  /// low on stack space (Currently, at least 256Kis guaranteed).
+  void warnOnStackNearlyExhausted(SourceLocation Loc);
+
+private:
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  DiagnosticsEngine &DiagsRef;
+  bool WarnedStackExhausted = false;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_STACK_H
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0faa5aed4eec3b..9c846f2b6b12f0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -49,6 +49,7 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -546,9 +547,6 @@ class Sema final : public SemaBase {
   /// Print out statistics about the semantic analysis.
   void PrintStats() const;
 
-  /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
-
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ee4e897b248882..2c6251ce0190c6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -445,7 +446,7 @@ class ASTReader
   DiagnosticsEngine &Diags;
   // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
   // has its own version.
-  bool WarnedStackExhausted = false;
+  SingleWarningStackAwareExecutor StackAwareExecutor;
 
   /// The semantic analysis object that will be processing the
   /// AST files and the translation unit that uses it.
@@ -2180,7 +2181,8 @@ class ASTReader
   /// Report a diagnostic.
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
 
-  void warnStackExhausted(SourceLocation Loc);
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
 
   IdentifierInfo *DecodeIdentifierInf

[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-18 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Deduplicate the logic that only warns once when stack is almost full (PR #112552)

2024-10-18 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,45 @@
+//===--- SingleWarningStackAwareExecutor.h - A utility for warning once when
+// close to out of stack space ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// Defines a utilitiy for warning once when close to out of stack space.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_SINGLE_WARNING_STACK_AWARE_EXECUTOR_H
+#define LLVM_CLANG_BASIC_SINGLE_WARNING_STACK_AWARE_EXECUTOR_H
+
+#include "clang/Basic/Diagnostic.h"
+
+namespace clang {
+class SingleWarningStackAwareExecutor {

bricknerb wrote:

Done.

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


[clang] [clang] When checking for covariant return types, make sure the poiners or references are to *classes* (PR #111856)

2024-10-10 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/111856

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.

Fixes: #111742 

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH] [clang] When checking for covariant return types, make sure
 the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

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


[clang] Move Sema::WarnedStackExhausted from public to private. (PR #111799)

2024-10-10 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/111799

None

>From cd82d15940ca3873d57de2e9f12e14c2544138dc Mon Sep 17 00:00:00 2001
From: bricknerb 
Date: Thu, 10 Oct 2024 07:44:19 +
Subject: [PATCH] Move Sema::WarnedStackExhausted from public to private.

---
 clang/include/clang/Sema/Sema.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e0a5397d8db80d..0ac33adfa1a85d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -872,8 +872,6 @@ class Sema final : public SemaBase {
   /// For example, user-defined classes, built-in "id" type, etc.
   Scope *TUScope;
 
-  bool WarnedStackExhausted = false;
-
   void incrementMSManglingNumber() const {
 return CurScope->incrementMSManglingNumber();
   }
@@ -1185,6 +1183,8 @@ class Sema final : public SemaBase {
   std::optional> CachedDarwinSDKInfo;
   bool WarnedDarwinSDKInfoMissing = false;
 
+  bool WarnedStackExhausted = false;
+
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 

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


[clang] [clang] Move Sema::WarnedStackExhausted from public to private. (PR #111799)

2024-10-10 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/113437

None

>From cdef16a4e359acb7bea578dfab5e44589f8b6e2b Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 23 Oct 2024 11:44:45 +0200
Subject: [PATCH] [clang] Update the lifetimebound example with up-to-date
 expected warning and change the sample code to be a fully working example.

---
 clang/include/clang/Basic/AttrDocs.td | 28 +++
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index ee8126cadae232..c2552d459cb474 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3702,20 +3702,32 @@ user-declared functions. For example:
 
 .. code-block:: c++
 
+#include 
+#include 
+
+using namespace std::literals;
+
 // Returns m[key] if key is present, or default_value if not.
 template
 const U &get_or_default(const std::map &m [[clang::lifetimebound]],
 const T &key, /* note, not lifetimebound */
-const U &default_value [[clang::lifetimebound]]);
+const U &default_value [[clang::lifetimebound]]) {
+  if (auto iter = m.find(key); iter != m.end()) return iter->second;
+  else return default_value;
+}
 
-std::map m;
-// warning: temporary "bar"s that might be bound to local reference 'val'
-// will be destroyed at the end of the full-expression
-const std::string &val = get_or_default(m, "foo"s, "bar"s);
+int main() {
+  std::map m;
+  // warning: temporary bound to local reference 'val1' will be destroyed
+  // at the end of the full-expression
+  const std::string &val1 = get_or_default(m, "foo"s, "bar"s);
 
-// No warning in this case.
-std::string def_val = "bar"s;
-const std::string &val = get_or_default(m, "foo"s, def_val);
+  // No warning in this case.
+  std::string def_val = "bar"s;
+  const std::string &val2 = get_or_default(m, "foo"s, def_val);
+
+  return 0;
+}
 
 The attribute can be applied to the implicit ``this`` parameter of a member
 function by writing the attribute after the function type:

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

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


[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-23 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

The Test documentation build failure seems unrelated since it refers to 
ClangCommandLineReference.

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


[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-14 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/111856

>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/5] [clang] When checking for covariant return types, make
 sure the pointers or references are to *classes*.

https://eel.is/c++draft/class.virtual#8.1

This prevents overriding methods with non class return types that have less 
cv-qualification.
---
 clang/lib/Sema/SemaDeclCXX.cpp  |  4 ++--
 clang/test/SemaCXX/virtual-override.cpp | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   }
 
   // The return types aren't either both pointers or references to a class 
type.
-  if (NewClassTy.isNull()) {
+  if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
 Diag(New->getLocation(),
  diag::err_different_return_type_for_overriding_virtual_function)
 << New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-}
+  }
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp 
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/5] Fix indentation.

---
 clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
   diag::err_covariant_return_incomplete,
   New->getDeclName()))
 return true;
-  }
+}
 
 // Check if the new class derives from the old class.
 if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {

>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/5] [clang] Add the breaking change to more correctly check
 covariance when return type doesn't point to a class to release notes.

---
 clang/docs/ReleaseNotes.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
 // Was error, now evaluates to false.
 constexpr bool b = f() == g();
 
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+  .. code-block:: c++
+
+  struct A {
+virtual const int *f() const;
+  };
+  struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+  };
+
 ABI Changes in This Version
 ---
 

>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Fri, 11 Oct 2024 13:38:39 +
Subject: [PATCH 4/5] [clang] Fix code indentation in release notes for fixing
 how we handle pointers to non classes when calculating covariance.

---
 clang/docs/ReleaseNotes.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
 
   .. code-block:: c++
 
-  struct 

[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)

2024-10-14 Thread Boaz Brickner via cfe-commits


@@ -289,3 +289,13 @@ namespace PR8168 {
 static void foo() {} // expected-error{{'static' member function 'foo' 
overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  struct A {
+virtual const int *f() const; // expected-note{{overridden virtual 
function is here}}
+  };
+
+  struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a 
different return type ('int *') than the function it overrides (which has 
return type 'const int *')}}
+  };
+}

bricknerb wrote:

Added tests in T2 and T6, and removed T13.
Please let me know if this is not what you meant.

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


[clang] [clang] Output an error when [[lifetimebound]] attribute is applied on a function parameter while the function returns void (PR #113460)

2024-10-25 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/113460

>From 4405d652029081cd63094e9a81dfa31e8611aad4 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 23 Oct 2024 15:50:43 +0200
Subject: [PATCH 1/4] [clang] Output a warning when [[lifetimebound]] attribute
 is applied on a function parameter while the function returns void.

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  3 +++
 clang/lib/Sema/SemaDecl.cpp  | 14 +-
 clang/test/SemaCXX/attr-lifetimebound.cpp|  3 +--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e4718008ece72..a1c20b22e736ed 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10097,6 +10097,9 @@ def err_lifetimebound_no_object_param : Error<
 def err_lifetimebound_ctor_dtor : Error<
   "'lifetimebound' attribute cannot be applied to a "
   "%select{constructor|destructor}0">;
+def err_lifetimebound_void_return_type : Error<
+  "'lifetimebound' attribute cannot be applied to a parameter of a function "
+  "that returns void; did you mean 'lifetime_capture_by(X)'">;
 
 // CHECK: returning address/reference of stack memory
 def warn_ret_stack_addr_ref : Warning<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 229c9080d558ec..e611bf60718bc5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6940,7 +6940,7 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
 }
   }
 
-  // Check the attributes on the function type, if any.
+  // Check the attributes on the function type and function params, if any.
   if (const auto *FD = dyn_cast(&ND)) {
 // Don't declare this variable in the second operand of the for-statement;
 // GCC miscompiles that by ending its lifetime before evaluating the
@@ -6970,6 +6970,18 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
 }
   }
 }
+
+for (unsigned int I = 0; I < FD->getNumParams(); ++I) {
+  const ParmVarDecl *P = FD->getParamDecl(I);
+
+  // The [[lifetimebound]] attribute can be applied to a function parameter
+  // only if the function returns a value.
+  if (auto *A = P->getAttr()) {
+if (!isa(FD) && FD->getReturnType()->isVoidType()) 
{
+  S.Diag(A->getLocation(), diag::err_lifetimebound_void_return_type);
+}
+  }
+}
   }
 }
 
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 1c5c79777c71c8..f790559b6b4769 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -1,8 +1,7 @@
 // RUN: %clang_cc1 -std=c++23 -verify %s
 
 namespace usage_invalid {
-  // FIXME: Should we diagnose a void return type?
-  void voidreturn(int ¶m [[clang::lifetimebound]]);
+  void voidreturn(int ¶m [[clang::lifetimebound]]); // expected-error 
{{'lifetimebound' attribute cannot be applied to a parameter of a function that 
returns void; did you mean 'lifetime_capture_by(X)'}}
 
   int *not_class_member() [[clang::lifetimebound]]; // expected-error 
{{non-member function has no implicit object parameter}}
   struct A {

>From 0a9a8629f5d188b1c1d648dc83438c33c14cc8cc Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 24 Oct 2024 09:55:01 +0200
Subject: [PATCH 2/4] [clang] Add a test for lifetimebound on a parameter when
 the return type is dependent on template argument but actually void.

---
 clang/test/SemaCXX/attr-lifetimebound.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index f790559b6b4769..33cce02936dda1 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -30,6 +30,11 @@ namespace usage_ok {
 return *(int*)param;
   }
 
+  template  R dependent_void(const T& t 
[[clang::lifetimebound]]);
+  void dependent_void_instantiation() {
+dependent_void(1);
+  }
+
   struct A {
 A();
 A(int);

>From aab99080ba2b6d990d3b56f78ffec0ea2aaea948 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 24 Oct 2024 10:13:14 +0200
Subject: [PATCH 3/4] [clang] Test a member method marked with lifetimebound
 that returns a void. This should diagnose but currently isn't. Added a FIXME
 comment.

---
 clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 33cce02936dda1..a0005ec1ccf4b4 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++23 -verify %s
 
 namespace usage_invalid {
-  void voidreturn(int ¶m [[clang::lifetimebound]]);

[clang] [clang] Output an error when [[lifetimebound]] attribute is applied on a function parameter while the function returns void (PR #113460)

2024-10-25 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb updated 
https://github.com/llvm/llvm-project/pull/113460

>From 4405d652029081cd63094e9a81dfa31e8611aad4 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 23 Oct 2024 15:50:43 +0200
Subject: [PATCH 1/5] [clang] Output a warning when [[lifetimebound]] attribute
 is applied on a function parameter while the function returns void.

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  3 +++
 clang/lib/Sema/SemaDecl.cpp  | 14 +-
 clang/test/SemaCXX/attr-lifetimebound.cpp|  3 +--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e4718008ece72..a1c20b22e736ed 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10097,6 +10097,9 @@ def err_lifetimebound_no_object_param : Error<
 def err_lifetimebound_ctor_dtor : Error<
   "'lifetimebound' attribute cannot be applied to a "
   "%select{constructor|destructor}0">;
+def err_lifetimebound_void_return_type : Error<
+  "'lifetimebound' attribute cannot be applied to a parameter of a function "
+  "that returns void; did you mean 'lifetime_capture_by(X)'">;
 
 // CHECK: returning address/reference of stack memory
 def warn_ret_stack_addr_ref : Warning<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 229c9080d558ec..e611bf60718bc5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6940,7 +6940,7 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
 }
   }
 
-  // Check the attributes on the function type, if any.
+  // Check the attributes on the function type and function params, if any.
   if (const auto *FD = dyn_cast(&ND)) {
 // Don't declare this variable in the second operand of the for-statement;
 // GCC miscompiles that by ending its lifetime before evaluating the
@@ -6970,6 +6970,18 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
 }
   }
 }
+
+for (unsigned int I = 0; I < FD->getNumParams(); ++I) {
+  const ParmVarDecl *P = FD->getParamDecl(I);
+
+  // The [[lifetimebound]] attribute can be applied to a function parameter
+  // only if the function returns a value.
+  if (auto *A = P->getAttr()) {
+if (!isa(FD) && FD->getReturnType()->isVoidType()) 
{
+  S.Diag(A->getLocation(), diag::err_lifetimebound_void_return_type);
+}
+  }
+}
   }
 }
 
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 1c5c79777c71c8..f790559b6b4769 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -1,8 +1,7 @@
 // RUN: %clang_cc1 -std=c++23 -verify %s
 
 namespace usage_invalid {
-  // FIXME: Should we diagnose a void return type?
-  void voidreturn(int ¶m [[clang::lifetimebound]]);
+  void voidreturn(int ¶m [[clang::lifetimebound]]); // expected-error 
{{'lifetimebound' attribute cannot be applied to a parameter of a function that 
returns void; did you mean 'lifetime_capture_by(X)'}}
 
   int *not_class_member() [[clang::lifetimebound]]; // expected-error 
{{non-member function has no implicit object parameter}}
   struct A {

>From 0a9a8629f5d188b1c1d648dc83438c33c14cc8cc Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 24 Oct 2024 09:55:01 +0200
Subject: [PATCH 2/5] [clang] Add a test for lifetimebound on a parameter when
 the return type is dependent on template argument but actually void.

---
 clang/test/SemaCXX/attr-lifetimebound.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index f790559b6b4769..33cce02936dda1 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -30,6 +30,11 @@ namespace usage_ok {
 return *(int*)param;
   }
 
+  template  R dependent_void(const T& t 
[[clang::lifetimebound]]);
+  void dependent_void_instantiation() {
+dependent_void(1);
+  }
+
   struct A {
 A();
 A(int);

>From aab99080ba2b6d990d3b56f78ffec0ea2aaea948 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Thu, 24 Oct 2024 10:13:14 +0200
Subject: [PATCH 3/5] [clang] Test a member method marked with lifetimebound
 that returns a void. This should diagnose but currently isn't. Added a FIXME
 comment.

---
 clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 33cce02936dda1..a0005ec1ccf4b4 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++23 -verify %s
 
 namespace usage_invalid {
-  void voidreturn(int ¶m [[clang::lifetimebound]]);

[clang] [clang] Update the lifetimebound example with up-to-date expected warning and change the sample code to be a fully working example (PR #113437)

2024-10-30 Thread Boaz Brickner via cfe-commits


@@ -3702,20 +3702,32 @@ user-declared functions. For example:
 
 .. code-block:: c++
 
+#include 
+#include 
+
+using namespace std::literals;
+
 // Returns m[key] if key is present, or default_value if not.
 template
 const U &get_or_default(const std::map &m [[clang::lifetimebound]],

bricknerb wrote:

I didn't change this one.
However, it makes sense to me since the function returns either a reference to 
a value in the map or the default value so both could be referred to from the 
return value.

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


[clang] [clang] Output an error when [[lifetimebound]] attribute is applied on a function implicit object parameter while the function returns void (PR #114203)

2024-10-30 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/114203

Fixes: https://github.com/llvm/llvm-project/issues/107556

>From 82d89b8b5d1e5faebefed57f3289eb43ad9f8d65 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 30 Oct 2024 11:24:07 +0100
Subject: [PATCH 1/2] [clang] Output an error when [[lifetimebound]] attribute
 is applied on a function implicit object parameter while the function returns
 void Fixes: #107556

---
 clang/docs/ReleaseNotes.rst  | 4 ++--
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 5 -
 clang/lib/Sema/SemaDecl.cpp  | 8 +++-
 clang/test/SemaCXX/attr-lifetimebound.cpp| 3 +--
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 920a2369f96435..5559875c0ebb7c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -134,8 +134,8 @@ C++ Specific Potentially Breaking Changes
 unsigned operator""_udl_name(unsigned long long);
 
 - Clang will now produce an error diagnostic when [[clang::lifetimebound]] is
-  applied on a parameter of a function that returns void. This was previously 
-  ignored and had no effect. (#GH107556)
+  applied on a parameter or an implicit object parameter of a function that
+  returns void. This was previously ignored and had no effect. (#GH107556)
 
   .. code-block:: c++
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9b9bdd7c800e37..681c2757b7c76d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10097,9 +10097,12 @@ def err_lifetimebound_no_object_param : Error<
 def err_lifetimebound_ctor_dtor : Error<
   "'lifetimebound' attribute cannot be applied to a "
   "%select{constructor|destructor}0">;
-def err_lifetimebound_void_return_type : Error<
+def err_lifetimebound_parameter_void_return_type : Error<
   "'lifetimebound' attribute cannot be applied to a parameter of a function "
   "that returns void">;
+def err_lifetimebound_implicit_object_parameter_void_return_type : Error<
+  "'lifetimebound' attribute cannot be applied to an implicit object "
+  "parameter of a function that returns void">;
 
 // CHECK: returning address/reference of stack memory
 def warn_ret_stack_addr_ref : Warning<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f8e5f3c6d309d6..c09ff4d1975e24 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6967,6 +6967,11 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
 } else if (isa(MD) || isa(MD)) {
   S.Diag(A->getLocation(), diag::err_lifetimebound_ctor_dtor)
   << isa(MD) << A->getRange();
+} else if (FD->getReturnType()->isVoidType()) {
+  S.Diag(
+  FD->getLocation(),
+  diag::
+  
err_lifetimebound_implicit_object_parameter_void_return_type);
 }
   }
 }
@@ -6978,7 +6983,8 @@ static void checkAttributesAfterMerging(Sema &S, 
NamedDecl &ND) {
   // only if the function returns a value.
   if (auto *A = P->getAttr()) {
 if (!isa(FD) && FD->getReturnType()->isVoidType()) 
{
-  S.Diag(A->getLocation(), diag::err_lifetimebound_void_return_type);
+  S.Diag(A->getLocation(),
+ diag::err_lifetimebound_parameter_void_return_type);
 }
   }
 }
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp 
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 804d61fb62ca40..81e9193cf76a04 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -11,8 +11,7 @@ namespace usage_invalid {
 int *explicit_object(this A&) [[clang::lifetimebound]]; // expected-error 
{{explicit object member function has no implicit object parameter}}
 int not_function [[clang::lifetimebound]]; // expected-error {{only 
applies to parameters and implicit object parameters}}
 int [[clang::lifetimebound]] also_not_function; // expected-error {{cannot 
be applied to types}}
-// FIXME: Should diagnose a void return type.
-void void_return_member() [[clang::lifetimebound]];
+void void_return_member() [[clang::lifetimebound]]; // expected-error 
{{'lifetimebound' attribute cannot be applied to an implicit object parameter 
of a function that returns void}}
   };
   int *attr_with_param(int ¶m [[clang::lifetimebound(42)]]); // 
expected-error {{takes no arguments}}
 }

>From ae34279079f482c22eaa6c128c7e4df96ae11f5d Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 30 Oct 2024 11:24:07 +0100
Subject: [PATCH 2/2] [clang] Output an error when [[lifetimebound]] attribute
 is applied on a function implicit object parameter while the function returns
 void Fixes: #107556

---
 clang/docs/ReleaseNotes.rst  | 4 ++--
 clang/in

[clang] [clang] Initialize DeclaratorDecl.ExtInfo.TInfo to null (PR #114198)

2024-10-30 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/114198

I believe this has no effect current behavior but would make code safer in case 
we forget to initialize this.

>From 647406c00b1b60b23c3fd4314a1e5b4baeb9e574 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Wed, 30 Oct 2024 10:36:57 +0100
Subject: [PATCH] [clang] Initialize DeclaratorDecl.ExtInfo.TInfo to null

I believe this has no effect current behavior but would make code safer in case 
we forget to initialize this.
---
 clang/include/clang/AST/Decl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 7ff35d73df5997..8c39ef3d5a9fa6 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -737,7 +737,7 @@ class DeclaratorDecl : public ValueDecl {
   // qualifier, to be used for the (uncommon) case of out-of-line declarations
   // and constrained function decls.
   struct ExtInfo : public QualifierInfo {
-TypeSourceInfo *TInfo;
+TypeSourceInfo *TInfo = nullptr;
 Expr *TrailingRequiresClause = nullptr;
   };
 

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-10-30 Thread Boaz Brickner via cfe-commits

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-10-30 Thread Boaz Brickner via cfe-commits

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-10-30 Thread Boaz Brickner via cfe-commits

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-10-30 Thread Boaz Brickner via cfe-commits

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-10-30 Thread Boaz Brickner via cfe-commits


@@ -477,6 +485,100 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor 
Flavor,
   setSeverity(Diag, Map, Loc);
 }
 
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {

bricknerb wrote:

Yes, I think the current state is pretty bad so if you're willing to refactor 
(in a separate PR, could also be a followup), this would be great. I think 
WarningsSpecialCaseList::createOrDie() would be surprising and I assume future 
changes could be harder to make and/or make the API much less clear.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -477,6 +485,100 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor 
Flavor,
   setSeverity(Diag, Map, Loc);
 }
 
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {

bricknerb wrote:

Add a FIXME/TODO ?

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -946,6 +955,27 @@ class DiagnosticsEngine : public 
RefCountedBase {
 return (Level)Diags->getDiagnosticLevel(DiagID, Loc, *this);
   }
 
+  /// Diagnostic suppression mappings can be used to suppress specific
+  /// diagnostics in specific files.
+  /// Mapping file is expected to be a special case list with sections denoting
+  /// diagnostic groups and `src` entries for globs to suppress. `emit` 
category
+  /// can be used to disable suppression. Longest glob that matches a filepath
+  /// takes precendence. For example:

bricknerb wrote:

```suggestion
  /// takes precedence. For example:
```

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -477,6 +486,118 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor 
Flavor,
   setSeverity(Diag, Map, Loc);
 }
 
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+  static std::unique_ptr
+  create(const llvm::MemoryBuffer &MB, std::string &Err) {
+auto SCL = std::make_unique();

bricknerb wrote:

Replace SCL with WarningSuppressionList

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.

bricknerb wrote:

When you say "or" do you we try to match both or do we match in one of these 
methods? I think it's worth to clarify that.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.

bricknerb wrote:

precedence

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th
+current working directory, or as absolute paths.
+When a source file matches multiple globs, the longest one takes precendence.

bricknerb wrote:

What happens if a warning that belongs to multiple diagnostic groups is in a 
source file that matches multiple globs in multiple diagnostic groups?
Would it be the longest glob in the last group or the longest glob in all 
categories?
I think it's clear from the example that the warning would only trigger the 
section of the latest group, but it might worth clarifying.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -477,6 +486,118 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor 
Flavor,
   setSeverity(Diag, Map, Loc);
 }
 
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+  static std::unique_ptr
+  create(const llvm::MemoryBuffer &MB, std::string &Err) {

bricknerb wrote:

Avoid putting the implementation in an anonymous namespace.
Per https://llvm.org/docs/CodingStandards.html#anonymous-namespaces, we should 
only put the class declaration into anonymous namespace.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from
+suppression.
+Source files are matched against these globs either as paths relative to th

bricknerb wrote:

"th" typo.
```suggestion
Source files are matched against these globs either as paths relative to the
```

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to

bricknerb wrote:

Perhaps refer somewhere to what is a "diagnostic group" so a user can easily 
what diagnostic groups are available?

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -70,6 +76,16 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
   else
 Diags.setExtensionHandlingBehavior(diag::Severity::Ignored);
 
+  if (!Opts.DiagnosticSuppressionMappingsFile.empty()) {
+if (auto Buf =

bricknerb wrote:

[Buf](https://github.com/search?type=code&auto_enroll=true&q=repo%3Allvm%2Fllvm-project+%2Fbuf%5B%5Ef%5D.*%3D.*%5Cn%3F.*getBufferForFile%2F)
 is less common, and more so if we focus on Clang code. IIUC, this is the 
content of the file, FileBuffer or FileContent is more descriptive, and Buffer 
is better than Buf.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) {
   // Make sure an empty warning can round-trip with \c StoredDiagnostic.
   Diags.Report(CaptureConsumer.StoredDiags.front());
 }
+
+class SuppressionMappingTest : public testing::Test {

bricknerb wrote:

Do you also test pragmas here?

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes
+precendence.
+
+Afterwards in each section, users can have multiple entities that match source
+files based on the globs. These entities look like ``src:*/my/dir/*``.
+Users can also use the ``emit`` category to exclude a subdirectory from

bricknerb wrote:

"category" here might be confused with a diagnostic category, which is probably 
not what you mean here.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits




bricknerb wrote:

Consider also testing the same warning belonging to multiple groups.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits




bricknerb wrote:

Given that Test documentation build check failed, it's worth clarifying how the 
documentation was tested and perhaps add a few screenshots.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits




bricknerb wrote:

Consider referring to pragmas.

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name

bricknerb wrote:

> Sections are denoted as ``[unused]`` in this format.

Isn't "unused" only an example for a diagnostic group?
Did you mean to refer to the specific example?

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


[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

2024-11-04 Thread Boaz Brickner via cfe-commits


@@ -0,0 +1,90 @@
+
+Warning suppression mappings
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Warning suppression mappings enable users to suppress Clang's diagnostics in a
+per-file granular manner. Enabling enforcement of diagnostics in specific parts
+of the project, even if there are violations in some headers.
+
+Goal and usage
+==
+
+Clang allows diagnostics to be configured at a translation-unit granularity.
+If a ``foo.cpp`` is compiled with ``-Wfoo``, all transitively included headers
+also need to be clean. Hence turning on new warnings in large codebases can be
+difficult today. Since it requires cleaning up all the existing warnings,
+which might not be possible when some dependencies aren't in the project 
owner's
+control or because new violations are creeping up quicker than the clean up.
+
+Warning suppression mappings aim to alleviate some of these concerns by making
+diagnostic configuration granularity finer, at a source file level.
+
+To achieve this, user can create a file that lists which diagnostic groups to
+suppress in which files or paths, and pass it as a command line argument to
+Clang with the ``--warning-suppression-mappings`` flag.
+
+Note that this mechanism won't enable any diagnostics on its own. Users should
+still turn on warnings in their compilations with explicit ``-Wfoo`` flags.
+
+Example
+===
+
+.. code-block:: bash
+
+  $ cat my/user/code.cpp
+  #include 
+  namespace { void unused_func1(); }
+
+  $ cat foo/bar.h
+  namespace { void unused_func2(); }
+
+  $ cat suppression_mappings.txt
+  # Suppress -Wunused warnings in all files, apart from the ones under `foo/`.
+  [unused]
+  src:*
+  src:*foo/*=emit
+  $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt 
my/user/code.cpp
+  # prints warning: unused function 'unused_func2', but no warnings for 
`unused_func1`.
+
+Format
+==
+
+Warning suppression mappings uses the same format as
+:doc:`SanitizerSpecialCaseList`.
+
+Users can mention sections to describe which diagnostic group behaviours to
+change. Sections are denoted as ``[unused]`` in this format. Each section name
+must match a diagnostic group.
+When a diagnostic is matched by multiple groups, the latest one takes

bricknerb wrote:

After seeing the below text and example, I think it's better to say that a 
diagnostic "belongs to" multiple groups.
When you say "matched", I assumed you mean the diagnostic has matching global 
in multiple group sections.

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


[clang] [clang] Make source locations space usage diagnostics numbers easier to read (PR #114999)

2024-11-05 Thread Boaz Brickner via cfe-commits

https://github.com/bricknerb created 
https://github.com/llvm/llvm-project/pull/114999

Instead of writing "12345678B", write "12345678B (12.34MB)".

>From 6f3c9f95f7ad558659bc7d868ab4d5e5f6af05c0 Mon Sep 17 00:00:00 2001
From: Boaz Brickner 
Date: Tue, 5 Nov 2024 15:29:10 +0100
Subject: [PATCH] [clang] Make source locations space usage diagnostics numbers
 easier to read Instead of write "12345678B", write "12345678B (12.34MB)".

---
 .../clang/Basic/DiagnosticCommonKinds.td  | 11 ---
 clang/lib/Basic/SourceManager.cpp | 33 +--
 clang/test/Lexer/SourceLocationsOverflow.c| 10 +++---
 clang/test/Misc/sloc-usage.cpp|  4 +--
 4 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index ae709e45a700a1..457abea0b81471 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -389,13 +389,14 @@ def remark_sloc_usage : Remark<
   "source manager location address space usage:">,
   InGroup>, DefaultRemark, ShowInSystemHeader;
 def note_total_sloc_usage : Note<
-  "%0B in local locations, %1B in locations loaded from AST files, for a total 
"
-  "of %2B (%3%% of available space)">;
+  "%0B (%1B) in local locations, %2B (%3B) "
+  "in locations loaded from AST files, for a total of %4B (%5B) "
+  "(%6%% of available space)">;
 def note_file_sloc_usage : Note<
-  "file entered %0 time%s0 using %1B of space"
-  "%plural{0:|: plus %2B for macro expansions}2">;
+  "file entered %0 time%s0 using %1B (%2B) of space"
+  "%plural{0:|: plus %3B (%4B) for macro expansions}3">;
 def note_file_misc_sloc_usage : Note<
-  "%0 additional files entered using a total of %1B of space">;
+  "%0 additional files entered using a total of %1B (%2B) of space">;
 
 // Modules
 def err_module_format_unhandled : Error<
diff --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index 65a8a7253e054f..cbc2b840150321 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -2227,6 +2227,28 @@ LLVM_DUMP_METHOD void SourceManager::dump() const {
   }
 }
 
+static std::string NumberToHumanString(uint64_t number) {
+  static constexpr std::array, 4> Units = {
+  {{1'000'000'000'000UL, 'T'},
+   {1'000'000'000UL, 'G'},
+   {1'000'000UL, 'M'},
+   {1'000UL, 'k'}}};
+
+  std::string human_string;
+  llvm::raw_string_ostream human_string_stream(human_string);
+  for (const auto &[UnitSize, UnitSign] : Units) {
+if (number >= UnitSize) {
+  human_string_stream << llvm::format(
+  "%.2f%c", number / static_cast(UnitSize), UnitSign);
+  break;
+}
+  }
+  if (human_string.empty()) {
+human_string_stream << number;
+  }
+  return human_string;
+}
+
 void SourceManager::noteSLocAddressSpaceUsage(
 DiagnosticsEngine &Diag, std::optional MaxNotes) const {
   struct Info {
@@ -2296,7 +2318,9 @@ void SourceManager::noteSLocAddressSpaceUsage(
   int UsagePercent = static_cast(100.0 * double(LocalUsage + LoadedUsage) 
/
   MaxLoadedOffset);
   Diag.Report(SourceLocation(), diag::note_total_sloc_usage)
-<< LocalUsage << LoadedUsage << (LocalUsage + LoadedUsage) << UsagePercent;
+  << LocalUsage << NumberToHumanString(LocalUsage) << LoadedUsage
+  << NumberToHumanString(LoadedUsage) << (LocalUsage + LoadedUsage)
+  << NumberToHumanString(LocalUsage + LoadedUsage) << UsagePercent;
 
   // Produce notes on sloc address space usage for each file with a high usage.
   uint64_t ReportedSize = 0;
@@ -2304,14 +2328,17 @@ void SourceManager::noteSLocAddressSpaceUsage(
llvm::make_range(SortedUsage.begin(), SortedEnd)) {
 Diag.Report(FileInfo.Loc, diag::note_file_sloc_usage)
 << FileInfo.Inclusions << FileInfo.DirectSize
-<< (FileInfo.TotalSize - FileInfo.DirectSize);
+<< NumberToHumanString(FileInfo.DirectSize)
+<< (FileInfo.TotalSize - FileInfo.DirectSize)
+<< NumberToHumanString(FileInfo.TotalSize - FileInfo.DirectSize);
 ReportedSize += FileInfo.TotalSize;
   }
 
   // Describe any remaining usage not reported in the per-file usage.
   if (ReportedSize != CountedSize) {
 Diag.Report(SourceLocation(), diag::note_file_misc_sloc_usage)
-<< (SortedUsage.end() - SortedEnd) << CountedSize - ReportedSize;
+<< (SortedUsage.end() - SortedEnd) << CountedSize - ReportedSize
+<< NumberToHumanString(CountedSize - ReportedSize);
   }
 }
 
diff --git a/clang/test/Lexer/SourceLocationsOverflow.c 
b/clang/test/Lexer/SourceLocationsOverflow.c
index f058c09428e6e7..26b0d204c49ff5 100644
--- a/clang/test/Lexer/SourceLocationsOverflow.c
+++ b/clang/test/Lexer/SourceLocationsOverflow.c
@@ -3,17 +3,17 @@
 // CHECK-NEXT: inc1.h{{.*}}: fatal error: translation unit is too large for 
Clang to process: ran out of source location

[clang] [clang] Make source locations space usage diagnostics numbers easier to read (PR #114999)

2024-11-05 Thread Boaz Brickner via cfe-commits

bricknerb wrote:

> Do we really care about the exact byte numbers? Maybe we should only show the 
> human-friendly version? It's appealing to have less noise if we can.

Yes, I was considering both options.
It might be useful to see the full number in case you want to diff between 
logs, and the diff would be relatively small (people might care about zero diff 
vs. tiny diff), so I decided to make this change not lose any information.

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


  1   2   3   4   >