From 9ebb618d03cb29c37e3178428dcf52e1ac4f1cc2 Mon Sep 17 00:00:00 2001 From: foxtran <39676482+foxtran@users.noreply.github.com> Date: Wed, 19 Feb 2025 19:05:44 +0100 Subject: [PATCH 1/3] [Clang] [Sema] Combine fallout warnings to just one warning (#127546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This merges several falloff and noreturn-related warnings and removes unused diagnostic arguments. Changes: - `warn_maybe_falloff_nonvoid_function` and `warn_falloff_nonvoid_function`, `warn_maybe_falloff_nonvoid_coroutine` and `warn_falloff_nonvoid_coroutine`, `warn_maybe_falloff_nonvoid_lambda` and `warn_falloff_nonvoid_lambda` were combined into `warn_falloff_nonvoid`, - `err_maybe_falloff_nonvoid_block` and `err_falloff_nonvoid_block` were combined into `err_falloff_nonvoid` - `err_noreturn_block_has_return_expr` and `err_noreturn_lambda_has_return_expr` were merged into `err_noreturn_has_return_expr` with the same semantics as `warn_falloff_nonvoid` or `err_falloff_nonvoid`. - Removed some diagnostic args that weren’t being used by the diagnostics. --- .../clang/Basic/DiagnosticSemaKinds.td | 41 ++--- clang/lib/Sema/AnalysisBasedWarnings.cpp | 144 ++++++------------ clang/lib/Sema/SemaStmt.cpp | 6 +- 3 files changed, 66 insertions(+), 125 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ee1ad214d81df..feef50812eca9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -713,13 +713,14 @@ 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; -def warn_falloff_nonvoid_function : Warning< - "non-void function does not return a value">, +def warn_falloff_nonvoid : Warning< + "non-void " + "%enum_select{%Function{function}|%Block{block}|%Lambda{lambda}|%Coroutine{coroutine}}0" + " does not return a value%select{| in all control paths}1">, InGroup; +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; @@ -727,16 +728,6 @@ def warn_pure_function_returns_void : Warning< "'%select{pure|const}0' attribute on function returning 'void'; attribute ignored">, InGroup; -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; -def warn_falloff_nonvoid_coroutine : Warning< - "non-void coroutine does not return a value">, - InGroup; def warn_suggest_noreturn_function : Warning< "%select{function|method}0 %1 could be declared with attribute 'noreturn'">, InGroup, DefaultIgnore; @@ -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; - def warn_falloff_nonvoid_lambda : Warning< - "non-void lambda does not return a value">, - InGroup; 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 @@ -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; -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; +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; -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">; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index ce7d9be8d2faa..f21e571e6e0ce 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -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. @@ -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. @@ -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(D)) { if (const auto *CBody = dyn_cast(Body)) @@ -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(D)) { - S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD; - } else if (const ObjCMethodDecl *MD = dyn_cast(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(D)) { + S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD; + } else if (const ObjCMethodDecl *MD = dyn_cast(D)) { + S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 1 << MD; + } else { + S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn); } - break; - case NeverFallThrough: - break; + } + break; + case NeverFallThrough: + break; } } @@ -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 diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 0394edb7889ba..d0b713f074c33 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3590,7 +3590,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, if (auto *CurBlock = dyn_cast(CurCap)) { if (CurBlock->FunctionType->castAs()->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(CurCap)) { @@ -3601,7 +3602,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, if (CurLambda->CallOperator->getType() ->castAs() ->getNoReturnAttr()) { - Diag(ReturnLoc, diag::err_noreturn_lambda_has_return_expr); + Diag(ReturnLoc, diag::err_noreturn_has_return_expr) + << diag::FalloffFunctionKind::Lambda; return StmtError(); } } From 2bf473bd546e65f8fc2f0d5006b8c8ef07259e24 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 19 Feb 2025 10:17:07 -0800 Subject: [PATCH 2/3] [GlobalOpt] Don't query TTI on a llvm.memcpy declaration. (#127760) Querying TTI creates a Subtarget object, but an llvm.memcpy declaration doesn't have target-cpu and target-feature attributes like functions with definitions. This can cause a warning to be printed on RISC-V because the target-abi in the Module requires floating point, but the subtarget features don't enable floating point. So far we've only seen this in LTO when an -mcpu is not supplied for the TargetMachine. To fix this, get TTI for the calling function instead. Fixes the issue reported here https://github.com/llvm/llvm-project/issues/69780#issuecomment-2665273161 --- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index 9586fc97a39f7..1a2a27d22ae68 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -2186,8 +2186,10 @@ static bool tryWidenGlobalArraysUsedByMemcpy( if (NumElementsToCopy != DZSize || DZSize != SZSize) continue; - unsigned NumBytesToPad = GetTTI(*F).getNumBytesToPadGlobalArray( - NumBytesToCopy, SourceDataArray->getType()); + unsigned NumBytesToPad = + GetTTI(*CI->getFunction()) + .getNumBytesToPadGlobalArray(NumBytesToCopy, + SourceDataArray->getType()); if (NumBytesToPad) { return tryWidenGlobalArrayAndDests(F, GV, NumBytesToPad, NumBytesToCopy, BytesToCopyOp, SourceDataArray); From a6f48ed01292d0007e19a2605cba1acd4ecd123a Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 19 Feb 2025 10:19:40 -0800 Subject: [PATCH 3/3] [MC] Remove MCRegister::isStackSlot. (#127755) Stack slots should only be stored in Register. The only caller was Register::isStackSlot so just inline it there. --- llvm/include/llvm/CodeGen/Register.h | 5 +++-- llvm/include/llvm/MC/MCRegister.h | 8 -------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/CodeGen/Register.h b/llvm/include/llvm/CodeGen/Register.h index 8a0bf3dc71ad2..ad05368bea6a4 100644 --- a/llvm/include/llvm/CodeGen/Register.h +++ b/llvm/include/llvm/CodeGen/Register.h @@ -42,11 +42,12 @@ class Register { /// /// FIXME: remove in favor of member. static constexpr bool isStackSlot(unsigned Reg) { - return MCRegister::isStackSlot(Reg); + return MCRegister::FirstStackSlot <= Reg && + Reg < MCRegister::VirtualRegFlag; } /// Return true if this is a stack slot. - constexpr bool isStack() const { return MCRegister::isStackSlot(Reg); } + constexpr bool isStack() const { return isStackSlot(Reg); } /// Compute the frame index from a register value representing a stack slot. static int stackSlot2Index(Register Reg) { diff --git a/llvm/include/llvm/MC/MCRegister.h b/llvm/include/llvm/MC/MCRegister.h index 53005bb03c2ee..16d0709753b35 100644 --- a/llvm/include/llvm/MC/MCRegister.h +++ b/llvm/include/llvm/MC/MCRegister.h @@ -54,14 +54,6 @@ class MCRegister { static constexpr unsigned FirstStackSlot = 1u << 30; static constexpr unsigned VirtualRegFlag = 1u << 31; - /// This is the portion of the positive number space that is not a physical - /// register. StackSlot values do not exist in the MC layer, see - /// Register::isStackSlot() for the more information on them. - /// - static constexpr bool isStackSlot(unsigned Reg) { - return FirstStackSlot <= Reg && Reg < VirtualRegFlag; - } - /// Return true if the specified register number is in /// the physical register namespace. static constexpr bool isPhysicalRegister(unsigned Reg) {