draft Attempt to get the specific_decl_iterator replaced #114135
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While working to try to replace the Decl linked-list for the 'next' item, I discovered that the propsensity of us to use these iterators ends up causing some pretty significant problems with the transition. I'm hopeful we can replace specific_decl_iterator and filtered_decl_iterator with variants that just use llvm::make_filtered_range. This patch starts that effort by making an alternative for specific_iterator, and replaces it in the 'range for' cases that it gets used.
You can test this locally with the following command:git-clang-format --diff d661aea4c5668fc9b06f4b26d9fb072b1a6d7ff4 a703e9e7437135de291036f4dbe3698e149dabce --extensions h,cpp -- clang-tools-extra/clang-tidy/utils/DesignatedInitializers.cpp clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h clang/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp clang/lib/ARCMigrate/TransProperties.cpp clang/lib/ARCMigrate/Transforms.cpp clang/lib/AST/ASTStructuralEquivalence.cpp clang/lib/AST/ComparisonCategories.cpp clang/lib/AST/Decl.cpp clang/lib/AST/DeclBase.cpp clang/lib/AST/Expr.cpp clang/lib/AST/ExprConstant.cpp clang/lib/AST/RecordLayoutBuilder.cpp clang/lib/AST/Type.cpp clang/lib/Analysis/UninitializedValues.cpp clang/lib/CodeGen/ABIInfoImpl.cpp clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGExprAgg.cpp clang/lib/CodeGen/CGExprConstant.cpp clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp clang/lib/CodeGen/CGRecordLayoutBuilder.cpp clang/lib/CodeGen/CGStmt.cpp clang/lib/CodeGen/CGStmtOpenMP.cpp clang/lib/CodeGen/CodeGenTBAA.cpp clang/lib/CodeGen/Targets/Mips.cpp clang/lib/CodeGen/Targets/X86.cpp clang/lib/InstallAPI/Visitor.cpp clang/lib/Sema/SemaCodeComplete.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaDeclObjC.cpp clang/lib/Sema/SemaInit.cpp clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/unittests/AST/ASTImporterTest.cppView the diff from clang-format here.diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 3be3921113..61f5b3c5c5 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2353,22 +2353,22 @@ public:
decl_range decls() const;
decl_iterator decls_begin() const { return decls().begin(); }
decl_iterator decls_end() const { return decls().end(); }
- bool decls_empty() const {return decls().empty(); }
+ bool decls_empty() const { return decls().empty(); }
/// noload_decls_begin/end - Iterate over the declarations stored in this
/// context that are currently loaded; don't attempt to retrieve anything
/// from an external source.
decl_range noload_decls() const {
- return { decl_iterator(FirstDecl), decl_iterator{}};
+ return {decl_iterator(FirstDecl), decl_iterator{}};
}
decl_iterator noload_decls_begin() const { return noload_decls().begin(); }
decl_iterator noload_decls_end() const { return noload_decls().end(); }
template <typename ItTy, typename SpecificDecl>
- class decl_cast_iterator : public llvm::mapped_iterator_base<
- decl_cast_iterator<ItTy, SpecificDecl>, ItTy,
- SpecificDecl *> {
+ class decl_cast_iterator
+ : public llvm::mapped_iterator_base<
+ decl_cast_iterator<ItTy, SpecificDecl>, ItTy, SpecificDecl *> {
using BaseT =
llvm::mapped_iterator_base<decl_cast_iterator<ItTy, SpecificDecl>, ItTy,
SpecificDecl *>;
@@ -2377,9 +2377,7 @@ public:
decl_cast_iterator() : BaseT(ItTy{}) {}
decl_cast_iterator(ItTy Itr) : BaseT(Itr) {}
- SpecificDecl *mapElement(Decl *D) const {
- return cast<SpecificDecl>(D);
- }
+ SpecificDecl *mapElement(Decl *D) const { return cast<SpecificDecl>(D); }
};
template <typename SpecificDecl, typename ItTy>
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 908be1edb3..c9ec288690 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1834,11 +1834,11 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
// Check the fields for consistency.
QualType D2Type = Context.ToCtx.getTypeDeclType(D2);
- std::optional<FieldDecl*> Field2Opt = std::nullopt;
+ std::optional<FieldDecl *> Field2Opt = std::nullopt;
for (auto FieldsTuple : llvm::zip_first(D1->fields(), D2->fields())) {
FieldDecl *Field1 = std::get<0>(FieldsTuple);
- std::optional<FieldDecl*> Field2Opt = std::get<1>(FieldsTuple);
+ std::optional<FieldDecl *> Field2Opt = std::get<1>(FieldsTuple);
if (!Field2Opt.has_value()) {
if (Context.Complain) {
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 6a5c68115f..5c1c2151f6 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1805,7 +1805,7 @@ void CGDebugInfo::CollectRecordLambdaFields(
// both concurrently.
const ASTRecordLayout &layout = CGM.getContext().getASTRecordLayout(CXXDecl);
for (const auto &[fieldno, C, Field] :
- llvm::enumerate(CXXDecl->captures(), CXXDecl->fields())) {
+ llvm::enumerate(CXXDecl->captures(), CXXDecl->fields())) {
if (C.capturesVariable()) {
SourceLocation Loc = C.getLocation();
assert(!Field->isBitField() && "lambdas don't have bitfield members!");
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 0da28ed903..2cddb81346 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -905,8 +905,8 @@ bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD,
return false;
if (ZeroInitPadding) {
- if (!DoZeroInitPadding(Layout, FieldNo, *Field, AllowOverwrite,
- SizeSoFar, ZeroFieldSize))
+ if (!DoZeroInitPadding(Layout, FieldNo, *Field, AllowOverwrite, SizeSoFar,
+ ZeroFieldSize))
return false;
if (ZeroFieldSize)
SizeSoFar += CharUnits::fromQuantity(
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 519194ea44..16ae384999 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -394,9 +394,9 @@ void CodeGenFunction::GenerateOpenMPCapturedVars(
// and load it as a void pointer.
if (!CurField->getType()->isAnyPointerType()) {
ASTContext &Ctx = getContext();
- Address DstAddr = CreateMemTemp(
- Ctx.getUIntPtrType(),
- Twine(CurCap.getCapturedVar()->getName(), ".casted"));
+ Address DstAddr =
+ CreateMemTemp(Ctx.getUIntPtrType(),
+ Twine(CurCap.getCapturedVar()->getName(), ".casted"));
LValue DstLV = MakeAddrLValue(DstAddr, Ctx.getUIntPtrType());
llvm::Value *SrcAddrVal = EmitScalarConversion(
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 131101269e..5a2b64a2a5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15469,11 +15469,12 @@ LambdaScopeInfo *Sema::RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator) {
if (VD->isInitCapture())
CurrentInstantiationScope->InstantiatedLocal(VD, VD);
const bool ByRef = C.getCaptureKind() == LCK_ByRef;
- LSI->addCapture(VD, /*IsBlock*/false, ByRef,
- /*RefersToEnclosingVariableOrCapture*/true, C.getLocation(),
- /*EllipsisLoc*/C.isPackExpansion()
- ? C.getEllipsisLoc() : SourceLocation(),
- (*I)->getType(), /*Invalid*/false);
+ LSI->addCapture(VD, /*IsBlock*/ false, ByRef,
+ /*RefersToEnclosingVariableOrCapture*/ true,
+ C.getLocation(),
+ /*EllipsisLoc*/ C.isPackExpansion() ? C.getEllipsisLoc()
+ : SourceLocation(),
+ (*I)->getType(), /*Invalid*/ false);
} else if (C.capturesThis()) {
LSI->addThisCapture(/*Nested*/ false, C.getLocation(), (*I)->getType(),
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 4fd4c8451f..bc0c1ab6d4 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -2289,7 +2289,10 @@ void InitListChecker::CheckStructUnionTypes(
Field != FieldEnd; ++Field) {
if ((*Field)->hasInClassInitializer() ||
((*Field)->isAnonymousStructOrUnion() &&
- (*Field)->getType()->getAsCXXRecordDecl()->hasInClassInitializer())) {
+ (*Field)
+ ->getType()
+ ->getAsCXXRecordDecl()
+ ->hasInClassInitializer())) {
StructuredList->setInitializedFieldInUnion(*Field);
// FIXME: Actually build a CXXDefaultInitExpr?
return;
@@ -3012,7 +3015,6 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
}
}
-
// Update the designator with the field declaration.
if (!VerifyOnly)
D->setFieldDecl(*Field);
@@ -3034,8 +3036,9 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
SemaRef.Diag(NextD->getBeginLoc(),
diag::err_designator_into_flexible_array_member)
<< SourceRange(NextD->getBeginLoc(), DIE->getEndLoc());
- SemaRef.Diag((*Field)->getLocation(), diag::note_flexible_array_member)
- << *Field;
+ SemaRef.Diag((*Field)->getLocation(),
+ diag::note_flexible_array_member)
+ << *Field;
}
Invalid = true;
}
@@ -3047,8 +3050,9 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
SemaRef.Diag(DIE->getInit()->getBeginLoc(),
diag::err_flexible_array_init_needs_braces)
<< DIE->getInit()->getSourceRange();
- SemaRef.Diag((*Field)->getLocation(), diag::note_flexible_array_member)
- << *Field;
+ SemaRef.Diag((*Field)->getLocation(),
+ diag::note_flexible_array_member)
+ << *Field;
}
Invalid = true;
}
@@ -8408,7 +8412,8 @@ ExprResult InitializationSequence::Perform(Sema &S,
return InvalidType();
} else {
if ((*Field)->isBitField() ||
- !S.Context.hasSameType((*Field)->getType(), S.Context.getSizeType()))
+ !S.Context.hasSameType((*Field)->getType(),
+ S.Context.getSizeType()))
return InvalidType();
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the things holding up solutions to #113950 is the pretty rough uses of iterators in a weird way all over the place. This patch set attempted to replace them with a llvm::make_filtered_range + a casting iterator. I only tried to replace 'field_iterator' here plus a couple of for-each type fields, doing some algorithmic symplification where I would.
Good news: it compiles
Bad news: check-clang has ~57 failures, including failing a stack-exhaustion test, so it seems the greater size of the field-iterator (now 2x in size, since it needs to hold an 'end' iterator instead of being able to count on null) has caused our stack size to increase too much.
At the moment, I'm probably abandoning this effort. There IS a bunch of perf that would come from the linked-list-to-vector, mixed with making the local
VarDeclnot have to care about its next-decls or decl-context (thus giving us less to transform), but this is likely a pretty sizable lift.