[clang] Make [[clang::lifetimebound]] work for expressions coming from default arguments (PR #112047)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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