[ASTImporter] Fix name conflict handling with different strategies

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.

 * Introduce ODR handling strategies

Reviewers: a_sidorin, shafik

Differential Revision: https://reviews.llvm.org/D59692

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@370045 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 11 insertions, 6 deletions

diff --git a/include/clang/AST/ASTImporter.h b/include/clang/AST/ASTImporter.h
index 6f30fa47d8..c82dcab35d 100644
--- a/include/clang/AST/ASTImporter.h
+++ b/include/clang/AST/ASTImporter.h
@@ -90,6 +90,8 @@ class TypeSourceInfo;
     using FileIDImportHandlerType =
         std::function<void(FileID /*ToID*/, FileID /*FromID*/)>;
 
+    enum class ODRHandlingType { Conservative, Liberal };
+
     // An ImportPath is the list of the AST nodes which we visit during an
     // Import call.
     // If node `A` depends on node `B` then the path contains an `A`->`B` edge.
@@ -236,6 +238,8 @@ class TypeSourceInfo;
     /// Whether to perform a minimal import.
     bool Minimal;
 
+    ODRHandlingType ODRHandling;
+
     /// Whether the last diagnostic came from the "from" context.
     bool LastDiagFromFrom = false;
 
@@ -326,6 +330,8 @@ class TypeSourceInfo;
     /// to-be-completed forward declarations when possible.
     bool isMinimalImport() const { return Minimal; }
 
+    void setODRHandling(ODRHandlingType T) { ODRHandling = T; }
+
     /// \brief Import the given object, returns the result.
     ///
     /// \param To Import the object into this variable.
@@ -517,12 +523,11 @@ class TypeSourceInfo;
     ///
     /// \param NumDecls the number of conflicting declarations in \p Decls.
     ///
-    /// \returns the name that the newly-imported declaration should have.
-    virtual DeclarationName HandleNameConflict(DeclarationName Name,
-                                               DeclContext *DC,
-                                               unsigned IDNS,
-                                               NamedDecl **Decls,
-                                               unsigned NumDecls);
+    /// \returns the name that the newly-imported declaration should have. Or
+    /// an error if we can't handle the name conflict.
+    virtual Expected<DeclarationName>
+    HandleNameConflict(DeclarationName Name, DeclContext *DC, unsigned IDNS,
+                       NamedDecl **Decls, unsigned NumDecls);
 
     /// Retrieve the context that AST nodes are being imported into.
     ASTContext &getToContext() const { return ToContext; }