[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName
Shafik Yaghmour via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 25 09:08:03 PDT 2019
shafik added a comment.
In D59665#1439070 <https://reviews.llvm.org/D59665#1439070>, @martong wrote:
> Hi Shafik,
>
> Thank you for looking into this. This is indeed a bug, because whenever a we encounter an unnamed EnumDecl in the "from" context then we return with a nameconflict error.
> However, I think we should fix this differently.
> First of all, currently HandleNameConflict just returns the parameter `Name` it received. This may be unnamed in some cases and thus converts to true. So I think HandleNameConflict should be changed that it would return a default initialized DeclarationName; so once we have anything in the ConflictingDecls then we return with the NameConflict error:
>
> DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name,
> DeclContext *DC,
> unsigned IDNS,
> NamedDecl **Decls,
> unsigned NumDecls) {
> - return Name;
> + return DeclarationName();
> }
>
>
> And most importantly, I think we should push into the ConflictingDecls only if the structural match indicates a non-match:
>
> if (auto *FoundEnum = dyn_cast<EnumDecl>(FoundDecl)) {
> if (isStructuralMatch(D, FoundEnum, true))
> return Importer.MapImported(D, FoundEnum);
> + ConflictingDecls.push_back(FoundDecl);
> }
> -
> - ConflictingDecls.push_back(FoundDecl);
>
>
> I am aware of similar errors like this with other AST nodes. We had a patch in our fork to fix that in January (https://github.com/Ericsson/clang/pull/569/files) I am going to prepare a patch from that cause I see now this affects you guys in LLDB too.
This sounds reasonable, let me test it out and make sure it looks good.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59665/new/
https://reviews.llvm.org/D59665
More information about the cfe-commits
mailing list