Merge pull request #78 from OleksandrKvl:OleksandrKvl/better-class-names

Better names mangling
OleksandrKvl · Dec 18, 2024 · b4cd5a6 · b4cd5a6
2 parents a5c5e23 + 87292d7
commit b4cd5a6
Show file tree

Hide file tree

Showing 28 changed files with 1,772 additions and 586 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 1.5.0
+
+- implement more user-friendly names mangling
+
+---
+
 # 1.4.2
 
 - add build and test GitHub action

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.11)
 
 project(sbepp
-    VERSION 1.4.2
+    VERSION 1.5.0
     LANGUAGES CXX
 )
 

diff --git a/doc/representation.md b/doc/representation.md
@@ -67,11 +67,37 @@ on schema names.
 ## Schema names
 
 `sbepp` preserves names of all schema entities without any modification. It
-means that messages, types, fields, etc. will have the same type/function names
-in the generated code. Of course standard
+means that messages, types, fields, etc. will have the same class/function names
+in the *public* part of the generated code. Of course standard
 C++ naming rules are still applied and usually you'll get an error from `sbeppc`
 if schema uses wrong name.
 
+### Names mangling {#names-mangling}
+
+\note Things described below are implementation details and are only provided to
+simplify understanding of compiler error messages. Don't rely on them directly.
+
+Although original names are preserved in public interface, sometimes underlying
+implementation is located in `detail` and a public alias is provided for it.
+Names from `detail` should never be used explicitly but in case of error,
+compiler usually uses them in error message so it's useful to know how they are
+formed.
+
+`sbepp` tries to preserve schema names for everything but when it's not
+possible, class name is mangled like `<original_name>_<N>` where `N` is a
+number. Group entries have class names like `<group_name>_entry` where
+`group_name` is a potentially mangled group name. Tag types can be mangled as
+well and their names always match those of the corresponding implementation
+types.
+
+For example, public encoding `User` is always accessible as
+`schema_name::types::User` but can actually be an alias to
+`schema_name::detail::types::User_0`. Similar, its tag is
+`schema_name::schema::types::User` but it can be an alias to
+`schema_name::detail::schema::types::User_0`. When schema doesn't have a lot of
+repetitive names, looking at the trailing class name is enough to understand
+which schema entity it represents.
+
 ---
 
 ## Semantics

diff --git a/sbeppc/src/sbepp/sbeppc/context_manager.hpp b/sbeppc/src/sbepp/sbeppc/context_manager.hpp
@@ -21,21 +21,21 @@ struct type_context
     // here and below, set only if encoding is located inside a composite
     std::optional<offset_t> offset_in_composite;
     std::string tag;
-    std::string impl_name;
     bool is_template;
     std::string public_type;
     std::string underlying_type;
     std::string impl_type;
+    std::optional<std::string> mangled_name;
 };
 
 struct composite_context
 {
     std::size_t size;
     std::optional<offset_t> offset_in_composite;
     std::string tag;
-    std::string impl_name;
     std::string impl_type;
     std::string public_type;
+    std::optional<std::string> mangled_name;
 };
 
 struct ref_context
@@ -58,10 +58,10 @@ struct enumeration_context
     std::optional<offset_t> offset_in_composite;
     std::string primitive_type;
     std::string tag;
-    std::string impl_name;
     std::string public_type;
     std::string impl_type;
     std::string underlying_type;
+    std::optional<std::string> mangled_name;
 };
 
 struct set_choice_context
@@ -75,19 +75,19 @@ struct set_context
     std::optional<offset_t> offset_in_composite;
     std::string primitive_type;
     std::string tag;
-    std::string impl_name;
     std::string public_type;
     std::string impl_type;
     std::string underlying_type;
+    std::optional<std::string> mangled_name;
 };
 
 struct message_context
 {
     std::string tag;
-    std::string impl_name;
     std::string impl_type;
     std::string public_type;
     block_length_t actual_block_length;
+    std::optional<std::string> mangled_name;
 };
 
 struct field_context
