Skip to content

Commit 794edd1

Browse files
authored
[clang] Suppress noreturn warning if last statement in a function is a throw (#145166)
Fixes #144952
1 parent 07f1502 commit 794edd1

File tree

7 files changed

+132
-3
lines changed

7 files changed

+132
-3
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,13 @@ Improvements to Clang's diagnostics
649649
#GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490,
650650
#GH36703, #GH32903, #GH23312, #GH69874.
651651

652+
- Clang now avoids issuing `-Wreturn-type` warnings in some cases where
653+
the final statement of a non-void function is a `throw` expression, or
654+
a call to a function that is trivially known to always throw (i.e., its
655+
body consists solely of a `throw` statement). This avoids certain
656+
false positives in exception-heavy code, though only simple patterns
657+
are currently recognized.
658+
652659

653660
Improvements to Clang's time-trace
654661
----------------------------------

clang/include/clang/Basic/Attr.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,13 @@ def AnalyzerNoReturn : InheritableAttr {
965965
let Documentation = [Undocumented];
966966
}
967967

968+
def InferredNoReturn : InheritableAttr {
969+
let Spellings = [];
970+
let SemaHandler = 0;
971+
let Subjects = SubjectList<[Function], ErrorDiag>;
972+
let Documentation = [InternalOnly];
973+
}
974+
968975
def Annotate : InheritableParamOrStmtAttr {
969976
let Spellings = [Clang<"annotate">];
970977
let Args = [StringArgument<"Annotation">, VariadicExprArgument<"Args">];

clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,8 @@ enum class CCEKind {
834834
///< message.
835835
};
836836

837+
void inferNoReturnAttr(Sema &S, const Decl *D);
838+
837839
/// Sema - This implements semantic analysis and AST building for C.
838840
/// \nosubgrouping
839841
class Sema final : public SemaBase {

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
643643
ReturnsVoid = CBody->getFallthroughHandler() != nullptr;
644644
else
645645
ReturnsVoid = FD->getReturnType()->isVoidType();
646-
HasNoReturn = FD->isNoReturn();
646+
HasNoReturn = FD->isNoReturn() || FD->hasAttr<InferredNoReturnAttr>();
647647
}
648648
else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
649649
ReturnsVoid = MD->getReturnType()->isVoidType();
@@ -681,6 +681,28 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
681681
if (CD.diag_FallThrough_HasNoReturn)
682682
S.Diag(RBrace, CD.diag_FallThrough_HasNoReturn) << CD.FunKind;
683683
} else if (!ReturnsVoid && CD.diag_FallThrough_ReturnsNonVoid) {
684+
// If the final statement is a call to an always-throwing function,
685+
// don't warn about the fall-through.
686+
if (const auto *FD = D->getAsFunction()) {
687+
if (const auto *CS = dyn_cast<CompoundStmt>(Body);
688+
CS && !CS->body_empty()) {
689+
const Stmt *LastStmt = CS->body_back();
690+
// Unwrap ExprWithCleanups if necessary.
691+
if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) {
692+
LastStmt = EWC->getSubExpr();
693+
}
694+
if (const auto *CE = dyn_cast<CallExpr>(LastStmt)) {
695+
if (const FunctionDecl *Callee = CE->getDirectCallee();
696+
Callee && Callee->hasAttr<InferredNoReturnAttr>()) {
697+
return; // Don't warn about fall-through.
698+
}
699+
}
700+
// Direct throw.
701+
if (isa<CXXThrowExpr>(LastStmt)) {
702+
return; // Don't warn about fall-through.
703+
}
704+
}
705+
}
684706
bool NotInAllControlPaths = FallThroughType == MaybeFallThrough;
685707
S.Diag(RBrace, CD.diag_FallThrough_ReturnsNonVoid)
686708
<< CD.FunKind << NotInAllControlPaths;

clang/lib/Sema/Sema.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,9 +2434,10 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
24342434
OpenMP().popOpenMPFunctionRegion(Scope.get());
24352435

24362436
// Issue any analysis-based warnings.
2437-
if (WP && D)
2437+
if (WP && D) {
2438+
inferNoReturnAttr(*this, D);
24382439
AnalysisWarnings.IssueWarnings(*WP, Scope.get(), D, BlockType);
2439-
else
2440+
} else
24402441
for (const auto &PUD : Scope->PossiblyUnreachableDiags)
24412442
Diag(PUD.Loc, PUD.PD);
24422443

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "clang/Sema/ParsedAttr.h"
4141
#include "clang/Sema/Scope.h"
4242
#include "clang/Sema/ScopeInfo.h"
43+
#include "clang/Sema/Sema.h"
4344
#include "clang/Sema/SemaAMDGPU.h"
4445
#include "clang/Sema/SemaARM.h"
4546
#include "clang/Sema/SemaAVR.h"
@@ -1938,6 +1939,49 @@ static void handleNakedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
19381939
D->addAttr(::new (S.Context) NakedAttr(S.Context, AL));
19391940
}
19401941

