https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/77079
>From 544e2b12695fa572b08abc8efdceeadd63ebbde5 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 5 Jan 2024 10:41:55 +0000 Subject: [PATCH 1/2] [clang][ASTImporter] Only reorder fields of RecordDecls Prior to `e9536698720ec524cc8b72599363622bc1a31558` (https://reviews.llvm.org/D154764) we only re-ordered the fields of `RecordDecl`s. The change refactored this logic to make sure `FieldDecl`s are imported before other member decls. However, this change also widened the types of `DeclContext`s we consider for re-ordering from `RecordDecl` to anything that's a `DeclContext`. This seems to have been just a drive-by cleanup. Internally we've seen numerous crashes in LLDB where we try to perform this re-ordering fields of `ObjCInterfaceDecl`s. This patch restores old behaviour where we limit the re-ordering to just `RecordDecl`s. --- clang/lib/AST/ASTImporter.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 9ffae72346f2af..f03ea6e09e8b7e 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -2034,10 +2034,14 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) { return ToDCOrErr.takeError(); } + const auto *FromRD = dyn_cast<RecordDecl>(FromDC); + if (!FromRD) + return ChildErrors; + DeclContext *ToDC = *ToDCOrErr; // Remove all declarations, which may be in wrong order in the // lexical DeclContext and then add them in the proper order. - for (auto *D : FromDC->decls()) { + for (auto *D : FromRD->decls()) { if (!MightNeedReordering(D)) continue; @@ -2055,7 +2059,7 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) { } // Import everything else. - for (auto *From : FromDC->decls()) { + for (auto *From : FromRD->decls()) { if (MightNeedReordering(From)) continue; >From a33d2fe1dda862956bcc25455239e830fa2040ed Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 5 Jan 2024 13:23:50 +0000 Subject: [PATCH 2/2] fixup! make sure we do import non-FieldDecls for FromDC --- clang/lib/AST/ASTImporter.cpp | 40 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index f03ea6e09e8b7e..5e5570bb42a1ef 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -2034,32 +2034,30 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) { return ToDCOrErr.takeError(); } - const auto *FromRD = dyn_cast<RecordDecl>(FromDC); - if (!FromRD) - return ChildErrors; - - DeclContext *ToDC = *ToDCOrErr; - // Remove all declarations, which may be in wrong order in the - // lexical DeclContext and then add them in the proper order. - for (auto *D : FromRD->decls()) { - if (!MightNeedReordering(D)) - continue; + if (const auto *FromRD = dyn_cast<RecordDecl>(FromDC)) { + DeclContext *ToDC = *ToDCOrErr; + // Remove all declarations, which may be in wrong order in the + // lexical DeclContext and then add them in the proper order. + for (auto *D : FromRD->decls()) { + if (!MightNeedReordering(D)) + continue; - assert(D && "DC contains a null decl"); - if (Decl *ToD = Importer.GetAlreadyImportedOrNull(D)) { - // Remove only the decls which we successfully imported. - assert(ToDC == ToD->getLexicalDeclContext() && ToDC->containsDecl(ToD)); - // Remove the decl from its wrong place in the linked list. - ToDC->removeDecl(ToD); - // Add the decl to the end of the linked list. - // This time it will be at the proper place because the enclosing for - // loop iterates in the original (good) order of the decls. - ToDC->addDeclInternal(ToD); + assert(D && "DC contains a null decl"); + if (Decl *ToD = Importer.GetAlreadyImportedOrNull(D)) { + // Remove only the decls which we successfully imported. + assert(ToDC == ToD->getLexicalDeclContext() && ToDC->containsDecl(ToD)); + // Remove the decl from its wrong place in the linked list. + ToDC->removeDecl(ToD); + // Add the decl to the end of the linked list. + // This time it will be at the proper place because the enclosing for + // loop iterates in the original (good) order of the decls. + ToDC->addDeclInternal(ToD); + } } } // Import everything else. - for (auto *From : FromRD->decls()) { + for (auto *From : FromDC->decls()) { if (MightNeedReordering(From)) continue; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
