From d79937fb62b7ef5e8c4efd27daf8e5f6e64f9333 Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Tue, 15 Jun 2021 21:03:55 -0400 Subject: [PATCH] Improve forward declaring types (#596) * improve forward declare type * display type differences * make codacy happy --- common/type_system/Type.cpp | 332 +++++++++++++++++- common/type_system/Type.h | 22 +- common/type_system/TypeSystem.cpp | 129 +++++-- common/type_system/TypeSystem.h | 9 +- common/type_system/deftype.cpp | 4 +- decompiler/config/all-types.gc | 3 +- decompiler/util/DecompilerTypeSystem.cpp | 8 +- docs/markdown/progress-notes/changelog.md | 4 +- goal_src/kernel-defs.gc | 2 +- goalc/compiler/compilation/Define.cpp | 4 - goalc/compiler/compilation/Type.cpp | 8 +- .../reference/all_forward_declarations.gc | 9 +- test/test_type_system.cpp | 2 +- 13 files changed, 459 insertions(+), 77 deletions(-) diff --git a/common/type_system/Type.cpp b/common/type_system/Type.cpp index efcae0c3f..33af71736 100644 --- a/common/type_system/Type.cpp +++ b/common/type_system/Type.cpp @@ -35,6 +35,30 @@ bool MethodInfo::operator==(const MethodInfo& other) const { defined_in_type == other.defined_in_type && other.no_virtual == no_virtual; } +std::string MethodInfo::diff(const MethodInfo& other) const { + std::string result; + if (id != other.id) { + result += fmt::format("id: {} vs. {}\n", id, other.id); + } + + if (name != other.name) { + result += fmt::format("name: {} vs. {}\n", name, other.name); + } + + if (type != other.type) { + result += fmt::format("type: {} vs. {}\n", type.print(), other.type.print()); + } + + if (defined_in_type != other.defined_in_type) { + result += fmt::format("defined_in_type: {} vs. {}\n", defined_in_type, other.defined_in_type); + } + + if (no_virtual != other.no_virtual) { + result += fmt::format("no_virtual: {} vs. {}\n", no_virtual, other.no_virtual); + } + return result; +} + /*! * Print a one-line description of a method (name and type) */ @@ -97,6 +121,48 @@ bool Field::operator==(const Field& other) const { // clang-format on } +std::string Field::diff(const Field& other) const { + std::string result; + if (m_name != other.m_name) { + result += fmt::format("name: {} vs. {}\n", m_name, other.m_name); + } + + if (m_type != other.m_type) { + result += fmt::format("type: {} vs. {}\n", m_type.print(), other.m_type.print()); + } + + if (m_offset != other.m_offset) { + result += fmt::format("offset: {} vs. {}\n", m_offset, other.m_offset); + } + + if (m_inline != other.m_inline) { + result += fmt::format("inline: {} vs. {}\n", m_inline, other.m_inline); + } + + if (m_dynamic != other.m_dynamic) { + result += fmt::format("dynamic: {} vs. {}\n", m_dynamic, other.m_dynamic); + } + + if (m_array != other.m_array) { + result += fmt::format("array: {} vs. {}\n", m_array, other.m_array); + } + + if (m_array_size != other.m_array_size) { + result += fmt::format("array_size: {} vs. {}\n", m_array_size, other.m_array_size); + } + + if (m_alignment != other.m_alignment) { + result += fmt::format("alignment: {} vs. {}\n", m_alignment, other.m_alignment); + } + + if (m_skip_in_static_decomp != other.m_skip_in_static_decomp) { + result += fmt::format("skip_in_static_decomp: {} vs. {}\n", m_skip_in_static_decomp, + other.m_skip_in_static_decomp); + } + + return result; +} + ///////////// // Type ///////////// @@ -143,11 +209,64 @@ std::string Type::get_parent() const { * Compare the name/parent/method info for equality. The more complete operator== should be used * to check if two types are really identical. */ -bool Type::is_equal(const Type& other) const { - return m_parent == other.m_parent && m_name == other.m_name && m_is_boxed == other.m_is_boxed && - m_methods == other.m_methods && m_new_method_info == other.m_new_method_info && +bool Type::common_type_info_equal(const Type& other) const { + // clang-format off + return m_methods == other.m_methods && + m_new_method_info == other.m_new_method_info && m_new_method_info_defined == other.m_new_method_info_defined && + m_parent == other.m_parent && + m_name == other.m_name && + m_allow_in_runtime == other.m_allow_in_runtime && + m_runtime_name == other.m_runtime_name && + m_is_boxed == other.m_is_boxed && m_heap_base == other.m_heap_base; + // clang-format on +} + +std::string Type::common_type_info_diff(const Type& other) const { + std::string result; + if (m_methods != other.m_methods) { + if (m_methods.size() != other.m_methods.size()) { + result += fmt::format("Number of additional methods {} vs. {}\n", m_methods.size(), + other.m_methods.size()); + } + + for (size_t i = 0; i < std::min(m_methods.size(), other.m_methods.size()); i++) { + if (m_methods.at(i) != other.m_methods.at(i)) { + result += fmt::format("Method def {} ({}/{}):\n", i, m_methods.at(i).name, + other.m_methods.at(i).name); + result += m_methods.at(i).diff(other.m_methods.at(i)); + result += "\n"; + } + } + } + if (m_new_method_info != other.m_new_method_info) { + result += m_new_method_info.diff(other.m_new_method_info); + } + if (m_new_method_info_defined != other.m_new_method_info_defined) { + result += fmt::format("new_method_info_defined: {} vs. {}\n", m_new_method_info_defined, + other.m_new_method_info_defined); + } + if (m_parent != other.m_parent) { + result += fmt::format("parent: {} vs. {}\n", m_parent, other.m_parent); + } + if (m_name != other.m_name) { + result += fmt::format("name: {} vs. {}\n", m_name, other.m_name); + } + if (m_allow_in_runtime != other.m_allow_in_runtime) { + result += + fmt::format("allow_in_runtime: {} vs. {}\n", m_allow_in_runtime, other.m_allow_in_runtime); + } + if (m_runtime_name != other.m_runtime_name) { + result += fmt::format("runtime_name: {} vs. {}\n", m_runtime_name, other.m_runtime_name); + } + if (m_is_boxed != other.m_is_boxed) { + result += fmt::format("is_boxed: {} vs. {}\n", m_is_boxed, other.m_is_boxed); + } + if (m_heap_base != other.m_heap_base) { + result += fmt::format("heap_base: {} vs. {}\n", m_heap_base, other.m_heap_base); + } + return result; } /*! @@ -250,6 +369,18 @@ std::string Type::print_method_info() const { return result; } +std::string Type::incompatible_diff(const Type& other) const { + return fmt::format("diff is not implemented between {} and {}\n", typeid((*this)).name(), + typeid(other).name()); +} + +std::string Type::diff(const Type& other) const { + std::string result; + result += common_type_info_diff(other); + result += diff_impl(other); + return result; +} + ///////////// // NullType ///////////// @@ -305,6 +436,14 @@ bool NullType::operator==(const Type& other) const { return this == &other; } +std::string NullType::diff_impl(const Type& other) const { + if ((*this) != other) { + return "NullType error"; + } else { + return ""; + } +} + ///////////// // ValueType ///////////// @@ -419,7 +558,7 @@ bool ValueType::operator==(const Type& other) const { auto* p_other = dynamic_cast(&other); // clang-format off - return other.is_equal(*this) && + return other.common_type_info_equal(*this) && m_size == p_other->m_size && m_sign_extend == p_other->m_sign_extend && m_reg_kind == p_other->m_reg_kind && @@ -427,6 +566,29 @@ bool ValueType::operator==(const Type& other) const { // clang-format on } +std::string ValueType::diff_impl(const Type& other_) const { + if (typeid(ValueType) != typeid(other_)) { + return Type::incompatible_diff(other_); + } + + std::string result; + auto& other = *dynamic_cast(&other_); + + if (m_size != other.m_size) { + result += fmt::format("size: {} vs. {}\n", m_size, other.m_size); + } + if (m_offset != other.m_offset) { + result += fmt::format("offset: {} vs. {}\n", m_offset, other.m_offset); + } + if (m_sign_extend != other.m_sign_extend) { + result += fmt::format("sign_extend: {} vs. {}\n", m_sign_extend, other.m_sign_extend); + } + if (m_reg_kind != other.m_reg_kind) { + result += fmt::format("reg_kind: {} vs. {}\n", (int)m_reg_kind, (int)other.m_reg_kind); + } + return result; +} + ///////////////// // ReferenceType ///////////////// @@ -515,15 +677,71 @@ bool StructureType::operator==(const Type& other) const { auto* p_other = dynamic_cast(&other); // clang-format off - return other.is_equal(*this) && + return other.common_type_info_equal(*this) && m_fields == p_other->m_fields && m_dynamic == p_other->m_dynamic && m_size_in_mem == p_other->m_size_in_mem && m_pack == p_other->m_pack && - m_allow_misalign == p_other->m_allow_misalign; + m_allow_misalign == p_other->m_allow_misalign && + m_offset == p_other->m_offset && + m_idx_of_first_unique_field == p_other->m_idx_of_first_unique_field; // clang-format on } +std::string StructureType::diff_impl(const Type& other_) const { + if (typeid(*this) != typeid(other_)) { + return incompatible_diff(other_); + } + + auto& other = *dynamic_cast(&other_); + return diff_structure_common(other); +} + +std::string StructureType::diff_structure_common(const StructureType& other) const { + std::string result; + if (m_fields != other.m_fields) { + if (m_fields.size() != other.m_fields.size()) { + result += fmt::format("Number of fields {} vs. {}\n", m_fields.size(), other.m_fields.size()); + } + + for (size_t i = 0; i < std::min(m_fields.size(), other.m_fields.size()); i++) { + if (m_fields.at(i) != other.m_fields.at(i)) { + result += fmt::format("field {} ({}/{}):\n", i, m_fields.at(i).name(), + other.m_fields.at(i).name()); + result += m_fields.at(i).diff(other.m_fields.at(i)); + result += "\n"; + } + } + } + + if (m_dynamic != other.m_dynamic) { + result += fmt::format("dynamic: {} vs. {}\n", m_dynamic, other.m_dynamic); + } + + if (m_size_in_mem != other.m_size_in_mem) { + result += fmt::format("size_in_mem: {} vs. {}\n", m_size_in_mem, other.m_size_in_mem); + } + + if (m_pack != other.m_pack) { + result += fmt::format("pack: {} vs. {}\n", m_pack, other.m_pack); + } + + if (m_allow_misalign != other.m_allow_misalign) { + result += fmt::format("allow_misalign: {} vs. {}\n", m_allow_misalign, other.m_allow_misalign); + } + + if (m_offset != other.m_offset) { + result += fmt::format("offset: {} vs. {}\n", m_offset, other.m_offset); + } + + if (m_idx_of_first_unique_field != other.m_idx_of_first_unique_field) { + result += fmt::format("idx_of_first_unique_field: {} vs. {}\n", m_idx_of_first_unique_field, + other.m_idx_of_first_unique_field); + } + + return result; +} + int StructureType::get_size_in_memory() const { return m_size_in_mem; } @@ -637,16 +855,33 @@ bool BasicType::operator==(const Type& other) const { auto* p_other = dynamic_cast(&other); // clang-format off - return other.is_equal(*this) && + return other.common_type_info_equal(*this) && m_fields == p_other->m_fields && m_dynamic == p_other->m_dynamic && m_size_in_mem == p_other->m_size_in_mem && m_pack == p_other->m_pack && m_allow_misalign == p_other->m_allow_misalign && + m_offset == p_other->m_offset && + m_idx_of_first_unique_field == p_other->m_idx_of_first_unique_field && m_final == p_other->m_final; // clang-format on } +std::string BasicType::diff_impl(const Type& other_) const { + if (typeid(*this) != typeid(other_)) { + return incompatible_diff(other_); + } + + auto& other = *dynamic_cast(&other_); + std::string result = diff_structure_common(other); + + if (m_final != other.m_final) { + result += fmt::format("final: {} vs. {}\n", m_final, other.m_final); + } + + return result; +} + ///////////////// // Bitfield ///////////////// @@ -659,6 +894,28 @@ bool BitField::operator==(const BitField& other) const { other.m_size == m_size; } +std::string BitField::diff(const BitField& other) const { + std::string result; + + if (m_type != other.m_type) { + result += fmt::format("type: {} vs. {}\n", m_type.print(), other.m_type.print()); + } + + if (m_name != other.m_name) { + result += fmt::format("name: {} vs. {}\n", m_name, other.m_name); + } + + if (m_offset != other.m_offset) { + result += fmt::format("offset: {} vs. {}\n", m_offset, other.m_offset); + } + + if (m_size != other.m_size) { + result += fmt::format("size: {} vs. {}\n", m_size, other.m_size); + } + + return result; +} + BitFieldType::BitFieldType(std::string parent, std::string name, int size, bool sign_extend) : ValueType(std::move(parent), std::move(name), false, size, sign_extend, RegClass::GPR_64) {} @@ -693,7 +950,45 @@ bool BitFieldType::operator==(const Type& other) const { } auto* p_other = dynamic_cast(&other); - return other.is_equal(*this) && m_fields == p_other->m_fields; + return other.common_type_info_equal(*this) && m_fields == p_other->m_fields; +} + +std::string BitFieldType::diff_impl(const Type& other_) const { + if (typeid(BitFieldType) != typeid(other_)) { + return Type::incompatible_diff(other_); + } + + std::string result; + auto& other = *dynamic_cast(&other_); + + if (m_size != other.m_size) { + result += fmt::format("size: {} vs. {}\n", m_size, other.m_size); + } + if (m_offset != other.m_offset) { + result += fmt::format("offset: {} vs. {}\n", m_offset, other.m_offset); + } + if (m_sign_extend != other.m_sign_extend) { + result += fmt::format("sign_extend: {} vs. {}\n", m_sign_extend, other.m_sign_extend); + } + if (m_reg_kind != other.m_reg_kind) { + result += fmt::format("reg_kind: {} vs. {}\n", (int)m_reg_kind, (int)other.m_reg_kind); + } + + if (m_fields != other.m_fields) { + if (m_fields.size() != other.m_fields.size()) { + result += fmt::format("Number of fields {} vs. {}\n", m_fields.size(), other.m_fields.size()); + } + + for (size_t i = 0; i < std::min(m_fields.size(), other.m_fields.size()); i++) { + if (m_fields.at(i) != other.m_fields.at(i)) { + result += fmt::format("field {} ({}/{}):\n", i, m_fields.at(i).name(), + other.m_fields.at(i).name()); + result += m_fields.at(i).diff(other.m_fields.at(i)); + result += "\n"; + } + } + } + return result; } ///////////////// @@ -723,6 +1018,25 @@ bool EnumType::operator==(const Type& other) const { } auto* p_other = dynamic_cast(&other); - return other.is_equal(*this) && (m_entries == p_other->m_entries) && + return other.common_type_info_equal(*this) && (m_entries == p_other->m_entries) && (m_is_bitfield == p_other->m_is_bitfield); } + +std::string EnumType::diff_impl(const Type& other_) const { + if (typeid(EnumType) != typeid(other_)) { + return Type::incompatible_diff(other_); + } + + std::string result; + auto& other = *dynamic_cast(&other_); + + if (m_is_bitfield != other.m_is_bitfield) { + result += fmt::format("is_bitfield: {} vs. {}\n", m_is_bitfield, other.m_is_bitfield); + } + + if (m_entries != other.m_entries) { + result += "Entries are different.\n"; + } + + return result; +} \ No newline at end of file diff --git a/common/type_system/Type.h b/common/type_system/Type.h index 7558df013..277b0f3e3 100644 --- a/common/type_system/Type.h +++ b/common/type_system/Type.h @@ -23,7 +23,9 @@ struct MethodInfo { bool no_virtual = false; bool operator==(const MethodInfo& other) const; + bool operator!=(const MethodInfo& other) const { return !((*this) == other); } std::string print_one_line() const; + std::string diff(const MethodInfo& other) const; }; /*! @@ -57,13 +59,15 @@ class Type { virtual int get_inline_array_start_alignment() const = 0; virtual bool operator==(const Type& other) const = 0; + std::string diff(const Type& other) const; // print some information for debugging virtual std::string print() const = 0; bool operator!=(const Type& other) const { return !(*this == other); } - bool is_equal(const Type& other) const; + bool common_type_info_equal(const Type& other) const; + std::string common_type_info_diff(const Type& other) const; bool has_parent() const; std::string get_name() const; @@ -98,6 +102,8 @@ class Type { protected: Type(std::string parent, std::string name, bool is_boxed, int heap_base); + virtual std::string diff_impl(const Type& other) const = 0; + std::string incompatible_diff(const Type& other) const; std::vector m_methods; MethodInfo m_new_method_info; @@ -129,6 +135,7 @@ class NullType : public Type { int get_in_memory_alignment() const override; std::string print() const override; bool operator==(const Type& other) const override; + std::string diff_impl(const Type& other) const override; ~NullType() = default; }; @@ -155,11 +162,11 @@ class ValueType : public Type { int get_inline_array_start_alignment() const override; std::string print() const override; bool operator==(const Type& other) const override; + std::string diff_impl(const Type& other) const override; ~ValueType() = default; - void inherit(const ValueType* parent); - private: + protected: friend class TypeSystem; void set_offset(int offset); int m_size = -1; @@ -202,6 +209,8 @@ class Field { bool skip_in_decomp() const { return m_skip_in_static_decomp; } bool user_placed() const { return m_placed_by_user; } bool operator==(const Field& other) const; + bool operator!=(const Field& other) const { return !((*this) == other); } + std::string diff(const Field& other) const; int alignment() const { assert(m_alignment != -1); @@ -244,6 +253,8 @@ class StructureType : public ReferenceType { void inherit(StructureType* parent); const std::vector& fields() const { return m_fields; } bool operator==(const Type& other) const override; + std::string diff_impl(const Type& other) const override; + std::string diff_structure_common(const StructureType& other) const; int get_size_in_memory() const override; int get_offset() const override; int get_in_memory_alignment() const override; @@ -290,6 +301,7 @@ class BasicType : public StructureType { void set_final() { m_final = true; } ~BasicType() = default; bool operator==(const Type& other) const override; + std::string diff_impl(const Type& other) const override; protected: bool m_final = false; @@ -304,6 +316,8 @@ class BitField { int size() const { return m_size; } const TypeSpec& type() const { return m_type; } bool operator==(const BitField& other) const; + bool operator!=(const BitField& other) const { return !((*this) == other); } + std::string diff(const BitField& other) const; std::string print() const; private: @@ -320,6 +334,7 @@ class BitFieldType : public ValueType { std::string print() const override; bool operator==(const Type& other) const override; const std::vector& fields() const { return m_fields; } + std::string diff_impl(const Type& other) const override; private: friend class TypeSystem; @@ -336,6 +351,7 @@ class EnumType : public ValueType { bool operator==(const Type& other) const override; const std::unordered_map& entries() const { return m_entries; } bool is_bitfield() const { return m_is_bitfield; } + std::string diff_impl(const Type& other) const override; private: friend class TypeSystem; diff --git a/common/type_system/TypeSystem.cpp b/common/type_system/TypeSystem.cpp index ea224cfa0..494b358bf 100644 --- a/common/type_system/TypeSystem.cpp +++ b/common/type_system/TypeSystem.cpp @@ -57,8 +57,9 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr type) m_types[name] = std::move(type); } else { throw_typesystem_error( - "Inconsistent type definition. Type {} was originally\n{}\nand is redefined as\n{}\n", - kv->second->get_name(), kv->second->print(), type->print()); + "Inconsistent type definition. Type {} was originally\n{}\nand is redefined " + "as\n{}\nDiff:\n{}\n", + kv->second->get_name(), kv->second->print(), type->print(), kv->second->diff(*type)); } } } else { @@ -79,6 +80,14 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr type) } m_types[name] = std::move(type); + auto fwd_it = m_forward_declared_types.find(name); + if (fwd_it != m_forward_declared_types.end()) { + // need to check parent is correct. + if (!tc(TypeSpec(fwd_it->second), TypeSpec(name))) { + throw_typesystem_error("Type {} was original declared as a child of {}, but is not.\n", + name, fwd_it->second); + } + } m_forward_declared_types.erase(name); } @@ -90,9 +99,19 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr type) * This will allow the type system to generate TypeSpecs for this type, but not access detailed * information, or know the exact size. */ -void TypeSystem::forward_declare_type(const std::string& name) { - if (m_types.find(name) == m_types.end()) { - m_forward_declared_types[name] = TYPE; +void TypeSystem::forward_declare_type_as_type(const std::string& name) { + auto type_it = m_types.find(name); + if (type_it != m_types.end()) { + return; + } + + auto it = m_forward_declared_types.find(name); + if (it == m_forward_declared_types.end()) { + m_forward_declared_types[name] = "object"; + } else { + throw_typesystem_error( + "Tried to forward declare {} as a type multiple times. Previous: {} Current: object", name, + it->second); } } @@ -101,20 +120,60 @@ void TypeSystem::forward_declare_type(const std::string& name) { * This allows the type to be used in a few specific places. For instance a basic can have * a field where an element is the same type. */ -void TypeSystem::forward_declare_type_as_basic(const std::string& name) { - if (m_types.find(name) == m_types.end()) { - m_forward_declared_types[name] = BASIC; - } -} +void TypeSystem::forward_declare_type_as(const std::string& new_type, + const std::string& parent_type) { + auto type_it = m_types.find(new_type); + if (type_it != m_types.end()) { + auto parent_it = m_types.find(parent_type); + if (parent_it == m_types.end()) { + throw_typesystem_error( + "Got a forward declaration for known type {} where the parent {} is unknown", new_type, + parent_type); + } -/*! - * Inform the type system that there will eventually be a type named "name" and that it's a - * structure. This allows the type to be used in a few specific places. For instance a structure can - * have a field where an element is the same type. - */ -void TypeSystem::forward_declare_type_as_structure(const std::string& name) { - if (m_types.find(name) == m_types.end()) { - m_forward_declared_types[name] = STRUCTURE; + if (!tc(TypeSpec(parent_type), TypeSpec(new_type))) { + throw_typesystem_error( + "Got a forward definition that type {} is a {} which disagrees with existing " + "fully-defined types.", + new_type, parent_type); + } + + // ignore forward declaration + return; + } + + auto fwd_it = m_forward_declared_types.find(new_type); + if (fwd_it == m_forward_declared_types.end()) { + m_forward_declared_types[new_type] = parent_type; + } else { + if (fwd_it->second != parent_type) { + auto old_parent_it = m_types.find(fwd_it->second); + auto new_parent_it = m_types.find(parent_type); + + auto old_ts = TypeSpec(old_parent_it->second->get_name()); + auto new_ts = TypeSpec(new_parent_it->second->get_name()); + + if (old_parent_it != m_types.end() && new_parent_it != m_types.end()) { + if (tc(old_ts, new_ts)) { + // new is more specific or equal to old: + m_forward_declared_types[new_type] = new_ts.base_type(); + } else if (tc(new_ts, old_ts)) { + // old is more specific or equal to new: + } else { + throw_typesystem_error( + "Got a forward declaration that type {} is a {}, which disagrees with a previous " + "forward declaration that it was a {} (incompatible types)\n", + new_type, parent_type, fwd_it->second); + } + } else { + // not enough info to know if this is safe or not!. + throw_typesystem_error( + "Got a forward declaration that type {} is a {}, which disagrees with a previous " + "forward declaration that it was a {} (not enough information to know this is okay, " + "forward declare more types to resolve this)\n", + new_type, parent_type, fwd_it->second); + } + } } } @@ -312,20 +371,28 @@ Type* TypeSystem::lookup_type_allow_partial_def(const std::string& name) const { return kv->second.get(); } - auto fwd_dec = m_forward_declared_types.find(name); - if (fwd_dec != m_forward_declared_types.end()) { - if (fwd_dec->second == STRUCTURE) { - return lookup_type("structure"); - } else if (fwd_dec->second == BASIC) { - return lookup_type("basic"); - } else { - throw_typesystem_error( - "The type {} is known to be a type, but has not been defined with deftype " - "or properly forward declared with declare-type\n", - name); + Type* result = nullptr; + std::string current_name = name; + + while (!result) { + auto fwd_dec = m_forward_declared_types.find(current_name); + if (fwd_dec == m_forward_declared_types.end()) { + if (current_name == name) { + throw_typesystem_error("The type {} is unknown (2).\n", name); + } else { + throw_typesystem_error("When looking up forward defined type {}, could not find a type {}.", + name, current_name); + } + } + current_name = fwd_dec->second; + + auto type_lookup = m_types.find(current_name); + if (type_lookup != m_types.end()) { + result = type_lookup->second.get(); } } - throw_typesystem_error("The type {} is unknown.\n", name); + + return result; } MethodInfo TypeSystem::declare_method(const std::string& type_name, @@ -803,7 +870,7 @@ void TypeSystem::add_builtin_types() { uint_type->disallow_in_runtime(); // Methods and Fields - forward_declare_type_as_structure("memory-usage-block"); + forward_declare_type_as("memory-usage-block", "basic"); // OBJECT declare_method(obj_type, "new", false, diff --git a/common/type_system/TypeSystem.h b/common/type_system/TypeSystem.h index 722c78c06..da521b9cc 100644 --- a/common/type_system/TypeSystem.h +++ b/common/type_system/TypeSystem.h @@ -91,9 +91,8 @@ class TypeSystem { TypeSystem(); Type* add_type(const std::string& name, std::unique_ptr type); - void forward_declare_type(const std::string& name); - void forward_declare_type_as_basic(const std::string& name); - void forward_declare_type_as_structure(const std::string& name); + void forward_declare_type_as_type(const std::string& name); + void forward_declare_type_as(const std::string& new_type, const std::string& parent_type); std::string get_runtime_type(const TypeSpec& ts); DerefInfo get_deref_info(const TypeSpec& ts) const; @@ -241,10 +240,8 @@ class TypeSystem { RegClass reg = RegClass::GPR_64); void builtin_structure_inherit(StructureType* st); - enum ForwardDeclareKind { TYPE, STRUCTURE, BASIC }; - std::unordered_map> m_types; - std::unordered_map m_forward_declared_types; + std::unordered_map m_forward_declared_types; std::vector> m_old_types; bool m_allow_redefinition = false; diff --git a/common/type_system/deftype.cpp b/common/type_system/deftype.cpp index e5ae02f9d..b59ae6b8f 100644 --- a/common/type_system/deftype.cpp +++ b/common/type_system/deftype.cpp @@ -464,7 +464,7 @@ DeftypeResult parse_deftype(const goos::Object& deftype, TypeSystem* ts) { parent_type_name)); } new_type->inherit(pto); - ts->forward_declare_type_as_basic(name); + ts->forward_declare_type_as(name, "basic"); auto sr = parse_structure_def(new_type.get(), ts, field_list_obj, options_obj); result.flags = sr.flags; result.create_runtime_type = sr.generate_runtime_type; @@ -488,7 +488,7 @@ DeftypeResult parse_deftype(const goos::Object& deftype, TypeSystem* ts) { auto pto = dynamic_cast(ts->lookup_type(parent_type)); assert(pto); new_type->inherit(pto); - ts->forward_declare_type_as_structure(name); + ts->forward_declare_type_as(name, "structure"); auto sr = parse_structure_def(new_type.get(), ts, field_list_obj, options_obj); result.flags = sr.flags; result.create_runtime_type = sr.generate_runtime_type; diff --git a/decompiler/config/all-types.gc b/decompiler/config/all-types.gc index e06dc3271..40ddbc2eb 100644 --- a/decompiler/config/all-types.gc +++ b/decompiler/config/all-types.gc @@ -2437,7 +2437,8 @@ :flag-assert #x90000004c ) -(declare-type entity basic) +(declare-type res-lump basic) +(declare-type entity res-lump) (deftype ambient-sound (basic) ((spec sound-spec :offset-assert 4) (playing-id sound-id :offset-assert 8) diff --git a/decompiler/util/DecompilerTypeSystem.cpp b/decompiler/util/DecompilerTypeSystem.cpp index b35dc1e6c..f8893781c 100644 --- a/decompiler/util/DecompilerTypeSystem.cpp +++ b/decompiler/util/DecompilerTypeSystem.cpp @@ -74,13 +74,7 @@ void DecompilerTypeSystem::parse_type_defs(const std::vector& file_ if (!cdr(*rest).is_empty_list()) { throw std::runtime_error("malformed declare-type"); } - if (type_kind.as_symbol()->name == "basic") { - ts.forward_declare_type_as_basic(type_name.as_symbol()->name); - } else if (type_kind.as_symbol()->name == "structure") { - ts.forward_declare_type_as_structure(type_name.as_symbol()->name); - } else { - throw std::runtime_error("bad declare-type"); - } + ts.forward_declare_type_as(type_name.as_symbol()->name, type_kind.as_symbol()->name); } else if (car(o).as_symbol()->name == "defenum") { parse_defenum(cdr(o), &ts); // so far, enums are never runtime types so there's no symbol for them. diff --git a/docs/markdown/progress-notes/changelog.md b/docs/markdown/progress-notes/changelog.md index a8cc22429..c8d8fe3ee 100644 --- a/docs/markdown/progress-notes/changelog.md +++ b/docs/markdown/progress-notes/changelog.md @@ -162,4 +162,6 @@ - Fixed a bug where reader macros without a following form would trigger an assert. - It is now possible to take the address of a lexical variable. The variable will be spilled to the stack automatically. - GOOS supports `string-ref`, `string-length`, `ash`, and characters can now be treated as a signed 8-bit number -- Fixed a bug where saved xmm registers might be clobbered when calling a C++ function that wasn't `format`. \ No newline at end of file +- Fixed a bug where saved xmm registers might be clobbered when calling a C++ function that wasn't `format`. +- The `declare-type` form now supports any parent type. The type system will do a better job of trying to make things work out when only part of the type hierarchy is defined, and you can now chain type forward declarations. The compiler is stricter and will not accept forward declarations that are possibly incompatible. Instead, forward declare enough types and their parents for the compiler to be able to figure it out. +- The `deftype` form is more strict and will throw an error if the type definition is in any way incompatible with existing forward declarations of types. \ No newline at end of file diff --git a/goal_src/kernel-defs.gc b/goal_src/kernel-defs.gc index a11e1fcbc..502c2e8f0 100644 --- a/goal_src/kernel-defs.gc +++ b/goal_src/kernel-defs.gc @@ -147,7 +147,7 @@ (define-extern gs-store-image (function object object object)) (define-extern flush-cache (function int none)) ;; cpad-open -(declare-type cpad-info structure) +(declare-type cpad-info basic) (define-extern cpad-open (function cpad-info int cpad-info)) (define-extern cpad-get-data (function cpad-info cpad-info)) (define-extern install-handler (function int (function int) int)) ;; check return val diff --git a/goalc/compiler/compilation/Define.cpp b/goalc/compiler/compilation/Define.cpp index 2399645c5..b8e1c838b 100644 --- a/goalc/compiler/compilation/Define.cpp +++ b/goalc/compiler/compilation/Define.cpp @@ -90,10 +90,6 @@ Val* Compiler::compile_define_extern(const goos::Object& form, const goos::Objec } } - if (new_type == m_ts.make_typespec("type")) { - m_ts.forward_declare_type(symbol_string(sym)); - } - m_symbol_types[symbol_string(sym)] = new_type; m_symbol_info.add_fwd_dec(symbol_string(sym), form); return get_none(); diff --git a/goalc/compiler/compilation/Type.cpp b/goalc/compiler/compilation/Type.cpp index 91148bba2..c90c6b71e 100644 --- a/goalc/compiler/compilation/Type.cpp +++ b/goalc/compiler/compilation/Type.cpp @@ -1126,13 +1126,7 @@ Val* Compiler::compile_declare_type(const goos::Object& form, const goos::Object auto kind = symbol_string(args.unnamed.at(1)); auto type_name = symbol_string(args.unnamed.at(0)); - if (kind == "basic") { - m_ts.forward_declare_type_as_basic(type_name); - } else if (kind == "structure") { - m_ts.forward_declare_type_as_structure(type_name); - } else { - throw_compiler_error(form, "Invalid declare-type form: unrecognized option {}.", kind); - } + m_ts.forward_declare_type_as(type_name, kind); auto existing_type = m_symbol_types.find(type_name); if (existing_type != m_symbol_types.end() && existing_type->second != TypeSpec("type")) { diff --git a/test/decompiler/reference/all_forward_declarations.gc b/test/decompiler/reference/all_forward_declarations.gc index ab033cca9..317b08c31 100644 --- a/test/decompiler/reference/all_forward_declarations.gc +++ b/test/decompiler/reference/all_forward_declarations.gc @@ -535,7 +535,7 @@ ; This was likely originally defined inside `wind-h` ; but the decompiler won't output it, so we have to manually define it -(define-extern wind-work type) +(declare-type wind-work basic) (define-extern *wind-work* wind-work) (define-extern vector-y-angle (function vector float)) @@ -937,10 +937,11 @@ (unload-music) ) -(declare-type entity basic) +(declare-type res-lump basic) +(declare-type entity res-lump) ;; TODO - for prototype -(declare-type adgif-shader basic) +(declare-type adgif-shader structure) (define-extern adgif-shader-login-no-remap (function adgif-shader none)) ;; TODO - for video @@ -951,7 +952,7 @@ (define-extern *pause-context* font-context) (define-extern *profile-y* int) (define-extern get-aspect-ratio (function symbol)) -(declare-type video-parms basic) +(declare-type video-parms structure) (define-extern set-hud-aspect-ratio (function symbol symbol none)) (deftype progress (process) ((current-debug-string int32 :offset-assert 112) diff --git a/test/test_type_system.cpp b/test/test_type_system.cpp index 906794dc3..40ff67278 100644 --- a/test/test_type_system.cpp +++ b/test/test_type_system.cpp @@ -169,7 +169,7 @@ TEST(TypeSystem, ForwardDeclaration) { EXPECT_ANY_THROW(ts.make_typespec("test-type")); // after forward declaring, we should be able to create typespec, but not do a full lookup - ts.forward_declare_type("test-type"); + ts.forward_declare_type_as_type("test-type"); EXPECT_TRUE(ts.make_typespec("test-type").print() == "test-type"); EXPECT_ANY_THROW(ts.lookup_type("test-type"));