Skip to content

[clang][Sema] Combine fallout warnings to just one warning #127546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Feb 19, 2025
41 changes: 13 additions & 28 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -713,30 +713,21 @@ def err_thread_non_global : Error<
def err_thread_unsupported : Error<
"thread-local storage is not supported for the current target">;

// FIXME: Combine fallout warnings to just one warning.
def warn_maybe_falloff_nonvoid_function : Warning<
"non-void function does not return a value in all control paths">,
InGroup<ReturnType>;
def warn_falloff_nonvoid_function : Warning<
"non-void function does not return a value">,
def warn_falloff_nonvoid : Warning<
"non-void "
"%enum_select<FalloffFunctionKind>{%Function{function}|%Block{block}|%Lambda{lambda}|%Coroutine{coroutine}}0"
" does not return a value%select{| in all control paths}1">,
InGroup<ReturnType>;
def err_falloff_nonvoid : Error<
"non-void %select{function|block|lambda|coroutine}0 "
"does not return a value%select{| in all control paths}1">;
def warn_const_attr_with_pure_attr : Warning<
"'const' attribute imposes more restrictions; 'pure' attribute ignored">,
InGroup<IgnoredAttributes>;
def warn_pure_function_returns_void : Warning<
"'%select{pure|const}0' attribute on function returning 'void'; attribute ignored">,
InGroup<IgnoredAttributes>;

