[Lldb-commits] [lldb] r370045 - [ASTImporter] Fix name conflict handling with different strategies

Gabor Marton via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 27 04:36:10 PDT 2019


Author: martong
Date: Tue Aug 27 04:36:10 2019
New Revision: 370045

URL: http://llvm.org/viewvc/llvm-project?rev=370045&view=rev
Log:
[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

Modified:
    lldb/trunk/include/lldb/Symbol/ClangASTImporter.h

Modified: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTImporter.h?rev=370045&r1=370044&r2=370045&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h Tue Aug 27 04:36:10 2019
@@ -252,7 +252,9 @@ private:
         : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
                              master.m_file_manager, true /*minimal*/),
           m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
-          m_master(master), m_source_ctx(source_ctx) {}
+          m_master(master), m_source_ctx(source_ctx) {
+      setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+    }
 
     /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate
     /// and deattaches it at the end of the scope. Supports being used multiple




More information about the lldb-commits mailing list