Page MenuHomePhabricator

Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName
ClosedPublic

Authored by shafik on May 23 2019, 4:16 PM.

Details

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

Diff Detail

Repository
rL LLVM

Event Timeline

shafik created this revision.May 23 2019, 4:16 PM
martong accepted this revision.May 24 2019, 1:29 AM

Looks good, thank you!

This revision is now accepted and ready to land.May 24 2019, 1:29 AM
davide requested changes to this revision.May 24 2019, 9:01 AM
davide added a subscriber: davide.

Do you have a testcase?

This revision now requires changes to proceed.May 24 2019, 9:01 AM

@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 resigned from this revision.May 24 2019, 9:34 AM

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

This revision is now accepted and ready to land.May 24 2019, 9:34 AM

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.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2019, 9:51 AM