Skip to content

[clang][NFC] Use range-based for loop and algorithms in SemaDeclCXX.cpp #169938

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.

Sign up for GitHub

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

Open
davidstone wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[clang][NFC] Use range-based for loop and algorithms in SemaDeclCXX.cpp #169938

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1fb4651
[clang][NFC] Use range-based for loop and algorithms in `SemaDeclCXX.…
davidstone Nov 28, 2025
2f9d46e
Use `llvm::all_of`
davidstone Dec 2, 2025
c3593a5
Review comments
davidstone Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 73 additions & 101 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ void Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
UnparsedDefaultArgInstantiationsMap::iterator InstPos
= UnparsedDefaultArgInstantiations.find(Param);
if (InstPos != UnparsedDefaultArgInstantiations.end()) {
for (unsigned I = 0, N = InstPos->second.size(); I != N; ++I)
InstPos->second[I]->setUninstantiatedDefaultArg(Arg);
for (auto &Instantiation : InstPos->second)
Instantiation->setUninstantiatedDefaultArg(Arg);

// We're done tracking this parameter's instantiations.
UnparsedDefaultArgInstantiations.erase(InstPos);
Expand Down Expand Up @@ -2547,8 +2547,8 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
diag::ext_constexpr_function_never_constant_expr)
<< isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval()
<< Dcl->getNameInfo().getSourceRange();
for (size_t I = 0, N = Diags.size(); I != N; ++I)
SemaRef.Diag(Diags[I].first, Diags[I].second);
for (const auto &Diag : Diags)
SemaRef.Diag(Diag.first, Diag.second);
// Don't return false here: we allow this for compatibility in
// system headers.
}
Expand Down Expand Up @@ -3211,17 +3211,15 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
std::string Sema::getAmbiguousPathsDisplayString(CXXBasePaths &Paths) {
std::string PathDisplayStr;
std::set<unsigned> DisplayedPaths;
for (CXXBasePaths::paths_iterator Path = Paths.begin();
Path != Paths.end(); ++Path) {
if (DisplayedPaths.insert(Path->back().SubobjectNumber).second) {
for (const CXXBasePath &Path : Paths) {
if (DisplayedPaths.insert(Path.back().SubobjectNumber).second) {
// We haven't displayed a path to this particular base
// class subobject yet.
PathDisplayStr += "\n ";
PathDisplayStr += QualType(Context.getCanonicalTagType(Paths.getOrigin()))
.getAsString();
for (CXXBasePath::const_iterator Element = Path->begin();
Element != Path->end(); ++Element)
PathDisplayStr += " -> " + Element->Base->getType().getAsString();
for (const CXXBasePathElement &Element : Path)
PathDisplayStr += " -> " + Element.Base->getType().getAsString();
}
}

Expand Down Expand Up @@ -4260,10 +4258,9 @@ static bool FindBaseInitializer(Sema &SemaRef,
if (SemaRef.IsDerivedFrom(ClassDecl->getLocation(),
SemaRef.Context.getCanonicalTagType(ClassDecl),
BaseType, Paths)) {
for (CXXBasePaths::paths_iterator Path = Paths.begin();
Path != Paths.end(); ++Path) {
if (Path->back().Base->isVirtual()) {
VirtualBaseSpec = Path->back().Base;
for (const CXXBasePath &Path : Paths) {
if (Path.back().Base->isVirtual()) {
VirtualBaseSpec = Path.back().Base;
break;
}
}
Expand Down Expand Up @@ -5468,9 +5465,7 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,

bool HadError = false;

for (unsigned i = 0; i < Initializers.size(); i++) {
CXXCtorInitializer *Member = Initializers[i];

for (CXXCtorInitializer *Member : Initializers) {
if (Member->isBaseInitializer())
Info.AllBaseFields[Member->getBaseClass()->getAsCanonical<RecordType>()] =
Member;
Expand Down Expand Up @@ -5675,8 +5670,7 @@ static void DiagnoseBaseOrMemInitializerOrder(
// Don't check initializers order unless the warning is enabled at the
// location of at least one initializer.
bool ShouldCheckOrder = false;
for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
CXXCtorInitializer *Init = Inits[InitIndex];
for (CXXCtorInitializer *Init : Inits) {
if (!SemaRef.Diags.isIgnored(diag::warn_initializer_out_of_order,
Init->getSourceLocation())) {
ShouldCheckOrder = true;
Expand Down Expand Up @@ -6045,31 +6039,25 @@ void Sema::DiagnoseAbstractType(const CXXRecordDecl *RD) {
// more than once.
llvm::SmallPtrSet<const CXXMethodDecl *, 8> SeenPureMethods;

for (CXXFinalOverriderMap::iterator M = FinalOverriders.begin(),
MEnd = FinalOverriders.end();
M != MEnd;
++M) {
for (OverridingMethods::iterator SO = M->second.begin(),
SOEnd = M->second.end();
SO != SOEnd; ++SO) {
for (const auto &M : FinalOverriders) {
for (const auto &SO : M.second) {
// C++ [class.abstract]p4:
// A class is abstract if it contains or inherits at least one
// pure virtual function for which the final overrider is pure
// virtual.

//
if (SO->second.size() != 1)
if (SO.second.size() != 1)
continue;
const CXXMethodDecl *Method = SO.second.front().Method;

if (!SO->second.front().Method->isPureVirtual())
if (!Method->isPureVirtual())
continue;

if (!SeenPureMethods.insert(SO->second.front().Method).second)
if (!SeenPureMethods.insert(Method).second)
continue;

Diag(SO->second.front().Method->getLocation(),
diag::note_pure_virtual_function)
<< SO->second.front().Method->getDeclName() << RD->getDeclName();
Diag(Method->getLocation(), diag::note_pure_virtual_function)
<< Method->getDeclName() << RD->getDeclName();
}
}

Expand Down Expand Up @@ -7012,18 +7000,16 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) {
// C++ [class.mem]p14:
// In addition, if class T has a user-declared constructor (12.1), every
// non-static data member of class T shall have a name different from T.
DeclContext::lookup_result R = Record->lookup(Record->getDeclName());
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E;
++I) {
NamedDecl *D = (*I)->getUnderlyingDecl();
for (const NamedDecl *Element : Record->lookup(Record->getDeclName())) {
const NamedDecl *D = Element->getUnderlyingDecl();
// Invalid IndirectFieldDecls have already been diagnosed with
// err_anonymous_record_member_redecl in
// SemaDecl.cpp:CheckAnonMemberRedeclaration.
if (((isa<FieldDecl>(D) || isa<UnresolvedUsingValueDecl>(D)) &&
Record->hasUserDeclaredConstructor()) ||
(isa<IndirectFieldDecl>(D) && !D->isInvalidDecl())) {
Diag((*I)->getLocation(), diag::err_member_name_of_class)
<< D->getDeclName();
Diag(Element->getLocation(), diag::err_member_name_of_class)
<< D->getDeclName();
break;
}
}
Expand Down Expand Up @@ -10552,10 +10538,8 @@ void Sema::FindHiddenVirtualMethods(CXXMethodDecl *MD,
// Keep the base methods that were overridden or introduced in the subclass
// by 'using' in a set. A base method not in this set is hidden.
CXXRecordDecl *DC = MD->getParent();
DeclContext::lookup_result R = DC->lookup(MD->getDeclName());
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) {
NamedDecl *ND = *I;
if (UsingShadowDecl *shad = dyn_cast<UsingShadowDecl>(*I))
for (NamedDecl *ND : DC->lookup(MD->getDeclName())) {
if (UsingShadowDecl *shad = dyn_cast<UsingShadowDecl>(ND))
ND = shad->getTargetDecl();
if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(ND))
AddMostOverridenMethods(MD, FHVM.OverridenAndUsingBaseMethods);
Expand All @@ -10567,8 +10551,7 @@ void Sema::FindHiddenVirtualMethods(CXXMethodDecl *MD,

void Sema::NoteHiddenVirtualMethods(CXXMethodDecl *MD,
SmallVectorImpl<CXXMethodDecl*> &OverloadedMethods) {
for (unsigned i = 0, e = OverloadedMethods.size(); i != e; ++i) {
CXXMethodDecl *overloadedMD = OverloadedMethods[i];
for (const CXXMethodDecl *overloadedMD : OverloadedMethods) {
PartialDiagnostic PD = PDiag(
diag::note_hidden_overloaded_virtual_declared_here) << overloadedMD;
HandleFunctionTypeMismatch(PD, MD->getType(), overloadedMD->getType());
Expand Down Expand Up @@ -12787,9 +12770,8 @@ bool Sema::CheckUsingShadowDecl(BaseUsingDecl *BUD, NamedDecl *Orig,
// should redeclare it.
NamedDecl *NonTag = nullptr, *Tag = nullptr;
bool FoundEquivalentDecl = false;
for (LookupResult::iterator I = Previous.begin(), E = Previous.end();
I != E; ++I) {
NamedDecl *D = (*I)->getUnderlyingDecl();
for (NamedDecl *Element : Previous) {
NamedDecl *D = Element->getUnderlyingDecl();
// We can have UsingDecls in our Previous results because we use the same
// LookupResult for checking whether the UsingDecl itself is a valid
// redeclaration.
Expand All @@ -12810,7 +12792,7 @@ bool Sema::CheckUsingShadowDecl(BaseUsingDecl *BUD, NamedDecl *Orig,
}

if (IsEquivalentForUsingDecl(Context, D, Target)) {
if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(*I))
if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(Element))
PrevShadow = Shadow;
FoundEquivalentDecl = true;
} else if (isEquivalentInternalLinkageDeclaration(D, Target)) {
Expand Down Expand Up @@ -13298,8 +13280,8 @@ NamedDecl *Sema::BuildUsingDeclaration(
if (!R.getAsSingle<TypeDecl>() &&
!R.getAsSingle<UnresolvedUsingIfExistsDecl>()) {
Diag(IdentLoc, diag::err_using_typename_non_type);
for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I)
Diag((*I)->getUnderlyingDecl()->getLocation(),
for (const NamedDecl *D : R)
Diag(D->getUnderlyingDecl()->getLocation(),
diag::note_using_decl_target);
return BuildInvalid();
}
Expand Down Expand Up @@ -13337,10 +13319,10 @@ NamedDecl *Sema::BuildUsingDeclaration(
return UD;
}

for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
for (NamedDecl *D : R) {
UsingShadowDecl *PrevDecl = nullptr;
if (!CheckUsingShadowDecl(UD, *I, Previous, PrevDecl))
BuildUsingShadowDecl(S, UD, *I, PrevDecl);
if (!CheckUsingShadowDecl(UD, D, Previous, PrevDecl))
BuildUsingShadowDecl(S, UD, D, PrevDecl);
}

return UD;
Expand Down Expand Up @@ -13476,23 +13458,22 @@ bool Sema::CheckUsingDeclRedeclaration(SourceLocation UsingLoc,
}

NestedNameSpecifier CNNS = Qual.getCanonical();
for (LookupResult::iterator I = Prev.begin(), E = Prev.end(); I != E; ++I) {
NamedDecl *D = *I;

for (const NamedDecl *D : Prev) {
bool DTypename;
NestedNameSpecifier DQual = std::nullopt;
if (UsingDecl *UD = dyn_cast<UsingDecl>(D)) {
if (const UsingDecl *UD = dyn_cast<UsingDecl>(D)) {
DTypename = UD->hasTypename();
DQual = UD->getQualifier();
} else if (UnresolvedUsingValueDecl *UD
= dyn_cast<UnresolvedUsingValueDecl>(D)) {
} else if (const UnresolvedUsingValueDecl *UD =
dyn_cast<UnresolvedUsingValueDecl>(D)) {
DTypename = false;
DQual = UD->getQualifier();
} else if (UnresolvedUsingTypenameDecl *UD
= dyn_cast<UnresolvedUsingTypenameDecl>(D)) {
} else if (const UnresolvedUsingTypenameDecl *UD =
dyn_cast<UnresolvedUsingTypenameDecl>(D)) {
DTypename = true;
DQual = UD->getQualifier();
} else continue;
} else
continue;

// using decls differ if one says 'typename' and the other doesn't.
// FIXME: non-dependent using decls?
Expand Down Expand Up @@ -16378,8 +16359,8 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, CXXRecordDecl *ClassDecl) {
HasConstantInit) {
Diag(VD->getLocation(),
diag::err_constexpr_var_requires_const_destruction) << VD;
for (unsigned I = 0, N = Notes.size(); I != N; ++I)
Diag(Notes[I].first, Notes[I].second);
for (const PartialDiagnosticAt &Note : Notes)
Diag(Note.first, Note.second);
}
}

Expand Down Expand Up @@ -17658,21 +17639,20 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
Expr::EvalResult Result;
SmallString<12> ValueString;
bool Print;
} DiagSide[2] = {{LHS, Expr::EvalResult(), {}, false},
{RHS, Expr::EvalResult(), {}, false}};
for (unsigned I = 0; I < 2; I++) {
const Expr *Side = DiagSide[I].Cond;
} DiagSides[2] = {{LHS, Expr::EvalResult(), {}, false},
{RHS, Expr::EvalResult(), {}, false}};
for (auto &DiagSide : DiagSides) {
Copy link
Contributor

@zwuis zwuis Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use structured bindings here :)

const Expr *Side = DiagSide.Cond;

Side->EvaluateAsRValue(DiagSide[I].Result, Context, true);
Side->EvaluateAsRValue(DiagSide.Result, Context, true);

DiagSide[I].Print =
ConvertAPValueToString(DiagSide[I].Result.Val, Side->getType(),
DiagSide[I].ValueString, Context);
DiagSide.Print = ConvertAPValueToString(
DiagSide.Result.Val, Side->getType(), DiagSide.ValueString, Context);
}
if (DiagSide[0].Print && DiagSide[1].Print) {
if (DiagSides[0].Print && DiagSides[1].Print) {
Diag(Op->getExprLoc(), diag::note_expr_evaluates_to)
<< DiagSide[0].ValueString << Op->getOpcodeStr()
<< DiagSide[1].ValueString << Op->getSourceRange();
<< DiagSides[0].ValueString << Op->getOpcodeStr()
<< DiagSides[1].ValueString << Op->getSourceRange();
}
} else {
DiagnoseTypeTraitDetails(E);
Expand Down Expand Up @@ -17969,13 +17949,9 @@ DeclResult Sema::ActOnTemplatedFriendTag(

if (Invalid) return true;

bool isAllExplicitSpecializations = true;
for (unsigned I = TempParamLists.size(); I-- > 0; ) {
if (TempParamLists[I]->size()) {
isAllExplicitSpecializations = false;
break;
}
}
const bool isAllExplicitSpecializations = std::all_of(
Copy link
Contributor

@tbaederr tbaederr Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const bool isAllExplicitSpecializations = std::all_of(
bool isAllExplicitSpecializations = std::all_of(

Copy link
Contributor Author

@davidstone davidstone Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would I not want const here?

Copy link
Contributor

@tbaederr tbaederr Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Becaus we don't make local variables const unless they are pointers or references.

Copy link
Contributor Author

@davidstone davidstone Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find this in the coding standard and I can't find a Discourse topic about this. Is there documentation somewhere that I can read to get the justification for this? There are counter-examples in this same file (for instance, in Sema::CheckShadowInheritedFields) and it definitely helps my understanding to know a variable isn't mutated.

Copy link
Collaborator

@AaronBallman AaronBallman Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find this in the coding standard and I can't find a Discourse topic about this. Is there documentation somewhere that I can read to get the justification for this? There are counter-examples in this same file (for instance, in Sema::CheckShadowInheritedFields) and it definitely helps my understanding to know a variable isn't mutated.

We're consistently inconsistent because folks will slip them in from time to time, but I concur with the review feedback; drop top-level const on anything that's not a data member. The reason is: very little of the code base is const correct (we're slowly trying to improve that) and we have plenty of const_cast use in the code base. So declaring the object as actually const is in danger of accidentally turning that into UB (whereas leaving it non-const means we still have a maintenance task to attend to, but at least it's not turned into a bug).

TempParamLists.begin(), TempParamLists.end(),
[](const TemplateParameterList *List) { return List->size() == 0; });

// FIXME: don't ignore attributes.

Expand Down Expand Up @@ -18973,18 +18949,18 @@ void Sema::LoadExternalVTableUses() {
SmallVector<ExternalVTableUse, 4> VTables;
ExternalSource->ReadUsedVTables(VTables);
SmallVector<VTableUse, 4> NewUses;
for (unsigned I = 0, N = VTables.size(); I != N; ++I) {
llvm::DenseMap<CXXRecordDecl *, bool>::iterator Pos
= VTablesUsed.find(VTables[I].Record);
for (const ExternalVTableUse &VTable : VTables) {
llvm::DenseMap<CXXRecordDecl *, bool>::iterator Pos =
VTablesUsed.find(VTable.Record);
// Even if a definition wasn't required before, it may be required now.
if (Pos != VTablesUsed.end()) {
if (!Pos->second && VTables[I].DefinitionRequired)
if (!Pos->second && VTable.DefinitionRequired)
Pos->second = true;
continue;
}

VTablesUsed[VTables[I].Record] = VTables[I].DefinitionRequired;
NewUses.push_back(VTableUse(VTables[I].Record, VTables[I].Location));
VTablesUsed[VTable.Record] = VTable.DefinitionRequired;
NewUses.push_back(VTableUse(VTable.Record, VTable.Location));
}

VTableUses.insert(VTableUses.begin(), NewUses.begin(), NewUses.end());
Expand Down Expand Up @@ -19159,14 +19135,10 @@ void Sema::MarkVirtualMembersReferenced(SourceLocation Loc,
// Mark all functions which will appear in RD's vtable as used.
CXXFinalOverriderMap FinalOverriders;
RD->getFinalOverriders(FinalOverriders);
for (CXXFinalOverriderMap::const_iterator I = FinalOverriders.begin(),
E = FinalOverriders.end();
I != E; ++I) {
for (OverridingMethods::const_iterator OI = I->second.begin(),
OE = I->second.end();
OI != OE; ++OI) {
assert(OI->second.size() > 0 && "no final overrider");
CXXMethodDecl *Overrider = OI->second.front().Method;
for (const auto &FinalOverrider : FinalOverriders) {
for (const auto &OverridingMethod : FinalOverrider.second) {
assert(OverridingMethod.second.size() > 0 && "no final overrider");
CXXMethodDecl *Overrider = OverridingMethod.second.front().Method;

// C++ [basic.def.odr]p2:
// [...] A virtual member function is used if it is not pure. [...]
Expand Down Expand Up @@ -19262,8 +19234,8 @@ void Sema::CheckDelegatingCtorCycles() {
I != E; ++I)
DelegatingCycleHelper(*I, Valid, Invalid, Current, *this);

for (auto CI = Invalid.begin(), CE = Invalid.end(); CI != CE; ++CI)
(*CI)->setInvalidDecl();
for (CXXConstructorDecl *CI : Invalid)
CI->setInvalidDecl();
}

namespace {
Expand Down Expand Up @@ -19394,8 +19366,8 @@ bool Sema::checkThisInStaticMemberFunctionAttributes(CXXMethodDecl *Method) {
if (Arg && !Finder.TraverseStmt(Arg))
return true;

for (unsigned I = 0, N = Args.size(); I != N; ++I) {
if (!Finder.TraverseStmt(Args[I]))
for (Expr *A : Args) {
if (!Finder.TraverseStmt(A))
return true;
}
}
Expand Down