@@ -105,10 +105,11 @@ struct field_context
 struct group_context
 {
     std::string tag;
-    std::string impl_name;
     std::string entry_impl_type;
     std::string impl_type;
     block_length_t actual_block_length;
+    std::optional<std::string> mangled_name;
+    std::string entry_name;
 };
 
 struct data_context
@@ -122,6 +123,8 @@ struct message_schema_context
 {
     std::string name;
     std::string tag;
+    std::optional<std::string> mangled_tag_types_name;
+    std::optional<std::string> mangled_tag_messages_name;
 };
 
 template<typename T>

diff --git a/sbeppc/src/sbepp/sbeppc/main.cpp b/sbeppc/src/sbepp/sbeppc/main.cpp
@@ -11,6 +11,7 @@
 #include <sbepp/sbeppc/sbe_schema_validator.hpp>
 #include <sbepp/sbeppc/sbe_schema_cpp_validator.hpp>
 #include <sbepp/sbeppc/context_manager.hpp>
+#include <sbepp/sbeppc/names_generator.hpp>
 
 #include <fmt/core.h>
 
@@ -155,11 +156,11 @@ int main(int argc, char** argv)
         const auto& schema = parser.get_message_schema();
 
         context_manager ctx_manager;
-        sbe_schema_validator sbe_validator{reporter};
-        sbe_validator.validate(schema, ctx_manager);
+        sbe_schema_validator::validate(schema, ctx_manager, reporter);
+        sbe_schema_cpp_validator::validate(
+            schema, config.schema_name, ctx_manager, reporter);
 
-        sbe_schema_cpp_validator cpp_validator{reporter, ctx_manager};
-        cpp_validator.validate(schema, config.schema_name);
+        names_generator::generate(schema, ctx_manager);
 
         schema_compiler::compile(
             config.output_dir,

diff --git a/sbeppc/src/sbepp/sbeppc/messages_compiler.hpp b/sbeppc/src/sbepp/sbeppc/messages_compiler.hpp
@@ -26,8 +26,8 @@ class messages_compiler
 public:
     using on_message_cb_t = std::function<void(
         const std::string_view name,
-        const std::string_view implementation,
-        const std::string_view alias,
+        const std::string_view detail_message,
+        const std::string_view public_message,
         const std::unordered_set<std::string>& dependencies,
         const std::string_view traits)>;
 
@@ -54,7 +54,6 @@ class messages_compiler
     traits_generator* traits_gen{};
     context_manager* ctx_manager{};
     on_message_cb_t on_message_cb;
-    std::size_t group_entry_index{};
     std::unordered_set<std::string> dependencies;
 
     std::string value_ref_to_enumerator(const std::string_view value_ref) const
@@ -64,7 +63,7 @@ class messages_compiler
             *schema, parsed.enum_name);
 
         return fmt::format(
-            "{}::{}", ctx_manager->get(e).impl_type, parsed.enumerator);
+            "{}::{}", ctx_manager->get(e).public_type, parsed.enumerator);
     }
 
     std::string value_ref_to_enum_value(const std::string_view value_ref) const
@@ -191,22 +190,6 @@ class messages_compiler
         return res;
     }
 
