[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG360d901bf047: Revert [lldb] Disable minimal import 
mode for RecordDecls that back FieldDecls (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/test/API/commands/expression/pr52257/Makefile
  lldb/test/API/commands/expression/pr52257/TestExprCrash.py
  lldb/test/API/commands/expression/pr52257/main.cpp
  lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py

Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
+++ lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
@@ -7,6 +7,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailure("The fix for this was reverted due to llvm.org/PR52257")
 def test(self):
 self.build()
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
Index: lldb/test/API/commands/expression/pr52257/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/main.cpp
@@ -0,0 +1,12 @@
+template  struct pair {};
+struct A {
+  using iterator = pair;
+  pair a_[];
+};
+struct B {
+  using iterator = A::iterator;
+  iterator begin();
+  A *tag_set_;
+};
+B b;
+int main() {};
Index: lldb/test/API/commands/expression/pr52257/TestExprCrash.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/TestExprCrash.py
@@ -0,0 +1,18 @@
+"""
+Verify that LLDB doesn't crash during expression evaluation.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprCrashTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_pr52257(self):
+self.build()
+self.createTestTarget()
+self.expect_expr("b", result_type="B", result_children=[ValueCheck(name="tag_set_")])
Index: lldb/test/API/commands/expression/pr52257/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,7 +479,10 @@
decl->getDeclKindName(), ast_dump);
   }
 
-  CopyDecl(decl);
+  Decl *copied_decl = CopyDecl(decl);
+
+  if (!copied_decl)
+continue;
 
   // FIXME: We should add the copied decl to the 'decls' list. This would
   // add the copied Decl into the DeclContext and make sure that we
@@ -489,6 +492,12 @@
   // lookup issues later on.
   // We can't just add them for now as the ASTImporter already added the
   // decl into the DeclContext and this would add it twice.
+
+  if (FieldDecl *copied_field = dyn_cast(copied_decl)) {
+QualType copied_field_type = copied_field->getType();
+
+m_ast_importer_sp->RequireCompleteType(copied_field_type);
+  }
 } else {
   SkippedDecls = true;
 }
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -888,37 +888,6 @@
 LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
   }
 
-  // Disable the minimal import for fields that have record types. There is
-  // no point in minimally importing the record behind their type as Clang
-  // will anyway request their definition when the FieldDecl is added to the
-  // RecordDecl (as Clang will query the FieldDecl's type for things such
-  // as a deleted constexpr destructor).
-  // By importing the type ahead of time we avoid some corner cases where
-  // the FieldDecl's record is importing in the middle of Clang's
-  // `DeclContext::addDecl` logic.
-  if (clang::FieldDecl *fd = dyn_cast(From)) {
-// This is only necessary because we do the 'minimal import'. Remove this
-// once LLDB stopped using that mode.
-assert(isMinimalImport() && "Only necessary for minimal import");
-QualType field_type = fd->getType();
-if (field_type->isRecordType()) {
-  // First get the 

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D113449#3121070 , @teemperor wrote:

> I actually didn't see that the patch deleted the TestCppReferenceToOuterClass 
> test. Could you just add a `@skipIf # Crashes` or so above its `def test...` 
> method? The test itself is still valid user code that we shouldn't crash on.

I assume you mean something like `expectedFailure`, as this should 
unconditionally fail.

> I left some nits and the test source probably needs to be made a bit more 
> expressive in terms of what's its trying to test, but this can all be done 
> later. Let's just land this to get the regression fixed.

OK, retesting real quick against trunk and submitting now. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht updated this revision to Diff 386227.
rupprecht added a comment.

- Use better test APIs, add test back w/ a @expectedFailure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/test/API/commands/expression/pr52257/Makefile
  lldb/test/API/commands/expression/pr52257/TestExprCrash.py
  lldb/test/API/commands/expression/pr52257/main.cpp
  lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py

Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
+++ lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
@@ -7,6 +7,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailure("The fix for this was reverted due to llvm.org/PR52257")
 def test(self):
 self.build()
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
Index: lldb/test/API/commands/expression/pr52257/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/main.cpp
@@ -0,0 +1,12 @@
+template  struct pair {};
+struct A {
+  using iterator = pair;
+  pair a_[];
+};
+struct B {
+  using iterator = A::iterator;
+  iterator begin();
+  A *tag_set_;
+};
+B b;
+int main() {};
Index: lldb/test/API/commands/expression/pr52257/TestExprCrash.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/TestExprCrash.py
@@ -0,0 +1,18 @@
+"""
+Verify that LLDB doesn't crash during expression evaluation.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprCrashTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_pr52257(self):
+self.build()
+self.createTestTarget()
+self.expect_expr("b", result_type="B", result_children=[ValueCheck(name="tag_set_")])
Index: lldb/test/API/commands/expression/pr52257/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,7 +479,10 @@
decl->getDeclKindName(), ast_dump);
   }
 
