Skip to content

Commit ac4fe8d

Browse files
committed
Address comment
Created using spr 1.3.5
2 parents ff7c5db + 9ebb618 commit ac4fe8d

28 files changed

+428
-502
lines changed

.github/new-prs-labeler.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ clang:static analyzer:
499499
- clang/tools/scan-build/**
500500
- clang/utils/analyzer/**
501501
- clang/docs/analyzer/**
502+
- clang/test/Analysis/**
502503

503504
pgo:
504505
- llvm/lib/Transforms/Instrumentation/CGProfile.cpp

clang-tools-extra/clangd/CollectMacros.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@
1818
namespace clang {
1919
namespace clangd {
2020

21-
Range MacroOccurrence::toRange(const SourceManager &SM) const {
21+
CharSourceRange MacroOccurrence::toSourceRange(const SourceManager &SM) const {
2222
auto MainFile = SM.getMainFileID();
23-
return halfOpenToRange(
24-
SM, syntax::FileRange(MainFile, StartOffset, EndOffset).toCharRange(SM));
23+
return syntax::FileRange(MainFile, StartOffset, EndOffset).toCharRange(SM);
24+
}
25+
26+
Range MacroOccurrence::toRange(const SourceManager &SM) const {
27+
return halfOpenToRange(SM, toSourceRange(SM));
2528
}
2629

2730
void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,

clang-tools-extra/clangd/CollectMacros.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ struct MacroOccurrence {
3131
// True if the occurence is used in a conditional directive, e.g. #ifdef MACRO
3232
bool InConditionalDirective;
3333

34+
CharSourceRange toSourceRange(const SourceManager &SM) const;
3435
Range toRange(const SourceManager &SM) const;
3536
};
3637

clang-tools-extra/clangd/index/SymbolCollector.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,8 @@ void SymbolCollector::handleMacros(const MainFileMacros &MacroRefsToIndex) {
713713
// Add macro references.
714714
for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
715715
for (const auto &MacroRef : IDToRefs.second) {
716-
const auto &Range = MacroRef.toRange(SM);
716+
const auto &SR = MacroRef.toSourceRange(SM);
717+
auto Range = halfOpenToRange(SM, SR);
717718
bool IsDefinition = MacroRef.IsDefinition;
718719
Ref R;
719720
R.Location.Start.setLine(Range.start.line);
@@ -726,9 +727,7 @@ void SymbolCollector::handleMacros(const MainFileMacros &MacroRefsToIndex) {
726727
if (IsDefinition) {
727728
Symbol S;
728729
S.ID = IDToRefs.first;
729-
auto StartLoc = cantFail(sourceLocationInMainFile(SM, Range.start));
730-
auto EndLoc = cantFail(sourceLocationInMainFile(SM, Range.end));
731-
S.Name = toSourceCode(SM, SourceRange(StartLoc, EndLoc));
730+
S.Name = toSourceCode(SM, SR.getAsRange());
732731
S.SymInfo.Kind = index::SymbolKind::Macro;
733732
S.SymInfo.SubKind = index::SymbolSubKind::None;
734733
S.SymInfo.Properties = index::SymbolPropertySet();

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -713,30 +713,21 @@ def err_thread_non_global : Error<
713713
def err_thread_unsupported : Error<
714714
"thread-local storage is not supported for the current target">;
715715

716-
// FIXME: Combine fallout warnings to just one warning.
717-
def warn_maybe_falloff_nonvoid_function : Warning<
718-
"non-void function does not return a value in all control paths">,
719-
InGroup<ReturnType>;
720-
def warn_falloff_nonvoid_function : Warning<
721-
"non-void function does not return a value">,
716+
def warn_falloff_nonvoid : Warning<
717+
"non-void "
718+
"%enum_select<FalloffFunctionKind>{%Function{function}|%Block{block}|%Lambda{lambda}|%Coroutine{coroutine}}0"
719+
" does not return a value%select{| in all control paths}1">,
722720
InGroup<ReturnType>;
721+
def err_falloff_nonvoid : Error<
722+
"non-void %select{function|block|lambda|coroutine}0 "
723+
"does not return a value%select{| in all control paths}1">;
723724
def warn_const_attr_with_pure_attr : Warning<
724725
"'const' attribute imposes more restrictions; 'pure' attribute ignored">,
725726
InGroup<IgnoredAttributes>;
726727
def warn_pure_function_returns_void : Warning<
727728
"'%select{pure|const}0' attribute on function returning 'void'; attribute ignored">,
728729
InGroup<IgnoredAttributes>;
729730

730-
def err_maybe_falloff_nonvoid_block : Error<
731-
"non-void block does not return a value in all control paths">;
732-
def err_falloff_nonvoid_block : Error<
733-
"non-void block does not return a value">;
734-
def warn_maybe_falloff_nonvoid_coroutine : Warning<
735-
"non-void coroutine does not return a value in all control paths">,
736-
InGroup<ReturnType>;
737-
def warn_falloff_nonvoid_coroutine : Warning<
738-
"non-void coroutine does not return a value">,
739-
InGroup<ReturnType>;
740731
def warn_suggest_noreturn_function : Warning<
741732
"%select{function|method}0 %1 could be declared with attribute 'noreturn'">,
742733
InGroup<MissingNoreturn>, DefaultIgnore;
@@ -8406,14 +8397,6 @@ let CategoryName = "Lambda Issue" in {
84068397
"lambda expression in default argument cannot capture any entity">;
84078398
def err_lambda_incomplete_result : Error<
84088399
"incomplete result type %0 in lambda expression">;
8409-
def err_noreturn_lambda_has_return_expr : Error<
8410-
"lambda declared 'noreturn' should not return">;
8411-
def warn_maybe_falloff_nonvoid_lambda : Warning<
8412-
"non-void lambda does not return a value in all control paths">,
8413-
InGroup<ReturnType>;
8414-
def warn_falloff_nonvoid_lambda : Warning<
8415-
"non-void lambda does not return a value">,
8416-
InGroup<ReturnType>;
84178400
def err_access_lambda_capture : Error<
84188401
// The ERRORs represent other special members that aren't constructors, in
84198402
// hopes that someone will bother noticing and reporting if they appear
@@ -10603,14 +10586,16 @@ def err_ctor_dtor_returns_void : Error<
1060310586
def warn_noreturn_function_has_return_expr : Warning<
1060410587
"function %0 declared 'noreturn' should not return">,
1060510588
InGroup<InvalidNoreturn>;
10606-
def warn_falloff_noreturn_function : Warning<
10607-
"function declared 'noreturn' should not return">,
10589+
def warn_noreturn_has_return_expr : Warning<
10590+
"%select{function|block|lambda|coroutine}0 "
10591+
"declared 'noreturn' should not return">,
1060810592
InGroup<InvalidNoreturn>;
10593+
def err_noreturn_has_return_expr : Error<
10594+
"%select{function|block|lambda|coroutine}0 "
10595+
"declared 'noreturn' should not return">;
1060910596
def warn_noreturn_coroutine : Warning<
1061010597
"coroutine %0 cannot be declared 'noreturn' as it always returns a coroutine handle">,
1061110598
InGroup<InvalidNoreturn>;
10612-
def err_noreturn_block_has_return_expr : Error<
10613-
"block declared 'noreturn' should not return">;
1061410599
def err_carries_dependency_missing_on_first_decl : Error<
1061510600
"%select{function|parameter}0 declared '[[carries_dependency]]' "
1061610601
"after its first declaration">;

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 49 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -544,25 +544,17 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
544544
namespace {
545545

546546
struct CheckFallThroughDiagnostics {
547-
unsigned diag_MaybeFallThrough_HasNoReturn;
548-
unsigned diag_MaybeFallThrough_ReturnsNonVoid;
549-
unsigned diag_AlwaysFallThrough_HasNoReturn;
550-
unsigned diag_AlwaysFallThrough_ReturnsNonVoid;
551-
unsigned diag_NeverFallThroughOrReturn;
552-
enum { Function, Block, Lambda, Coroutine } funMode;
547+
unsigned diag_FallThrough_HasNoReturn = 0;
548+
unsigned diag_FallThrough_ReturnsNonVoid = 0;
549+
unsigned diag_NeverFallThroughOrReturn = 0;
550+
unsigned FunKind; // TODO: use diag::FalloffFunctionKind
553551
SourceLocation FuncLoc;
554552

555553
static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) {
556554
CheckFallThroughDiagnostics D;
557555
D.FuncLoc = Func->getLocation();
558-
D.diag_MaybeFallThrough_HasNoReturn =
559-
diag::warn_falloff_noreturn_function;
560-
D.diag_MaybeFallThrough_ReturnsNonVoid =
561-
diag::warn_maybe_falloff_nonvoid_function;
562-
D.diag_AlwaysFallThrough_HasNoReturn =
563-
diag::warn_falloff_noreturn_function;
564-
D.diag_AlwaysFallThrough_ReturnsNonVoid =
565-
diag::warn_falloff_nonvoid_function;
556+
D.diag_FallThrough_HasNoReturn = diag::warn_noreturn_has_return_expr;
557+
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;
566558

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

578570
if (!isVirtualMethod && !isTemplateInstantiation)
579-
D.diag_NeverFallThroughOrReturn =
580-
diag::warn_suggest_noreturn_function;
581-
else
582-
D.diag_NeverFallThroughOrReturn = 0;
571+
D.diag_NeverFallThroughOrReturn = diag::warn_suggest_noreturn_function;
583572

584-
D.funMode = Function;
573+
D.FunKind = diag::FalloffFunctionKind::Function;
585574
return D;
586575
}
587576

588577
static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) {
589578
CheckFallThroughDiagnostics D;
590579
D.FuncLoc = Func->getLocation();
591-
D.diag_MaybeFallThrough_HasNoReturn = 0;
592-
D.diag_MaybeFallThrough_ReturnsNonVoid =
593-
diag::warn_maybe_falloff_nonvoid_coroutine;
594-
D.diag_AlwaysFallThrough_HasNoReturn = 0;
595-
D.diag_AlwaysFallThrough_ReturnsNonVoid =
596-
diag::warn_falloff_nonvoid_coroutine;
597-
D.diag_NeverFallThroughOrReturn = 0;
598-
D.funMode = Coroutine;
580+
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;
581+
D.FunKind = diag::FalloffFunctionKind::Coroutine;
599582
return D;
600583
}
601584

602585
static CheckFallThroughDiagnostics MakeForBlock() {
603586
CheckFallThroughDiagnostics D;
604-
D.diag_MaybeFallThrough_HasNoReturn =
605-
diag::err_noreturn_block_has_return_expr;
606-
D.diag_MaybeFallThrough_ReturnsNonVoid =
607-
diag::err_maybe_falloff_nonvoid_block;
608-
D.diag_AlwaysFallThrough_HasNoReturn =
609-
diag::err_noreturn_block_has_return_expr;
610-
D.diag_AlwaysFallThrough_ReturnsNonVoid =
611-
diag::err_falloff_nonvoid_block;
612-
D.diag_NeverFallThroughOrReturn = 0;
613-
D.funMode = Block;
587+
D.diag_FallThrough_HasNoReturn = diag::err_noreturn_has_return_expr;
588+
D.diag_FallThrough_ReturnsNonVoid = diag::err_falloff_nonvoid;
589+
D.FunKind = diag::FalloffFunctionKind::Block;
614590
return D;
615591
}
616592

617593
static CheckFallThroughDiagnostics MakeForLambda() {
618594
CheckFallThroughDiagnostics D;
619-
D.diag_MaybeFallThrough_HasNoReturn =
620-
diag::err_noreturn_lambda_has_return_expr;
621-
D.diag_MaybeFallThrough_ReturnsNonVoid =
622-
diag::warn_maybe_falloff_nonvoid_lambda;
623-
D.diag_AlwaysFallThrough_HasNoReturn =
624-
diag::err_noreturn_lambda_has_return_expr;
625-
D.diag_AlwaysFallThrough_ReturnsNonVoid =
626-
diag::warn_falloff_nonvoid_lambda;
627-
D.diag_NeverFallThroughOrReturn = 0;
628-
D.funMode = Lambda;
595+
D.diag_FallThrough_HasNoReturn = diag::err_noreturn_has_return_expr;
596+
D.diag_FallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid;
597+
D.FunKind = diag::FalloffFunctionKind::Lambda;
629598
return D;
630599
}
631600

632601
bool checkDiagnostics(DiagnosticsEngine &D, bool ReturnsVoid,
633602
bool HasNoReturn) const {
634-
if (funMode == Function) {
603+
if (FunKind == diag::FalloffFunctionKind::Function) {
635604
return (ReturnsVoid ||
636-
D.isIgnored(diag::warn_maybe_falloff_nonvoid_function,
637-
FuncLoc)) &&
605+
D.isIgnored(diag::warn_falloff_nonvoid, FuncLoc)) &&
638606
(!HasNoReturn ||
639-
D.isIgnored(diag::warn_noreturn_function_has_return_expr,
640-
FuncLoc)) &&
607+
D.isIgnored(diag::warn_noreturn_has_return_expr, FuncLoc)) &&
641608
(!ReturnsVoid ||
642609
D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc));
643610
}
644-
if (funMode == Coroutine) {
611+
if (FunKind == diag::FalloffFunctionKind::Coroutine) {
645612
return (ReturnsVoid ||
646-
D.isIgnored(diag::warn_maybe_falloff_nonvoid_function, FuncLoc) ||
647-
D.isIgnored(diag::warn_maybe_falloff_nonvoid_coroutine,
648-
FuncLoc)) &&
613+
D.isIgnored(diag::warn_falloff_nonvoid, FuncLoc)) &&
649614
(!HasNoReturn);
650615
}
651616
// For blocks / lambdas.
@@ -662,12 +627,10 @@ struct CheckFallThroughDiagnostics {
662627
static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
663628
QualType BlockType,
664629
const CheckFallThroughDiagnostics &CD,
665-
AnalysisDeclContext &AC,
666-
sema::FunctionScopeInfo *FSI) {
630+
AnalysisDeclContext &AC) {
667631

668632
bool ReturnsVoid = false;
669633
bool HasNoReturn = false;
670-
bool IsCoroutine = FSI->isCoroutine();
671634

672635
if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
673636
if (const auto *CBody = dyn_cast<CoroutineBodyStmt>(Body))
@@ -696,49 +659,40 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
696659
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
697660
return;
698661
SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc();
699-
auto EmitDiag = [&](SourceLocation Loc, unsigned DiagID) {
700-
if (IsCoroutine) {
701-
if (DiagID != 0)
702-
S.Diag(Loc, DiagID) << FSI->CoroutinePromise->getType();
703-
} else {
704-
S.Diag(Loc, DiagID);
705-
}
706-
};
707662

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

712667
// Either in a function body compound statement, or a function-try-block.
713-
switch (CheckFallThrough(AC)) {
714-
case UnknownFallThrough:
715-
break;
668+
switch (int FallThroughType = CheckFallThrough(AC)) {
669+
case UnknownFallThrough:
670+
break;
716671

717-
case MaybeFallThrough:
718-
if (HasNoReturn)
719-
EmitDiag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn);
720-
else if (!ReturnsVoid)
721-
EmitDiag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid);
722-
break;
723-
case AlwaysFallThrough:
724-
if (HasNoReturn)
725-
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn);
726-
else if (!ReturnsVoid)
727-
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid);
728-
break;
729-
case NeverFallThroughOrReturn:
730-
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
731-
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
732-
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD;
733-
} else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
734-
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 1 << MD;
735-
} else {
736-
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn);
737-
}
672+
case MaybeFallThrough:
673+
case AlwaysFallThrough:
674+
if (HasNoReturn) {
675+
if (CD.diag_FallThrough_HasNoReturn)
676+
S.Diag(RBrace, CD.diag_FallThrough_HasNoReturn) << CD.FunKind;
677+
} else if (!ReturnsVoid && CD.diag_FallThrough_ReturnsNonVoid) {
678+
bool NotInAllControlPaths = FallThroughType == MaybeFallThrough;
679+
S.Diag(RBrace, CD.diag_FallThrough_ReturnsNonVoid)
680+
<< CD.FunKind << NotInAllControlPaths;
681+
}
682+
break;
683+
case NeverFallThroughOrReturn:
684+
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
685+
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
686+
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 0 << FD;
687+
} else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
688+
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn) << 1 << MD;
689+
} else {
690+
S.Diag(LBrace, CD.diag_NeverFallThroughOrReturn);
738691
}
739-
break;
740-
case NeverFallThrough:
741-
break;
692+
}
693+
break;
694+
case NeverFallThrough:
695+
break;
742696
}
743697
}
744698

@@ -2765,7 +2719,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
27652719
: (fscope->isCoroutine()
27662720
? CheckFallThroughDiagnostics::MakeForCoroutine(D)
27672721
: CheckFallThroughDiagnostics::MakeForFunction(D)));
2768-
CheckFallThroughForBody(S, D, Body, BlockType, CD, AC, fscope);
2722+
CheckFallThroughForBody(S, D, Body, BlockType, CD, AC);
27692723
}
27702724

27712725
// Warning: check for unreachable code

clang/lib/Sema/SemaStmt.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,7 +3590,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
35903590

35913591
if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap)) {
35923592
if (CurBlock->FunctionType->castAs<FunctionType>()->getNoReturnAttr()) {
3593-
Diag(ReturnLoc, diag::err_noreturn_block_has_return_expr);
3593+
Diag(ReturnLoc, diag::err_noreturn_has_return_expr)
3594+
<< diag::FalloffFunctionKind::Block;
35943595
return StmtError();
35953596
}
35963597
} else if (auto *CurRegion = dyn_cast<CapturedRegionScopeInfo>(CurCap)) {
@@ -3601,7 +3602,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
36013602
if (CurLambda->CallOperator->getType()
36023603
->castAs<FunctionType>()
36033604
->getNoReturnAttr()) {
3604-
Diag(ReturnLoc, diag::err_noreturn_lambda_has_return_expr);
3605+
Diag(ReturnLoc, diag::err_noreturn_has_return_expr)
3606+
<< diag::FalloffFunctionKind::Lambda;
36053607
return StmtError();
36063608
}
36073609
}

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "test_macros.h"
2020
#include "test_iterators.h"
21+
#include "type_algorithms.h"
2122

2223
class PaddedBase {
2324
public:
@@ -81,7 +82,7 @@ TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) {
8182
}
8283

8384
TEST_CONSTEXPR_CXX20 bool test() {
84-
types::for_each(types::cpp17_input_iterator_list<int*>(), TestInIters());
85+
types::for_each(types::cpp17_input_iterator_list<const int*>(), TestInIters());
8586

8687
{ // Make sure that padding bits aren't copied
8788
Derived src(1, 2, 3);
@@ -91,7 +92,6 @@ TEST_CONSTEXPR_CXX20 bool test() {
9192
assert(dst.b_ == 2);
9293
assert(dst.c_ == 6);
9394
}
94-
9595
{ // Make sure that overlapping ranges can be copied
9696
int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
9797
std::copy(a + 3, a + 10, a);

0 commit comments

Comments
 (0)