def err_maybe_falloff_nonvoid_block : Error<
"non-void block does not return a value in all control paths">;
def err_falloff_nonvoid_block : Error<
"non-void block does not return a value">;
def warn_maybe_falloff_nonvoid_coroutine : Warning<
"non-void coroutine does not return a value in all control paths">,
InGroup<ReturnType>;
def warn_falloff_nonvoid_coroutine : Warning<
"non-void coroutine does not return a value">,
InGroup<ReturnType>;
def warn_suggest_noreturn_function : Warning<
"%select{function|method}0 %1 could be declared with attribute 'noreturn'">,
InGroup<MissingNoreturn>, DefaultIgnore;
Expand Down Expand Up @@ -8406,14 +8397,6 @@ let CategoryName = "Lambda Issue" in {
"lambda expression in default argument cannot capture any entity">;
def err_lambda_incomplete_result : Error<
"incomplete result type %0 in lambda expression">;
def err_noreturn_lambda_has_return_expr : Error<
"lambda declared 'noreturn' should not return">;
def warn_maybe_falloff_nonvoid_lambda : Warning<
"non-void lambda does not return a value in all control paths">,
InGroup<ReturnType>;
def warn_falloff_nonvoid_lambda : Warning<
"non-void lambda does not return a value">,
InGroup<ReturnType>;
def err_access_lambda_capture : Error<
// The ERRORs represent other special members that aren't constructors, in
// hopes that someone will bother noticing and reporting if they appear
Expand Down Expand Up @@ -10603,14 +10586,16 @@ def err_ctor_dtor_returns_void : Error<
def warn_noreturn_function_has_return_expr : Warning<
"function %0 declared 'noreturn' should not return">,
InGroup<InvalidNoreturn>;
def warn_falloff_noreturn_function : Warning<
"function declared 'noreturn' should not return">,
def warn_noreturn_has_return_expr : Warning<
"%select{function|block|lambda|coroutine}0 "
"declared 'noreturn' should not return">,
InGroup<InvalidNoreturn>;
def err_noreturn_has_return_expr : Error<
"%select{function|block|lambda|coroutine}0 "
"declared 'noreturn' should not return">;
def warn_noreturn_coroutine : Warning<
"coroutine %0 cannot be declared 'noreturn' as it always returns a coroutine handle">,
InGroup<InvalidNoreturn>;
def err_noreturn_block_has_return_expr : Error<
"block declared 'noreturn' should not return">;
def err_carries_dependency_missing_on_first_decl : Error<
"%select{function|parameter}0 declared '[[carries_dependency]]' "
"after its first declaration">;
Expand Down
144 changes: 49 additions & 95 deletions clang/lib/Sema/AnalysisBasedWarnings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -544,25 +544,17 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
namespace {

struct CheckFallThroughDiagnostics {
unsigned diag_MaybeFallThrough_HasNoReturn;
unsigned diag_MaybeFallThrough_ReturnsNonVoid;
unsigned diag_AlwaysFallThrough_HasNoReturn;
unsigned diag_AlwaysFallThrough_ReturnsNonVoid;
unsigned diag_NeverFallThroughOrReturn;
enum { Function, Block, Lambda, Coroutine } funMode;
unsigned diag_FallThrough_HasNoReturn = 0;
unsigned diag_FallThrough_ReturnsNonVoid = 0;
unsigned diag_NeverFallThroughOrReturn = 0;
unsigned FunKind; // TODO: use diag::FalloffFunctionKind
SourceLocation FuncLoc;

static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) {
CheckFallThroughDiagnostics D;
D.FuncLoc = Func->getLocation();
D.diag_MaybeFallThrough_HasNoReturn =
diag::warn_falloff_noreturn_function;
D.diag_MaybeFallThrough_ReturnsNonVoid =
diag::warn_maybe_falloff_nonvoid_function;
D.diag_AlwaysFallThrough_HasNoReturn =
diag::warn_falloff_noreturn_function;
D.diag_AlwaysFallThrough_ReturnsNonVoid =
diag::warn_falloff_nonvoid_function;
D.diag_FallThrough_HasNoReturn = diag::warn_noreturn_has_return_expr;
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;

// Don't suggest that virtual functions be marked "noreturn", since they
// might be overridden by non-noreturn functions.
Expand All @@ -576,76 +568,49 @@ struct CheckFallThroughDiagnostics {
isTemplateInstantiation = Function->isTemplateInstantiation();

if (!isVirtualMethod && !isTemplateInstantiation)
D.diag_NeverFallThroughOrReturn =
diag::warn_suggest_noreturn_function;
else
D.diag_NeverFallThroughOrReturn = 0;
D.diag_NeverFallThroughOrReturn = diag::warn_suggest_noreturn_function;

D.funMode = Function;
D.FunKind = diag::FalloffFunctionKind::Function;
return D;
}

static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) {
CheckFallThroughDiagnostics D;
D.FuncLoc = Func->getLocation();
D.diag_MaybeFallThrough_HasNoReturn = 0;
D.diag_MaybeFallThrough_ReturnsNonVoid =
diag::warn_maybe_falloff_nonvoid_coroutine;
D.diag_AlwaysFallThrough_HasNoReturn = 0;
D.diag_AlwaysFallThrough_ReturnsNonVoid =
diag::warn_falloff_nonvoid_coroutine;
D.diag_NeverFallThroughOrReturn = 0;
D.funMode = Coroutine;
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;
D.FunKind = diag::FalloffFunctionKind::Coroutine;
return D;
}

static CheckFallThroughDiagnostics MakeForBlock() {
CheckFallThroughDiagnostics D;
D.diag_MaybeFallThrough_HasNoReturn =
diag::err_noreturn_block_has_return_expr;
D.diag_MaybeFallThrough_ReturnsNonVoid =
diag::err_maybe_falloff_nonvoid_block;
D.diag_AlwaysFallThrough_HasNoReturn =
diag::err_noreturn_block_has_return_expr;
D.diag_AlwaysFallThrough_ReturnsNonVoid =
diag::err_falloff_nonvoid_block;
D.diag_NeverFallThroughOrReturn = 0;
D.funMode = Block;
D.diag_FallThrough_HasNoReturn = diag::err_noreturn_has_return_expr;
D.diag_FallThrough_ReturnsNonVoid = diag::err_falloff_nonvoid;
D.FunKind = diag::FalloffFunctionKind::Block;
return D;
}

static CheckFallThroughDiagnostics MakeForLambda() {
CheckFallThroughDiagnostics D;
D.diag_MaybeFallThrough_HasNoReturn =
diag::err_noreturn_lambda_has_return_expr;
D.diag_MaybeFallThrough_ReturnsNonVoid =
diag::warn_maybe_falloff_nonvoid_lambda;
D.diag_AlwaysFallThrough_HasNoReturn =
diag::err_noreturn_lambda_has_return_expr;
D.diag_AlwaysFallThrough_ReturnsNonVoid =
diag::warn_falloff_nonvoid_lambda;
D.diag_NeverFallThroughOrReturn = 0;
D.funMode = Lambda;
D.diag_FallThrough_HasNoReturn = diag::err_noreturn_has_return_expr;
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;
D.FunKind = diag::FalloffFunctionKind::Lambda;
return D;
}

bool checkDiagnostics(DiagnosticsEngine &D, bool ReturnsVoid,
bool HasNoReturn) const {
if (funMode == Function) {
if (FunKind == diag::FalloffFunctionKind::Function) {
return (ReturnsVoid ||
D.isIgnored(diag::warn_maybe_falloff_nonvoid_function,
FuncLoc)) &&
D.isIgnored(diag::warn_falloff_nonvoid, FuncLoc)) &&
(!HasNoReturn ||
D.isIgnored(diag::warn_noreturn_function_has_return_expr,
FuncLoc)) &&
D.isIgnored(diag::warn_noreturn_has_return_expr, FuncLoc)) &&
(!ReturnsVoid ||
D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc));
}
if (funMode == Coroutine) {
if (FunKind == diag::FalloffFunctionKind::Coroutine) {
return (ReturnsVoid ||
D.isIgnored(diag::warn_maybe_falloff_nonvoid_function, FuncLoc) ||
D.isIgnored(diag::warn_maybe_falloff_nonvoid_coroutine,
FuncLoc)) &&
D.isIgnored(diag::warn_falloff_nonvoid, FuncLoc)) &&
(!HasNoReturn);
}
// For blocks / lambdas.
Expand All @@ -662,12 +627,10 @@ struct CheckFallThroughDiagnostics {
static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
QualType BlockType,
const CheckFallThroughDiagnostics &CD,
AnalysisDeclContext &AC,
sema::FunctionScopeInfo *FSI) {
AnalysisDeclContext &AC) {

bool ReturnsVoid = false;
bool HasNoReturn = false;
bool IsCoroutine = FSI->isCoroutine();

if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
if (const auto *CBody = dyn_cast<CoroutineBodyStmt>(Body))
Expand Down Expand Up @@ -696,49 +659,40 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
return;
SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc();
auto EmitDiag = [&](SourceLocation Loc, unsigned DiagID) {
if (IsCoroutine) {
if (DiagID != 0)
S.Diag(Loc, DiagID) << FSI->CoroutinePromise->getType();
} else {
S.Diag(Loc, DiagID);
}
};

// cpu_dispatch functions permit empty function bodies for ICC compatibility.
if (D->getAsFunction() && D->getAsFunction()->isCPUDispatchMultiVersion())
return;

// Either in a function body compound statement, or a function-try-block.
switch (CheckFallThrough(AC)) {
case UnknownFallThrough:
break;
switch (int FallThroughType = CheckFallThrough(AC)) {
case UnknownFallThrough:
break;

case MaybeFallThrough:
if (HasNoReturn)
EmitDiag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn);
else if (!ReturnsVoid)
EmitDiag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid);
break;
case AlwaysFallThrough:
if (HasNoReturn)
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn);
else if (!ReturnsVoid)
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid);
break;
case NeverFallThroughOrReturn:
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD;
} else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 1 << MD;
} else {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn);
}
case MaybeFallThrough:
case AlwaysFallThrough:
if (HasNoReturn) {
if (CD.diag_FallThrough_HasNoReturn)
S.Diag(RBrace, CD.diag_FallThrough_HasNoReturn) << CD.FunKind;
} else if (!ReturnsVoid && CD.diag_FallThrough_ReturnsNonVoid) {
bool NotInAllControlPaths = FallThroughType == MaybeFallThrough;
S.Diag(RBrace, CD.diag_FallThrough_ReturnsNonVoid)
<< CD.FunKind << NotInAllControlPaths;
}
break;
case NeverFallThroughOrReturn:
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD;
} else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 1 << MD;
} else {
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn);
}
break;
case NeverFallThrough:
break;
}
break;
case NeverFallThrough:
break;
}
}

