martong updated this revision to Diff 200033.
martong added a comment.
- se -> so
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61333/new/
https://reviews.llvm.org/D61333
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,18 +637,6 @@
m_ast_importer_sp->RequireCompleteType(copied_field_type);
}
-
- DeclContext *decl_context_non_const =
- const_cast<DeclContext *>(decl_context);
-
- if (copied_decl->getDeclContext() != decl_context) {
- if (copied_decl->getDeclContext()->containsDecl(copied_decl))
- copied_decl->getDeclContext()->removeDecl(copied_decl);
- copied_decl->setDeclContext(decl_context_non_const);
- }
-
- if (!decl_context_non_const->containsDecl(copied_decl))
- decl_context_non_const->addDeclInternal(copied_decl);
}
}
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
@@ -5,11 +5,11 @@
typedef struct {
int a;
int b;
-} FILE;
+} MYFILE;
int main()
{
- FILE *myFile = malloc(sizeof(FILE));
+ MYFILE *myFile = malloc(sizeof(MYFILE));
myFile->a = 5;
myFile->b = 9;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
@@ -0,0 +1,5 @@
+int main()
+{
+ int a = 0; // Set breakpoint 0 here.
+ return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
@@ -0,0 +1,77 @@
+"""Test that importing modules in C works as expected."""
+
+from __future__ import print_function
+
+
+from distutils.version import StrictVersion
+import os
+import time
+import platform
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CModulesTestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @skipIfFreeBSD
+ @skipIfLinux
+ @skipIfWindows
+ @skipIfNetBSD
+ @skipIf(macos_version=["<", "10.12"])
+ def test_expr(self):
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+ lldbutil.run_break_set_by_file_and_line(
+ self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # The stop reason of the thread should be breakpoint.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+ # The breakpoint should have a hit count of 1.
+ self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+ substrs=[' resolved, hit count = 1'])
+
+ # Enable logging of the imported AST.
+ log_file = os.path.join(self.getBuildDir(), "lldb-ast-log.txt")
+ self.runCmd("log enable lldb ast -f '%s'" % log_file)
+
+ self.expect(
+ "expr -l objc++ -- @import Darwin; 3",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ "int",
+ "3"])
+
+ # This expr command imports __sFILE with definition
+ # (FILE is a typedef to __sFILE.)
+ self.expect(
+ "expr *fopen(\"/dev/zero\", \"w\")",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ "FILE",
+ "_close"]
+ )
+
+ # Check that the AST log contains exactly one definition of __sFILE.
+ f = open(log_file)
+ log_lines = f.readlines()
+ f.close()
+ os.remove(log_file)
+ self.assertEqual(" ".join(log_lines).count("struct __sFILE definition"), 1)
+
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+ # Find the line number to break inside main().
+ self.line = line_number('main.c', '// Set breakpoint 0 here.')
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4583,6 +4583,50 @@
EXPECT_EQ(Imported->getPreviousDecl(), Friend);
}
+struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
+ LLDBLookupTest() {
+ Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+ return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport,
+ // We use the regular lookup.
+ /*LookupTable=*/nullptr);
+ };
+ }
+};
+
+TEST_P(LLDBLookupTest, ImporterShouldFindInTransparentContext) {
+ TranslationUnitDecl *ToTU = getToTuDecl(
+ R"(
+ extern "C" {
+ class X{};
+ };
+ )",
+ Lang_CXX);
+ auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("X")));
+
+ // Set up a stub external storage.
+ ToTU->setHasExternalLexicalStorage(true);
+ // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+ ToTU->setMustBuildLookupTable();
+ struct TestExternalASTSource : ExternalASTSource{};
+ ToTU->getASTContext().setExternalSource(new TestExternalASTSource());
+
+ Decl *FromTU = getTuDecl(
+ R"(
+ class X;
+ )",
+ Lang_CXX);
+ auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("X")));
+ auto *ImportedX = Import(FromX, Lang_CXX);
+ // The lookup must find the existing class definition in the LinkageSpecDecl.
+ // Then the importer renders the existing and the new decl into one chain.
+ EXPECT_EQ(ImportedX->getCanonicalDecl(), ToX->getCanonicalDecl());
+}
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions, );
@@ -4626,5 +4670,8 @@
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
+ DefaultTestValuesForRunOptions, );
+
} // end namespace ast_matchers
} // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1823,6 +1823,13 @@
if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
return Err;
+ // There are cases in LLDB when we first import a class without its members.
+ // The class will have DefinitionData, but no members. Then,
+ // importDefinition is called from LLDB, which tries to get the members, so
+ // when we get here, the class already has the DefinitionData set, so we must
+ // unset the CompleteDefinition here to be able to complete again the
+ // definition.
+ To->setCompleteDefinition(false);
To->completeDefinition();
return Error::success();
}
@@ -7700,10 +7707,20 @@
LookupTable->lookup(ReDC, Name);
return FoundDeclsTy(LookupResult.begin(), LookupResult.end());
} else {
- // FIXME Can we remove this kind of lookup?
- // Or lldb really needs this C/C++ lookup?
- FoundDeclsTy Result;
- ReDC->localUncachedLookup(Name, Result);
+ DeclContext::lookup_result NoloadLookupResult = ReDC->noload_lookup(Name);
+ FoundDeclsTy Result(NoloadLookupResult.begin(), NoloadLookupResult.end());
+ // We must search by the slow case of localUncachedLookup because that is
+ // working even if there is no LookupPtr for the DC. We could use
+ // DC::buildLookup() to create the LookupPtr, but that would load external
+ // decls again, we must avoid that case.
+ // Also, even if we had the LookupPtr, we must find Decls which are not
+ // in the LookupPtr, so we need the slow case.
+ // These cases are handled in ASTImporterLookupTable, but we cannot use
+ // that with LLDB since that traverses through the AST which initiates the
+ // load of external decls again via DC::decls(). And again, we must avoid
+ // loading external decls during the import.
+ if (Result.empty())
+ ReDC->localUncachedLookup(Name, Result);
return Result;
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
