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

Fri Sep 4 03:26:48 PDT 2020

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 tests in D71378 <https://reviews.llvm.org/D71378>?

In D86660#2255360 <https://reviews.llvm.org/D86660#2255360>, @shafik wrote:

> @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 experiment to see if maybe I was missing some crucial point and no luck. So while it seemed like a good direction it has not worked out, unless I missed something in your idea.

These are bad news, probably I am missing something. Still, we should not give up the investigation, or not at least in a long term. I really think we have to fix this sooner or later. What about the other approaches (trying to get rid of the minimal flag, trying to get rid of the kind, moving the minimal flag to the nodeimporter, and could be other ideas as well)?

> So my current approach may be the best we have until we can do some larger refactoring.

Okay, I understand this fixes the immediate issue and we don't have any other solution yet without significant more efforts.

> I had explored trying to fix this from the codegen and sema side of things but after discussing this w/ @rsmith there is no simple fix there that would not require some large refactoring on the LLDB side.

There must be a way to fix this in ASTImporter and/or in LLDB. We should be able to provide an AST that is meaningful for the codegen. But yeah, some refactoring might be needed.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86660/new/

https://reviews.llvm.org/D86660