https://github.com/dougsonos updated https://github.com/llvm/llvm-project/pull/238401
>From e3f854becfed5979de1decb18eeb8854852faeb4 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <[email protected]> Date: Mon, 2 Nov 2026 22:27:58 -0800 Subject: [PATCH 1/2] [Clang] FunctionEffects: accept throw-only callees from noexcept callers If a callee is unsafe for nonblocking/nonallocating only because an exception can escape it, a noexcept caller (where that exception would terminate) no longer gets a violation for calling it. The throw-only status propagates through non-noexcept callers; catching, or any other violation, makes a function not throw-only. --- clang/lib/Sema/SemaFunctionEffects.cpp | 90 +++++++++++++------ .../Sema/attr-nonblocking-constraints.cpp | 49 ++++++++++ .../attr-nonblocking-constraints.mm | 9 ++ 3 files changed, 121 insertions(+), 27 deletions(-) diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp index bb446179e0b9a4..bbc347ac50cdd1 100644 --- a/clang/lib/Sema/SemaFunctionEffects.cpp +++ b/clang/lib/Sema/SemaFunctionEffects.cpp @@ -88,12 +88,17 @@ struct Violation { SourceLocation Loc; const Decl *Callee = nullptr; // Only valid for ViolationIDs Calls{Decl,Expr}WithoutEffect. + // True if the violation is, or is solely due to, an exception which + // propagates out of the function uncaught. A noexcept caller turns such an + // exception into termination, so it need not treat the callee as unsafe. + bool ThrowOnly = false; Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS, SourceLocation Loc, const Decl *Callee = nullptr, - std::optional<FunctionEffect> CalleeEffect = std::nullopt) + std::optional<FunctionEffect> CalleeEffect = std::nullopt, + bool ThrowOnly = false) : Effect(Effect), CalleeEffectPreventingInference(CalleeEffect), ID(ID), - Site(VS), Loc(Loc), Callee(Callee) {} + Site(VS), Loc(Loc), Callee(Callee), ThrowOnly(ThrowOnly) {} unsigned diagnosticSelectIndex() const { return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex); @@ -125,6 +130,18 @@ static bool isNoexcept(const FunctionDecl *FD) { return FPT && (FPT->isNothrow() || FD->hasAttr<NoThrowAttr>()); } +static bool isNoexcept(const Decl *D) { + if (const auto *FD = D->getAsFunction()) + return isNoexcept(FD); + if (const auto *BD = dyn_cast<BlockDecl>(D)) { + if (auto *TSI = BD->getSignatureAsWritten()) { + auto *FPT = TSI->getType()->castAs<FunctionProtoType>(); + return FPT->isNothrow() || BD->hasAttr<NoThrowAttr>(); + } + } + return false; +} + // This list is probably incomplete. // FIXME: Investigate: // __builtin_eh_return? @@ -314,17 +331,27 @@ class EffectToViolationMap { std::unique_ptr<ImplVec> Impl; public: - // Insert a new Violation if we do not already have one for its effect. + // Insert a new Violation if we do not already have one for its effect. A + // throw-only Violation is replaced by any other kind, so that the stored + // Violation is throw-only only if all of the effect's Violations are. void maybeInsert(const Violation &Viol) { if (Impl == nullptr) Impl = std::make_unique<ImplVec>(); - else if (lookup(Viol.Effect) != nullptr) + else if (Violation *Existing = lookupMutable(Viol.Effect)) { + if (Existing->ThrowOnly && !Viol.ThrowOnly) + *Existing = Viol; return; + } Impl->push_back(Viol); } - const Violation *lookup(FunctionEffect Key) { + const Violation *lookup(FunctionEffect Key) { return lookupMutable(Key); } + + size_t size() const { return Impl ? Impl->size() : 0; } + +private: + Violation *lookupMutable(FunctionEffect Key) { if (Impl == nullptr) return nullptr; @@ -332,8 +359,6 @@ class EffectToViolationMap { *Impl, [&](const auto &Item) { return Item.Effect == Key; }); return Iter != Impl->end() ? &*Iter : nullptr; } - - size_t size() const { return Impl ? Impl->size() : 0; } }; // ---------- @@ -484,6 +509,13 @@ class CompleteFunctionAnalysis { return InferrableEffectToFirstViolation.lookup(Effect); } + // True if every reason this function lacks Effect is an exception escaping + // it, i.e. it would have the effect but for throwing. + bool isThrowOnly(FunctionEffect Effect) { + const Violation *V = firstViolationForEffect(Effect); + return V != nullptr && V->ThrowOnly; + } + void dump(llvm::raw_ostream &OS) const { OS << "Complete: Verified "; VerifiedEffects.dump(OS); @@ -630,16 +662,7 @@ class Analyzer { if (!(Effect.flags() & FunctionEffect::FE_ExcludeThrow)) continue; - bool IsNoexcept = false; - if (auto *FD = D->getAsFunction()) { - IsNoexcept = isNoexcept(FD); - } else if (auto *BD = dyn_cast<BlockDecl>(D)) { - if (auto *TSI = BD->getSignatureAsWritten()) { - auto *FPT = TSI->getType()->castAs<FunctionProtoType>(); - IsNoexcept = FPT->isNothrow() || BD->hasAttr<NoThrowAttr>(); - } - } - if (!IsNoexcept) + if (!isNoexcept(D)) S.Diag(D->getBeginLoc(), diag::warn_perf_constraint_implies_noexcept) << GetCallableDeclKind(D, nullptr) << Effect.name(); break; @@ -721,11 +744,12 @@ class Analyzer { bool IsInferencePossible = DirectCall; + CompleteFunctionAnalysis *CalleeAnalysis = nullptr; if (DirectCall) - if (CompleteFunctionAnalysis *CFA = - DeclAnalysis.completedAnalysisForDecl(Callee.CDecl)) { + if ((CalleeAnalysis = + DeclAnalysis.completedAnalysisForDecl(Callee.CDecl))) { // Combine declared effects with those which may have been inferred. - CalleeEffects.insert(CFA->VerifiedEffects); + CalleeEffects.insert(CalleeAnalysis->VerifiedEffects); IsInferencePossible = false; // We've already traversed it. } @@ -752,11 +776,19 @@ class Analyzer { // method/function ptr?), generate a Violation now. if (!IsInferencePossible || !(Effect.flags() & FunctionEffect::FE_InferrableOnCallees)) { - if (Callee.FuncType == SpecialFuncType::None) + if (Callee.FuncType == SpecialFuncType::None) { + // An exception escaping the callee cannot escape a noexcept caller; + // it terminates the program instead. + const bool CalleeThrowOnly = + CalleeAnalysis != nullptr && CalleeAnalysis->isThrowOnly(Effect); + if (CalleeThrowOnly && S.getLangOpts().CPlusPlus && + isNoexcept(Caller.CDecl)) + return; PFA.checkAddViolation(Inferring, {Effect, ViolationID::CallsDeclWithoutEffect, - VSite, CallLoc, Callee.CDecl}); - else + VSite, CallLoc, Callee.CDecl, std::nullopt, + CalleeThrowOnly}); + } else PFA.checkAddViolation( Inferring, {Effect, ViolationID::AllocatesMemory, VSite, CallLoc}); @@ -1027,11 +1059,13 @@ class Analyzer { void diagnoseLanguageConstruct(FunctionEffect::FlagBit Flag, ViolationID VID, SourceLocation Loc, const Decl *Callee = nullptr) { + const bool ThrowOnly = (Flag == FunctionEffect::FE_ExcludeThrow); // If there are any declared verifiable effects which forbid the construct // represented by the flag, store just one violation. for (FunctionEffect Effect : CurrentFunction.DeclaredVerifiableEffects) { if (Effect.flags() & Flag) { - addViolation(/*inferring=*/false, Effect, VID, Loc, Callee); + addViolation(/*inferring=*/false, Effect, VID, Loc, Callee, + ThrowOnly); break; } } @@ -1039,13 +1073,15 @@ class Analyzer { // violation, if we don't already have a violation for that effect. for (FunctionEffect Effect : CurrentFunction.EffectsToInfer) if (Effect.flags() & Flag) - addViolation(/*inferring=*/true, Effect, VID, Loc, Callee); + addViolation(/*inferring=*/true, Effect, VID, Loc, Callee, ThrowOnly); } void addViolation(bool Inferring, FunctionEffect Effect, ViolationID VID, - SourceLocation Loc, const Decl *Callee = nullptr) { + SourceLocation Loc, const Decl *Callee = nullptr, + bool ThrowOnly = false) { CurrentFunction.checkAddViolation( - Inferring, Violation(Effect, VID, VSite, Loc, Callee)); + Inferring, + Violation(Effect, VID, VSite, Loc, Callee, std::nullopt, ThrowOnly)); } // Here we have a call to a Decl, either explicitly via a CallExpr or some diff --git a/clang/test/Sema/attr-nonblocking-constraints.cpp b/clang/test/Sema/attr-nonblocking-constraints.cpp index 0f384aad5c434b..42a00be33ab875 100644 --- a/clang/test/Sema/attr-nonblocking-constraints.cpp +++ b/clang/test/Sema/attr-nonblocking-constraints.cpp @@ -614,6 +614,55 @@ void nb28() [[clang::nonblocking]] { bool x = __is_constructible(int, const int&); } +// --- Callees which are unsafe only because they throw --- +namespace ThrowOnly { + +void throws() { + throw 42; // expected-note {{function cannot be inferred 'nonblocking' because it throws or catches exceptions}} +} +void calls_throws() { throws(); } // expected-note {{function cannot be inferred 'nonblocking' because it calls non-'nonblocking' function 'ThrowOnly::throws'}} + +// A noexcept caller turns the exception into termination. The first call to +// throws() defers inference; later ones see the completed analysis. +void ok1() noexcept [[clang::nonblocking]] { throws(); } +void ok2() noexcept [[clang::nonblocking]] { throws(); } +void ok3() noexcept [[clang::nonblocking]] { calls_throws(); } +void inferred_noexcept() noexcept { throws(); } +void ok4() noexcept [[clang::nonblocking]] { inferred_noexcept(); } + +// Without noexcept, the exception can escape. +void bad1() [[clang::nonblocking]] { + calls_throws(); // expected-warning {{function with 'nonblocking' attribute must not call non-'nonblocking' function 'ThrowOnly::calls_throws'}} +} + +// Not throw-only: caught, or unsafe for another reason too. +void catches() { + try {} catch (...) {} // expected-note {{function cannot be inferred 'nonblocking' because it throws or catches exceptions}} +} +void throws_and_allocates() { + throw 42; + (void)new int; // expected-note {{function cannot be inferred 'nonblocking' because it allocates or deallocates memory}} +} +void bad2() noexcept [[clang::nonblocking]] { + catches(); // expected-warning {{function with 'nonblocking' attribute must not call non-'nonblocking' function 'ThrowOnly::catches'}} + throws_and_allocates(); // expected-warning {{function with 'nonblocking' attribute must not call non-'nonblocking' function 'ThrowOnly::throws_and_allocates'}} +} + +// A throw-only function in a call cycle. +void recurse_b(int); +void recurse_a(int n) { + if (n) + recurse_b(n - 1); + throw 42; // expected-note {{function cannot be inferred 'nonblocking' because it throws or catches exceptions}} +} +void recurse_b(int n) { recurse_a(n); } +void recurse_ok() noexcept [[clang::nonblocking]] { recurse_a(1); } +void recurse_bad() [[clang::nonblocking]] { + recurse_a(1); // expected-warning {{function with 'nonblocking' attribute must not call non-'nonblocking' function 'ThrowOnly::recurse_a'}} +} + +} // namespace ThrowOnly + // --- nonblocking implies noexcept --- #pragma clang diagnostic warning "-Wperf-constraint-implies-noexcept" diff --git a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm index d1c9b3cc98a1c0..7b9e1b9b204c8b 100644 --- a/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm +++ b/clang/test/SemaObjCXX/attr-nonblocking-constraints.mm @@ -37,3 +37,12 @@ void nb5() [[clang::nonblocking]] { @synchronized(someLock) { // expected-warning {{function with 'nonblocking' attribute must not access ObjC methods or properties}} } } + +// An uncaught @throw terminates a noexcept caller, like a C++ throw. +void objc_throws() { + @throw @"foo"; // expected-note {{function cannot be inferred 'nonblocking' because it throws or catches exceptions}} +} +void objc_ok() noexcept [[clang::nonblocking]] { objc_throws(); } +void objc_bad() [[clang::nonblocking]] { + objc_throws(); // expected-warning {{function with 'nonblocking' attribute must not call non-'nonblocking' function 'objc_throws'}} +} >From 63cbaca9435d4f45e73728219ae3dcc120345696 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <[email protected]> Date: Tue, 3 Nov 2026 11:47:09 -0800 Subject: [PATCH 2/2] review comments --- clang/lib/Sema/SemaFunctionEffects.cpp | 28 +++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp index bbc347ac50cdd1..cf6232881ddaa3 100644 --- a/clang/lib/Sema/SemaFunctionEffects.cpp +++ b/clang/lib/Sema/SemaFunctionEffects.cpp @@ -126,18 +126,24 @@ static bool functionIsVerifiable(const FunctionDecl *FD) { } static bool isNoexcept(const FunctionDecl *FD) { + assert(FD != nullptr); const auto *FPT = FD->getType()->getAs<FunctionProtoType>(); return FPT && (FPT->isNothrow() || FD->hasAttr<NoThrowAttr>()); } static bool isNoexcept(const Decl *D) { + assert(D != nullptr); if (const auto *FD = D->getAsFunction()) return isNoexcept(FD); if (const auto *BD = dyn_cast<BlockDecl>(D)) { - if (auto *TSI = BD->getSignatureAsWritten()) { - auto *FPT = TSI->getType()->castAs<FunctionProtoType>(); - return FPT->isNothrow() || BD->hasAttr<NoThrowAttr>(); - } + // The written signature is only the return type when the block has no + // explicit parameter list (e.g. `^int { ... }`), so it may not be a + // function type at all. + if (auto *TSI = BD->getSignatureAsWritten()) + if (const auto *FPT = TSI->getType()->getAs<FunctionProtoType>()) + if (FPT->isNothrow()) + return true; + return BD->hasAttr<NoThrowAttr>(); } return false; } @@ -346,12 +352,16 @@ class EffectToViolationMap { Impl->push_back(Viol); } - const Violation *lookup(FunctionEffect Key) { return lookupMutable(Key); } + const Violation *lookup(FunctionEffect Key) const { + return lookupMutable(Key); + } size_t size() const { return Impl ? Impl->size() : 0; } private: - Violation *lookupMutable(FunctionEffect Key) { + // const so that lookup() can share it: Impl's constness is shallow, so the + // Violations it owns are mutable here. Only maybeInsert() may modify them. + Violation *lookupMutable(FunctionEffect Key) const { if (Impl == nullptr) return nullptr; @@ -440,7 +450,7 @@ class PendingFunctionAnalysis { // Analysis is complete when there are no unverified direct calls. bool isComplete() const { return UnverifiedDirectCalls.empty(); } - const Violation *violationForInferrableEffect(FunctionEffect effect) { + const Violation *violationForInferrableEffect(FunctionEffect effect) const { return InferrableEffectToFirstViolation.lookup(effect); } @@ -505,13 +515,13 @@ class CompleteFunctionAnalysis { std::move(Pending.InferrableEffectToFirstViolation); } - const Violation *firstViolationForEffect(FunctionEffect Effect) { + const Violation *firstViolationForEffect(FunctionEffect Effect) const { return InferrableEffectToFirstViolation.lookup(Effect); } // True if every reason this function lacks Effect is an exception escaping // it, i.e. it would have the effect but for throwing. - bool isThrowOnly(FunctionEffect Effect) { + bool isThrowOnly(FunctionEffect Effect) const { const Violation *V = firstViolationForEffect(Effect); return V != nullptr && V->ThrowOnly; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