1942+
// FIXME: This is a best-effort heuristic.
1943+
// Currently only handles single throw expressions (optionally with
1944+
// ExprWithCleanups). We could expand this to perform control-flow analysis for
1945+
// more complex patterns.
1946+
static bool isKnownToAlwaysThrow(const FunctionDecl *FD) {
1947+
if (!FD->hasBody())
1948+
return false;
1949+
const Stmt *Body = FD->getBody();
1950+
const Stmt *OnlyStmt = nullptr;
1951+
1952+
if (const auto *Compound = dyn_cast<CompoundStmt>(Body)) {
1953+
if (Compound->size() != 1)
1954+
return false; // More than one statement, can't be known to always throw.
1955+
OnlyStmt = *Compound->body_begin();
1956+
} else {
1957+
OnlyStmt = Body;
1958+
}
1959+
1960+
// Unwrap ExprWithCleanups if necessary.
1961+
if (const auto *EWC = dyn_cast<ExprWithCleanups>(OnlyStmt)) {
1962+
OnlyStmt = EWC->getSubExpr();
1963+
}
1964+
// Check if the only statement is a throw expression.
1965+
return isa<CXXThrowExpr>(OnlyStmt);
1966+
}
1967+
1968+
void clang::inferNoReturnAttr(Sema &S, const Decl *D) {
1969+
auto *FD = dyn_cast<FunctionDecl>(D);
1970+
if (!FD)
1971+
return;
1972+
1973+
auto *NonConstFD = const_cast<FunctionDecl *>(FD);
1974+
DiagnosticsEngine &Diags = S.getDiagnostics();
1975+
if (Diags.isIgnored(diag::warn_falloff_nonvoid, FD->getLocation()) &&
1976+
Diags.isIgnored(diag::warn_suggest_noreturn_function, FD->getLocation()))
1977+
return;
1978+
1979+
if (!FD->hasAttr<NoReturnAttr>() && !FD->hasAttr<InferredNoReturnAttr>() &&
1980+
isKnownToAlwaysThrow(FD)) {
1981+
NonConstFD->addAttr(InferredNoReturnAttr::CreateImplicit(S.Context));
1982+
}
1983+
}
1984+
19411985
static void handleNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &Attrs) {
19421986
if (hasDeclarator(D)) return;
19431987

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -fexceptions -Wreturn-type -verify %s
2+
// expected-no-diagnostics
3+
4+
namespace std {
5+
class string {
6+
public:
7+
string(const char*); // constructor for runtime_error
8+
};
9+
class runtime_error {
10+
public:
11+
runtime_error(const string &);
12+
};
13+
}
14+
15+
// Non-template version.
16+
17+
void throwError(const std::string& msg) {
18+
throw std::runtime_error(msg);
19+
}
20+
21+
int ensureZero(const int i) {
22+
if (i == 0) return 0;
23+
throwError("ERROR"); // no-warning
24+
}
25+
26+
int alwaysThrows() {
27+
throw std::runtime_error("This function always throws"); // no-warning
28+
}
29+
30+
// Template version.
31+
32+
template<typename T>
33+
void throwErrorTemplate(const T& msg) {
34+
throw msg;
35+
}
36+
37+
template <typename T>
38+
int ensureZeroTemplate(T i) {
39+
if (i == 0) return 0;
40+
throwErrorTemplate("ERROR"); // no-warning
41+
}
42+
43+
void testTemplates() {
44+
throwErrorTemplate("ERROR");
45+
(void)ensureZeroTemplate(42);
46+
}

0 commit comments

Comments
 (0)