Fix issue with duplicated entity definition

Closes #21.

include/cppast/cpp_entity_index.hpp

@@ -62,11 +62,18 @@ namespace cppast
     class cpp_entity_index
     {
     public:
+        /// Exception thrown on duplicate entity definition.
+        class duplicate_definition_error : public std::logic_error
+        {
+        public:
+            duplicate_definition_error();
+        };
+
         /// \effects Registers a new [cppast::cpp_entity]() which is a definition.
         /// It will override any previously registered declarations of the same entity.
-        /// \requires If the entity has been registered before, it must be as declaration,
+        /// \throws duplicate_defintion_error if the entity has been registered as definition before.
-        /// and the entity must live as long as the index lives.
+        /// \requires The entity must live as long as the index lives,
-        /// \requires The entity must not be a namespace.
+        /// and it must not be a namespace.
         /// \notes This operation is thread safe.
         void register_definition(cpp_entity_id                           id,
                                  type_safe::object_ref<const cpp_entity> entity) const;

src/cpp_entity_index.cpp

@@ -10,6 +10,11 @@
 
 using namespace cppast;
 
+cpp_entity_index::duplicate_definition_error::duplicate_definition_error()
+: std::logic_error("duplicate registration of entity definition")
+{
+}
+
 void cpp_entity_index::register_definition(cpp_entity_id                           id,
                                            type_safe::object_ref<const cpp_entity> entity) const
 {
@@ -21,8 +26,8 @@ void cpp_entity_index::register_definition(cpp_entity_id
     {
         // already in map, override declaration
         auto& value = result.first->second;
-        DEBUG_ASSERT(!value.is_definition, detail::precondition_error_handler{},
+        if (value.is_definition)
-                     "duplicate entity registration");
+            throw duplicate_definition_error();
         value.is_definition = true;
         value.entity        = entity;
     }

src/libclang/enum_parser.cpp

@@ -96,6 +96,11 @@ std::unique_ptr<cpp_entity> detail::parse_cpp_enum(const detail::parse_context&
         {
             context.logger->log("libclang parser", ex.get_diagnostic());
         }
+        catch (std::logic_error& ex)
+        {
+            context.logger->log("libclang parser",
+                                diagnostic{ex.what(), make_location(child), severity::error});
+        }
     });
     if (clang_isCursorDefinition(cur))
         return builder.finish(*context.idx, get_entity_id(cur), std::move(semantic_parent));

src/libclang/function_parser.cpp

@@ -48,6 +48,12 @@ namespace
                 {
                     context.logger->log("libclang parser", ex.get_diagnostic());
                 }
+                catch (std::logic_error& ex)
+                {
+                    context.logger->log("libclang parser",
+                                        diagnostic{ex.what(), detail::make_location(child),
+                                                   severity::error});
+                }
             });
         }
         else
@@ -66,6 +72,14 @@ namespace
                 {
                     context.logger->log("libclang parser", ex.get_diagnostic());
                 }
+                catch (std::logic_error& ex)
+                {
+                    context.logger->log("libclang parser",
+                                        diagnostic{ex.what(),
+                                                   detail::make_location(
+                                                       clang_Cursor_getArgument(cur, unsigned(i))),
+                                                   severity::error});
+                }
         }
     }
 

src/libclang/parse_functions.cpp

@@ -15,11 +15,14 @@ cpp_entity_id detail::get_entity_id(const CXCursor& cur)
 {
     cxstring usr(clang_getCursorUSR(cur));
     DEBUG_ASSERT(!usr.empty(), detail::parse_error_handler{}, cur, "cannot create id for entity");
-    if (clang_getCursorKind(cur) == CXCursor_FunctionTemplate)
+    if (clang_getCursorKind(cur) == CXCursor_FunctionTemplate
+        || clang_getCursorKind(cur) == CXCursor_ConversionFunction)
     {
         // we have a function template
         // combine return type with USR to ensure no ambiguity when SFINAE is applied there
         // (and hope this prevents all collisions...)
+        // same workaround also applies to conversion functions,
+        // there template arguments in the result are ignored
         cxstring type_spelling(clang_getTypeSpelling(clang_getCursorResultType(cur)));
         return cpp_entity_id(std::string(usr.c_str()) + type_spelling.c_str());
     }
@@ -230,6 +233,12 @@ catch (parse_error& ex)
     context.logger->log("libclang parser", ex.get_diagnostic());
     return nullptr;
 }
+catch (std::logic_error& ex)
+{
+    context.logger->log("libclang parser",
+                        diagnostic{ex.what(), detail::make_location(cur), severity::error});
+    return nullptr;
+}
 
 std::unique_ptr<cpp_entity> detail::parse_cpp_static_assert(const detail::parse_context& context,
                                                             const CXCursor&              cur)

src/libclang/type_parser.cpp

