[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rG6807f244fa67: [ASTImporter] Modifying ImportDeclContext(...) to ensure that we also handle… (authored by shafik). Herald added projects: clang, LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik updated this revision to Diff 292607. shafik added a comment. Fix handling of multi-dimensional arrays. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86660/new/ https://reviews.llvm.org/D86660 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. In D86660#2278025 , @shafik wrote: > We are going to move forward with this approach (after dealing with the > multi-dimensional array case)

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik added a comment. Herald added a reviewer: JDevlieghere. We are going to move forward with this approach (after dealing with the multi-dimensional array case) temporarily. We are seeing crash bugs from this from users and we want to fix it while we explore the solution space more. This

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added a comment. The key question is this: Why exactly does `1735: ExpectedDecl ImportedOrErr = import(From);` fails to do the import properly when called from ASTImporter::ImportDefinition? Would it be possible to debug that from your LLDB test cases? Maybe starting with the simpler

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik marked an inline comment as done. shafik added a subscriber: rsmith. shafik added a comment. @martong I have been experimenting w/ how we pass around `ImportDefinitionKind` and pushing it through to more places does not help. I also tried defaulting most locations to `IDK_Everything` to

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

labath added a comment. I'm going to respond to the rest of your (very insightful) comment later. So far, I'm just responding to this: >> This isn't exactly layout related, but there is the question of covariant >> methods. If a method is covariant, then its return type must be complete. > >

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl =

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

labath added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl =

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik marked 2 inline comments as done. shafik added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl =

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1737 // If we are in the process of ImportDefinition(...) for a RecordDecl we // want to make sure that we are also completing each FieldDecl. There `ImportDefinition(...)` here

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

labath added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl =

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

shafik created this revision. shafik added reviewers: martong, teemperor. Herald added a subscriber: rnkovacs. shafik requested review of this revision. When we fixed `ImportDeclContext(...)` in D71378 to make sure we complete each `FieldDecl` of a `RecordDecl`