[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: a_sidorin, shafik, teemperor.
Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong added a parent revision: D55049: Changed every use of ASTImporter::Import to Import_New.

There are numorous flaws about the name conflict handling, this patch
attempts fixes them. Changes in details:

- HandleNameConflict return with a false DeclarationName

Hitherto we effectively never returned with a NameConflict error, even
if the preceding StructuralMatch indicated a conflict.
Because we just simply returned with the parameter `Name` in
HandleNameConflict and that name is almost always `true` when converted to
`bool`.

- Add tests which indicate wrong NameConflict handling

- Add to ConflictingDecls only if decl kind is different

Note, we might not indicate an ODR error when there is an existing record decl
and a enum is imported with same name.  But there are other cases. E.g. think
about the case when we import a FunctionTemplateDecl with name f and we found a
simple FunctionDecl with name f. They overload.  Or in case of a
ClassTemplateDecl and CXXRecordDecl, the CXXRecordDecl could be the 'templated'
class, so it would be false to report error.  So I think we should report a
name conflict error only when we are 100% sure of that.  That is why I think it
should be a general pattern to report the error only if the kind is the same.

- Fix failing ctu test with EnumConstandDecl

In ctu-main.c we have the enum class 'A' which brings in the enum
constant 'x' with value 0 into the global namespace.
In ctu-other.c we had the enum class 'B' which brought in the same name
('x') as an enum constant but with a different enum value (42). This is clearly
an ODR violation in the global namespace. The solution was to rename the
second enum constant.

- Fix lldb test failures

Remove the call of the unused and vexing GetOriginalDecl(). This information is
already maintained in the ImportedDecls member.


Repository:
  rC Clang

https://reviews.llvm.org/D59692

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Analysis/Inputs/ctu-other.c
  unittests/AST/ASTImporterTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59692.191864.patch
Type: text/x-patch
Size: 8720 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190322/253e2f67/attachment-0001.bin>