@@ -392,32 +392,38 @@ namespace
                                         const std::string& templ_name)
     {
         // look if the type has a declaration that is a template
-        auto decl = clang_getTypeDeclaration(type);
+        auto decl  = clang_getTypeDeclaration(type);
-        if (is_direct_templated(decl))
+        auto count = clang_Type_getNumTemplateArguments(clang_getCursorType(decl));
+        if (count > 0 || is_direct_templated(decl))
             return decl;
-
+        else
-        // look if the templ_name matches a template template parameter
+        {
-        auto param = clang_getNullCursor();
+            // look if the templ_name matches a template template parameter
-        detail::visit_children(cur, [&](const CXCursor& child) {
+            auto param = clang_getNullCursor();
-            if (clang_getCursorKind(child) == CXCursor_TemplateTemplateParameter
+            detail::visit_children(cur, [&](const CXCursor& child) {
-                && detail::get_cursor_name(child) == templ_name.c_str())
+                if (clang_getCursorKind(child) == CXCursor_TemplateTemplateParameter
-            {
+                    && detail::get_cursor_name(child) == templ_name.c_str())
-                DEBUG_ASSERT(clang_Cursor_isNull(param), detail::parse_error_handler{}, cur,
+                {
-                             "multiple template template parameters with the same name?!");
+                    DEBUG_ASSERT(clang_Cursor_isNull(param), detail::parse_error_handler{}, cur,
-                param = child;
+                                 "multiple template template parameters with the same name?!");
-            }
+                    param = child;
-        });
+                }
-        return param;
+            });
+            return param;
+        }
     }
 
     std::unique_ptr<cpp_type> try_parse_instantiation_type(const detail::parse_context&,
                                                            const CXCursor& cur, const CXType& type)
     {
         return make_leave_type(type, [&](std::string&& spelling) -> std::unique_ptr<cpp_type> {
-            auto        ptr = spelling.c_str();
+            auto ptr = spelling.c_str();
+
             std::string templ_name;
             for (; *ptr && *ptr != '<'; ++ptr)
                 templ_name += *ptr;
+            if (*ptr != '<')
+                return nullptr;
             ++ptr;
 
             auto templ = get_instantiation_template(cur, type, templ_name);
@@ -429,7 +435,8 @@ namespace
 
             // parse arguments
             // i.e. not parse really, just add the string
-            DEBUG_ASSERT(!spelling.empty() && spelling.back() == '>', detail::assert_handler{});
+            if (spelling.empty() || spelling.back() != '>')
+                return nullptr;
             spelling.pop_back();
             builder.add_unexposed_arguments(ptr);
 

test/cpp_member_function.cpp

@@ -157,18 +157,36 @@ TEST_CASE("cpp_conversion_op")
 namespace ns
 {
     template <typename T>
-    using type = char;
+    struct type {};
 }
 
 // most of it only need to be check in member function
 struct foo
 {
     /// operator int&();
-    operator int&();
+    operator int&()
+    {
+        static int i;
+        return i;
+    }
+
     /// explicit operator bool()const;
-    explicit operator bool() const;
+    explicit operator bool() const
+    {
+        return false;
+    }
+
     /// constexpr operator ns::type<int>();
-    constexpr operator ns::type<int>();
+    constexpr operator ns::type<int>()
+    {
+        return {};
+    }
+
+    /// constexpr operator ns::type<char>();
+    constexpr operator ns::type<char>()
+    {
+        return {};
+    }
 };
 )";
 
@@ -177,7 +195,7 @@ struct foo
     auto count            = test_visit<cpp_conversion_op>(*file, [&](const cpp_conversion_op& op) {
         REQUIRE(count_children(op.parameters()) == 0u);
         REQUIRE(!op.is_variadic());
-        REQUIRE(op.body_kind() == cpp_function_declaration);
+        REQUIRE(op.body_kind() == cpp_function_definition);
         REQUIRE(op.ref_qualifier() == cpp_ref_none);
         REQUIRE(!op.virtual_info());
         REQUIRE(!op.noexcept_condition());
@@ -198,17 +216,32 @@ struct foo
         }
         else if (!op.is_explicit() && op.is_constexpr())
         {
-            REQUIRE(op.name() == "operator ns::type<int>");
-            cpp_template_instantiation_type::builder builder(
-                cpp_template_ref(cpp_entity_id(""), "ns::type"));
-            builder.add_unexposed_arguments("int");
-            REQUIRE(equal_types(idx, op.return_type(), *builder.finish()));
             REQUIRE(op.cv_qualifier() == cpp_cv_none);
+            if (op.name() == "operator ns::type<int>")
+            {
+                REQUIRE(op.return_type().kind() == cpp_type_kind::template_instantiation_t);
+                auto& inst = static_cast<const cpp_template_instantiation_type&>(op.return_type());
+
+                REQUIRE(inst.primary_template().name() == "ns::type");
+                REQUIRE(!inst.arguments_exposed());
+                REQUIRE(inst.unexposed_arguments() == "int");
+            }
+            else if (op.name() == "operator ns::type<char>")
+            {
+                REQUIRE(op.return_type().kind() == cpp_type_kind::template_instantiation_t);
+                auto& inst = static_cast<const cpp_template_instantiation_type&>(op.return_type());
+
+                REQUIRE(inst.primary_template().name() == "ns::type");
+                REQUIRE(!inst.arguments_exposed());
+                REQUIRE(inst.unexposed_arguments() == "char");
+            }
+            else
+                REQUIRE(false);
         }
         else
             REQUIRE(false);
     });
-    REQUIRE(count == 3u);
+    REQUIRE(count == 4u);
 }
 
 TEST_CASE("cpp_constructor")