Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[shafik]

Summary

https://reviews.llvm.org/D51633 added error handling to the ASTNodeImporter::VisitRecordDecl for the conflicting names case. This could lead to erroneous return of an error in that case since we should have been using SearchName. Name may be empty in the case where we find the name via D->getTypedefNameForAnonDecl()->getDeclName().

This fix is very similar to https://reviews.llvm.org/D59665

[martong]

Looks good, thank you!

[davide]

Do you have a testcase?

[shafik]

@davide We have a reproducer but so far it has proven difficult to narrow it down to a test case. This is serious regression b/c this leads to fields being dropped in records during expression evaluation leading to results that looks like a bug but are actually expression parsing issues.

[davide]

I don't think I'm qualified enough to comment further on this patch.

[davide]

To elaborate further, I'm not familiar with this code so I really shouldn't be the person to approve the patch. On the other hand I'm somewhat nervous as refactors in this area of the codebase caused bugs/crashes in lldb, fundamentally due to changes in behavior previously left untested.

cfe/trunk/lib/AST/ASTImporter.cpp

for (auto *FoundDecl : FoundDecls) {
           break;
         }
       }
 
       ConflictingDecls.push_back(FoundDecl);
     } // for
 
     if (!ConflictingDecls.empty() && SearchName) {
-      Name = Importer.HandleNameConflict(Name, DC, IDNS,
+      Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
                                          ConflictingDecls.data(),
                                          ConflictingDecls.size());
       if (!Name)
         return make_error<ImportError>(ImportError::NameConflict);
     }
   }
 
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());