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

Shafik Yaghmour via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 26 16:13:33 PDT 2019

shafik accepted this revision.
shafik added a comment.

Other than my two comment this LGTM

Comment at: clang/lib/AST/ASTImporter.cpp:3452
         << FoundField->getType();
-      return make_error<ImportError>(ImportError::NameConflict);
+      ConflictingDecls.push_back(FoundField);
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?

Comment at: clang/unittests/AST/ASTImporterTest.cpp:5615
+TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
+  Decl *ToTU = getToTuDecl(
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.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list