-    std::string make_next_group_entry_name(const sbe::level_members& members)
-    {
-        group_entry_index++;
-        auto name = fmt::format("entry_{}", group_entry_index);
-
-        const auto member_names = get_member_names(members);
-        std::size_t minor_index{};
-        while(member_names.count(name))
-        {
-            minor_index++;
-            name = fmt::format("entry_{}_{}", group_entry_index, minor_index);
-        }
-
-        return name;
-    }
-
     static std::string make_level_size_bytes_impl(
         const bool is_flat,
         const std::string_view last_member,
@@ -338,8 +321,9 @@ R"(
         // entry should not take header size into account
         const auto accessors = make_level_accessors(g.members, 0);
 
-        const auto class_name = make_next_group_entry_name(g.members);
-        ctx_manager->get(g).entry_impl_type = fmt::format(
+        auto& group_context = ctx_manager->get(g);
+        const auto& class_name = group_context.entry_name;
+        group_context.entry_impl_type = fmt::format(
             "::{}::detail::messages::{}",
             ctx_manager->get(*schema).name,
             class_name);
@@ -412,7 +396,7 @@ class {name} : public {base_class}
     {
         const auto& t = get_header_element(c, "numInGroup");
 
-        return ctx_manager->get(t).impl_type;
+        return ctx_manager->get(t).public_type;
     }
 
     std::string make_group_header_filler(const sbe::group& g)
@@ -461,7 +445,7 @@ R"(
         context.impl_type = fmt::format(
             "::{}::detail::messages::{}",
             ctx_manager->get(*schema).name,
-            context.impl_name);
+            context.mangled_name.value_or(g.name));
         const auto& dimension_encoding =
             utils::get_schema_encoding_as<sbe::composite>(
                 *schema, g.dimension_type);
@@ -500,7 +484,7 @@ class {name} : public ::sbepp::detail::{base_class}<
 }};
 )",
             // clang-format on
-            fmt::arg("name", context.impl_name),
+            fmt::arg("name", context.mangled_name.value_or(g.name)),
             fmt::arg("dimension", dimension_type),
             fmt::arg("entry", context.entry_impl_type),
             fmt::arg("base_class", base_class),
@@ -1631,10 +1615,23 @@ R"(
             make_level_accessors(m.members, header_context.size);
         const auto& schema_name = ctx_manager->get(*schema).name;
 
-        message_context.impl_type = fmt::format(
-            "::{}::detail::messages::{}",
-            schema_name,
-            message_context.impl_name);
+        if(message_context.mangled_name)
+        {
+            message_context.impl_type = fmt::format(
+                "::{}::detail::messages::{}",
+                schema_name,
+                *message_context.mangled_name);
+        }
+        else
+        {
+            message_context.impl_type =
+                fmt::format("::{}::messages::{}", schema_name, m.name);
+        }
+
+        message_context.public_type = fmt::format(
+            "::{schema}::messages::{name}",
+            fmt::arg("schema", schema_name),
+            fmt::arg("name", m.name));
 
         const auto& header_type = header_context.public_type;
         const auto size_bytes_impl = make_level_size_bytes_impl(
@@ -1648,8 +1645,6 @@ R"(
         const auto implementation = fmt::format(
             // clang-format off
 R"(
-{groups}
-
 template<typename Byte>
 class {name} : public ::sbepp::detail::message_base<
     Byte, {header_type}<Byte>>
@@ -1675,23 +1670,29 @@ class {name} : public ::sbepp::detail::message_base<
 }};
 )",
             // clang-format on
-            fmt::arg("name", message_context.impl_name),
+            fmt::arg("name", message_context.mangled_name.value_or((m.name))),
             fmt::arg("header_type", header_type),
             fmt::arg("accessors", accessors),
             fmt::arg("size_getter", size_bytes_impl),
             fmt::arg("header_filler", make_message_header_filler(m)),
-            fmt::arg("groups", groups),
             fmt::arg("visit_children_impl", visit_children_impl),
             fmt::arg("tag", message_context.tag));
 
-        message_context.public_type = fmt::format(
-            "::{schema}::messages::{name}",
-            fmt::arg("schema", schema_name),
-            fmt::arg("name", m.name));
-
-        const auto alias = make_alias(m);
         const auto traits = traits_gen->make_message_traits(m);
-        on_message_cb(m.name, implementation, alias, dependencies, traits);
+
+        if(message_context.mangled_name)
+        {
+            on_message_cb(
+                m.name,
+                groups + implementation,
+                make_alias(m),
+                dependencies,
+                traits);
+        }
+        else
+        {
+            on_message_cb(m.name, groups, implementation, dependencies, traits);
+        }
     }
 };
 } // namespace sbepp::sbeppc