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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to