[Lldb-commits] [PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

Gabor Marton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 27 02:31:30 PDT 2019

martong marked 4 inline comments as done.
martong added inline comments.

Comment at: clang/lib/AST/ASTImporter.cpp:3452
         << FoundField->getType();
-      return make_error<ImportError>(ImportError::NameConflict);
+      ConflictingDecls.push_back(FoundField);
shafik wrote:
> I am a little concerned about this change. I am wondering if this was previously handled differently as a band-aid for some other issues that only showed up in rare cases when they iterated over all the results but not when they errored out on the first. 
> Could we make the changes to `VisitFieldDecl` a separate PR so it is easier to revert later on if we do find this causes a regression we are not currently covering?
Ok, I reverted these hunks. I am going to create a follow-up patch which will include these changes.

Comment at: clang/unittests/AST/ASTImporterTest.cpp:5615
+TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
+  Decl *ToTU = getToTuDecl(
shafik wrote:
> Since the "Liberal" strategy should be the current status quo, I think it might make sense to have a first PR that just has the `*LiberalStrategy` tests to verify that indeed this is the current behavior as we expect.
Ok, I removed the test suite `ConflictingDeclsWithConservativeStrategy`. I am going to create a subsequent patch which adds comprehensive testing for both strategies.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list