[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59665/new/

https://reviews.llvm.org/D59665