Expand Down Expand Up @@ -2765,7 +2719,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
: (fscope->isCoroutine()
? CheckFallThroughDiagnostics::MakeForCoroutine(D)
: CheckFallThroughDiagnostics::MakeForFunction(D)));
CheckFallThroughForBody(S, D, Body, BlockType, CD, AC, fscope);
CheckFallThroughForBody(S, D, Body, BlockType, CD, AC);
}

// Warning: check for unreachable code
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3590,7 +3590,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,

if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap)) {
if (CurBlock->FunctionType->castAs<FunctionType>()->getNoReturnAttr()) {
Diag(ReturnLoc, diag::err_noreturn_block_has_return_expr);
Diag(ReturnLoc, diag::err_noreturn_has_return_expr)
<< diag::FalloffFunctionKind::Block;
return StmtError();
}
} else if (auto *CurRegion = dyn_cast<CapturedRegionScopeInfo>(CurCap)) {
Expand All @@ -3601,7 +3602,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
if (CurLambda->CallOperator->getType()
->castAs<FunctionType>()
->getNoReturnAttr()) {
Diag(ReturnLoc, diag::err_noreturn_lambda_has_return_expr);
Diag(ReturnLoc, diag::err_noreturn_has_return_expr)
<< diag::FalloffFunctionKind::Lambda;
return StmtError();
}
}
Expand Down