-  CopyDecl(decl);
+  Decl *copied_decl = CopyDecl(decl);
+
+  if (!copied_decl)
+continue;
 
   // FIXME: We should add the copied decl to the 'decls' list. This would
   // add the copied Decl into the DeclContext and make sure that we
@@ -489,6 +492,12 @@
   // lookup issues later on.
   // We can't just add them for now as the ASTImporter already added the
   // decl into the DeclContext and this would add it twice.
+
+  if (FieldDecl *copied_field = dyn_cast(copied_decl)) {
+QualType copied_field_type = copied_field->getType();
+
+m_ast_importer_sp->RequireCompleteType(copied_field_type);
+  }
 } else {
   SkippedDecls = true;
 }
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -888,37 +888,6 @@
 LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
   }
 
-  // Disable the minimal import for fields that have record types. There is
-  // no point in minimally importing the record behind their type as Clang
-  // will anyway request their definition when the FieldDecl is added to the
-  // RecordDecl (as Clang will query the FieldDecl's type for things such
-  // as a deleted constexpr destructor).
-  // By importing the type ahead of time we avoid some corner cases where
-  // the FieldDecl's record is importing in the middle of Clang's
-  // `DeclContext::addDecl` logic.
-  if (clang::FieldDecl *fd = dyn_cast(From)) {
-// This is only necessary because we do the 'minimal import'. Remove this
-// once LLDB stopped using that mode.
-assert(isMinimalImport() && "Only necessary for minimal import");
-QualType field_type = fd->getType();
-if (field_type->isRecordType()) {
-  // First get the underlying record and minimally import it.
-  clang::TagDecl 

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I actually didn't see that the patch deleted the TestCppReferenceToOuterClass 
test. Could you just add a `@skipIf # Crashes` or so above its `def test...` 
method? The test itself is still valid user code that we shouldn't crash on.

I left some nits and the test source probably needs to be made a bit more 
expressive in terms of what's its trying to test, but this can all be done 
later. Let's just land this to get the regression fixed.




Comment at: lldb/test/API/commands/expression/pr52257/TestExprCrash.py:17
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.expect("expr b", substrs=["tag_set_ = nullptr"])

nit: `self.createTestTarget()` (which generates useful error messages on 
failure)



Comment at: lldb/test/API/commands/expression/pr52257/TestExprCrash.py:18
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.expect("expr b", substrs=["tag_set_ = nullptr"])

nit: `self.expect_expr("b", result_type="B", 
result_children=[ValueCheck(name="tag_set_")])`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht updated this revision to Diff 386114.
rupprecht added a comment.

- Switch crash repro from shell -> api test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/test/API/commands/expression/pr52257/Makefile
  lldb/test/API/commands/expression/pr52257/TestExprCrash.py
  lldb/test/API/commands/expression/pr52257/main.cpp
  lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
  lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
  lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp

Index: lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-struct Outer {
-  typedef int HookToOuter;
-  // When importing this type, we have to import all of it before adding it
-  // via the FieldDecl to 'Outer'. If we don't do this, then Clang will
-  // while adding 'NestedClassMember' ask for the full type of 'NestedClass'
-  // which then will start pulling in the 'RefToOuter' member. That member
-  // will import the typedef above and add it to 'Outer'. And adding a
-  // Decl to a DeclContext that is currently already in the process of adding
-  // another Decl will cause an inconsistent lookup.
-  struct NestedClass {
-HookToOuter RefToOuter;
-int SomeMember;
-  } NestedClassMember;
-};
-
-// We query the members of base classes of a type by doing a lookup via Clang.
-// As this tests is trying to find a borked lookup, we need a base class here
-// to make our 'GetChildMemberWithName' use the Clang lookup.
-struct In : Outer {};
-
-In test_var;
-
-int main() { return test_var.NestedClassMember.SomeMember; }
Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
+++ /dev/null
@@ -1,16 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def test(self):
-self.build()
-self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
-test_var = self.expect_expr("test_var", result_type="In")
-nested_member = test_var.GetChildMemberWithName('NestedClassMember')
-self.assertEqual("Outer::NestedClass",
- nested_member.GetType().GetName())
Index: lldb/test/API/commands/expression/pr52257/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/main.cpp
@@ -0,0 +1,12 @@
+template  struct pair {};
+struct A {
+  using iterator = pair;
+  pair a_[];
+};
+struct B {
+  using iterator = A::iterator;
+  iterator begin();
+  A *tag_set_;
+};
+B b;
+int main() {};
\ No newline at end of file
Index: lldb/test/API/commands/expression/pr52257/TestExprCrash.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/pr52257/TestExprCrash.py
@@ -0,0 +1,18 @@
+"""
+Verify that LLDB doesn't crash during expression evaluation.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprCrashTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_pr52257(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.expect("expr b", substrs=["tag_set_ = nullptr"])
Index: lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
@@ -1,3 +0,0 @@
-CXX_SOURCES := main.cpp
-
-include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,7 +479,10 @@
decl->getDeclKindName(), ast_dump);
   }
 
-  CopyDecl(decl);
+  Decl *copied_decl = CopyDecl(decl);
+
+  if (!copied_decl)
+continue;
 
   // FIXME: We should add the copied decl to the 'decls' list. This would
   // add the copied Decl into the DeclContext and make sure that we
@@ -489,6 +492,12 @@
   // lookup issues later on.
   // We can't just add them for now as the ASTImporter already added the
   // decl into the DeclContext 

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp:21
+};
+B b;

teemperor wrote:
> FWIW, I think probably should be an API test (for a bunch of reasons from not 
> relying on formatting output to remote device testing), but given this is 
> just a revert I won't insist on that. Thanks again!
Makes sense -- this was just the easiest way to copy/paste the repro from the 
bug. I'll convert it to an API test when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp:21
+};
+B b;

FWIW, I think probably should be an API test (for a bunch of reasons from not 
relying on formatting output to remote device testing), but given this is just 
a revert I won't insist on that. Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

Given that this patch has been in tree for half a year, it'd be good to get 
confirmation here this can be reverted given there is now a test case for 
causing a crash. I got an offline comment that this is OK to revert, so if 
nobody has objections, I'll land sometime tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
rupprecht added reviewers: labath, teemperor.
Herald added a reviewer: shafik.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This reverts commit 3bf96b0329be554c67282b0d7d8da6a864b9e38f 
. It 
causes crashes as reported in PR52257 and a few other places. A reproducer is 
bundled with this commit to verify any fix forward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113449

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
  lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
  lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
  lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp

Index: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp
@@ -0,0 +1,21 @@
+// Verify that LLDB doesn't crash. See llvm.org/PR52257.
+
+// RUN: %clangxx_host -g -c %s -o %t.o
+// RUN: %lldb -b -o "print b" %t.o | FileCheck %s
+
+// CHECK: (lldb) print b
+// CHECK: (B) $0 = {
+// CHECK:   tag_set_ = nullptr
+// CHECK: }
+
+template  struct pair {};
+struct A {
+  using iterator = pair;
+  pair a_[];
+};
+struct B {
+  using iterator = A::iterator;
+  iterator begin();
+  A *tag_set_;
+};
+B b;
Index: lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-struct Outer {
-  typedef int HookToOuter;
-  // When importing this type, we have to import all of it before adding it
-  // via the FieldDecl to 'Outer'. If we don't do this, then Clang will
-  // while adding 'NestedClassMember' ask for the full type of 'NestedClass'
-  // which then will start pulling in the 'RefToOuter' member. That member
-  // will import the typedef above and add it to 'Outer'. And adding a
-  // Decl to a DeclContext that is currently already in the process of adding
-  // another Decl will cause an inconsistent lookup.
-  struct NestedClass {
-HookToOuter RefToOuter;
-int SomeMember;
-  } NestedClassMember;
-};
-
-// We query the members of base classes of a type by doing a lookup via Clang.
-// As this tests is trying to find a borked lookup, we need a base class here
-// to make our 'GetChildMemberWithName' use the Clang lookup.
-struct In : Outer {};
-
-In test_var;
-
-int main() { return test_var.NestedClassMember.SomeMember; }
Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
+++ /dev/null
@@ -1,16 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def test(self):
-self.build()
-self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
-test_var = self.expect_expr("test_var", result_type="In")
-nested_member = test_var.GetChildMemberWithName('NestedClassMember')
-self.assertEqual("Outer::NestedClass",
- nested_member.GetType().GetName())
Index: lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
===
--- lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-CXX_SOURCES := main.cpp
-
-include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,7 +479,10 @@
decl->getDeclKindName(), ast_dump);
   }
 
-  CopyDecl(decl);
+  Decl *copied_decl = CopyDecl(decl);
+
+  if (!copied_decl)
+continue;
 
   // FIXME: We should add the copied decl to the 'decls' list. This would
   // add the copied Decl into the DeclContext and make sure that we
@@ -489,6 +492,12 @@
   // lookup issues later on.
   // We can't just add them for now as the ASTImporter already added the
   // decl into the DeclContext and this would add it twice.
+
+  if (FieldDecl *copied_field = dyn_cast(copied_decl)) {
+QualType copied_field_type = copied_field->getType();
+
+m_ast_importer_sp->RequireCompleteType(copied_field_type);
+  }
 } else {
   SkippedDecls = true;
 }
Index: