Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/spider/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ set(SPIDER_TDL_SHARED_SOURCES
tdl/parser/ast/node_impl/type_impl/primitive_impl/Bool.cpp
tdl/parser/ast/node_impl/type_impl/primitive_impl/Float.cpp
tdl/parser/ast/node_impl/type_impl/primitive_impl/Int.cpp
tdl/parser/ast/node_impl/type_impl/Struct.cpp
tdl/parser/ast/utils.cpp
CACHE INTERNAL
"spider task definition language shared source files"
Expand All @@ -227,6 +228,7 @@ set(SPIDER_TDL_SHARED_HEADERS
tdl/parser/ast/node_impl/type_impl/primitive_impl/Bool.hpp
tdl/parser/ast/node_impl/type_impl/primitive_impl/Float.hpp
tdl/parser/ast/node_impl/type_impl/primitive_impl/Int.hpp
tdl/parser/ast/node_impl/type_impl/Struct.hpp
tdl/parser/ast/utils.hpp
CACHE INTERNAL
"spider task definition language shared header files"
Expand Down
79 changes: 79 additions & 0 deletions src/spider/tdl/parser/ast/node_impl/type_impl/Struct.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#include "Struct.hpp"

#include <cstddef>
#include <memory>
#include <string>
#include <utility>

#include <fmt/format.h>
#include <ystdlib/error_handling/ErrorCode.hpp>
#include <ystdlib/error_handling/Result.hpp>

#include <spider/tdl/parser/ast/Node.hpp>
#include <spider/tdl/parser/ast/node_impl/Identifier.hpp>
#include <spider/tdl/parser/ast/node_impl/StructSpec.hpp>
#include <spider/tdl/parser/ast/utils.hpp>

using spider::tdl::parser::ast::node_impl::type_impl::Struct;
using StructErrorCodeCategory = ystdlib::error_handling::ErrorCategory<Struct::ErrorCodeEnum>;

template <>
auto StructErrorCodeCategory::name() const noexcept -> char const* {
return "spider::tdl::parser::ast::node_impl::type_impl::Struct";
}

template <>
auto StructErrorCodeCategory::message(Struct::ErrorCodeEnum error_enum) const -> std::string {
switch (error_enum) {
case Struct::ErrorCodeEnum::NullStructSpec:
return "The struct spec is NULL.";
case Struct::ErrorCodeEnum::StructSpecAlreadySet:
return "The struct spec is already set.";
case Struct::ErrorCodeEnum::StructSpecNameMismatch:
return "The struct spec name does not match the type name.";
default:
return "Unknown error code enum";
}
}

namespace spider::tdl::parser::ast::node_impl::type_impl {
auto Struct::create(std::unique_ptr<Node> name)
-> ystdlib::error_handling::Result<std::unique_ptr<Node>> {
YSTDLIB_ERROR_HANDLING_TRYV(validate_child_node_type<Identifier>(name.get()));

auto struct_node{std::make_unique<Struct>(Struct{})};
YSTDLIB_ERROR_HANDLING_TRYV(struct_node->add_child(std::move(name)));
return struct_node;
}

auto Struct::serialize_to_str(size_t indentation_level) const
-> ystdlib::error_handling::Result<std::string> {
return fmt::format(
"{}[Type[Struct]]:\n{}Name:\n{}",
create_indentation(indentation_level),
create_indentation(indentation_level + 1),
YSTDLIB_ERROR_HANDLING_TRYX(
// The factory function ensures that the first child is of type `Identifier`.
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
get_child_unsafe(0)->serialize_to_str(indentation_level + 2)
)
);
}
Comment thread
LinZhihao-723 marked this conversation as resolved.

auto Struct::set_spec(std::shared_ptr<StructSpec> spec) -> ystdlib::error_handling::Result<void> {
if (nullptr != m_spec) {
return Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecAlreadySet};
}

if (nullptr == spec) {
return Struct::ErrorCode{Struct::ErrorCodeEnum::NullStructSpec};
}

if (get_name() != spec->get_name()) {
return Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecNameMismatch};
}

m_spec = std::move(spec);
return ystdlib::error_handling::success();
}
} // namespace spider::tdl::parser::ast::node_impl::type_impl
77 changes: 77 additions & 0 deletions src/spider/tdl/parser/ast/node_impl/type_impl/Struct.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#ifndef SPIDER_TDL_PARSER_AST_NODE_IMPL_TYPE_IMPL_STRUCT_HPP
#define SPIDER_TDL_PARSER_AST_NODE_IMPL_TYPE_IMPL_STRUCT_HPP

#include <cstddef>
#include <cstdint>
#include <memory>
#include <string>
#include <string_view>

#include <ystdlib/error_handling/ErrorCode.hpp>
#include <ystdlib/error_handling/Result.hpp>

#include <spider/tdl/parser/ast/Node.hpp>
#include <spider/tdl/parser/ast/node_impl/Identifier.hpp>
#include <spider/tdl/parser/ast/node_impl/StructSpec.hpp>
#include <spider/tdl/parser/ast/node_impl/Type.hpp>

namespace spider::tdl::parser::ast::node_impl::type_impl {
class Struct : public Type {
public:
// Types
enum class ErrorCodeEnum : uint8_t {
NullStructSpec = 1,
StructSpecAlreadySet,
StructSpecNameMismatch,
};

using ErrorCode = ystdlib::error_handling::ErrorCode<ErrorCodeEnum>;

// Factory function
/**
* @param name
* @return A result containing a unique pointer to a new `Struct` instance with the given name
* on success, or an error code indicating the failure:
* - Forwards `validate_child_node_type`'s return values.
*/
[[nodiscard]] static auto create(std::unique_ptr<Node> name)
-> ystdlib::error_handling::Result<std::unique_ptr<Node>>;

// Methods implementing `Node`
[[nodiscard]] auto serialize_to_str(size_t indentation_level) const
-> ystdlib::error_handling::Result<std::string> override;

// Methods
[[nodiscard]] auto get_name() const -> std::string_view {
// The factory function ensures that the first child is of type `Identifier`.
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
return static_cast<Identifier const*>(get_child_unsafe(0))->get_name();
}

/**
* Sets the specification for this struct.
* @param spec
* @return A void result on success, or an error code indicating the failure:
* - ErrorCodeEnum::NullStructSpec if `spec` is a nullptr.
* - ErrorCodeEnum::StructSpecNameMismatch if `spec`'s name does not match the underlying name.
* - ErrorCodeEnum::StructSpecAlreadySet if the specification has already been set.
*/
[[nodiscard]] auto set_spec(std::shared_ptr<StructSpec> spec)
-> ystdlib::error_handling::Result<void>;

[[nodiscard]] auto get_spec() const -> StructSpec const* { return m_spec.get(); }

private:
// Constructor
Struct() = default;

// Variables
std::shared_ptr<StructSpec> m_spec;
};
} // namespace spider::tdl::parser::ast::node_impl::type_impl

YSTDLIB_ERROR_HANDLING_MARK_AS_ERROR_CODE_ENUM(
spider::tdl::parser::ast::node_impl::type_impl::Struct::ErrorCodeEnum
);

#endif // SPIDER_TDL_PARSER_AST_NODE_IMPL_TYPE_IMPL_STRUCT_HPP
67 changes: 67 additions & 0 deletions tests/tdl/test-parser-ast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <spider/tdl/parser/ast/node_impl/type_impl/primitive_impl/Bool.hpp>
#include <spider/tdl/parser/ast/node_impl/type_impl/primitive_impl/Float.hpp>
#include <spider/tdl/parser/ast/node_impl/type_impl/primitive_impl/Int.hpp>
#include <spider/tdl/parser/ast/node_impl/type_impl/Struct.hpp>

namespace {
TEST_CASE("test-ast-node", "[tdl][ast][Node]") {
Expand All @@ -37,6 +38,7 @@ TEST_CASE("test-ast-node", "[tdl][ast][Node]") {
using spider::tdl::parser::ast::node_impl::type_impl::primitive_impl::Bool;
using spider::tdl::parser::ast::node_impl::type_impl::primitive_impl::Float;
using spider::tdl::parser::ast::node_impl::type_impl::primitive_impl::Int;
using spider::tdl::parser::ast::node_impl::type_impl::Struct;
using ystdlib::error_handling::Result;

SECTION("Identifier") {
Expand Down Expand Up @@ -384,6 +386,71 @@ TEST_CASE("test-ast-node", "[tdl][ast][Node]") {
== StructSpec::ErrorCode{StructSpec::ErrorCodeEnum::EmptyStruct});
}
}

SECTION("Struct") {
constexpr std::string_view cTestStructName{"TestStruct"};

// Create a `StructSpec`
auto int_field_result{
NamedVar::create(Identifier::create("m_int"), Int::create(IntSpec::Int64))
};
REQUIRE_FALSE(int_field_result.has_error());
std::vector<std::unique_ptr<Node>> fields;
fields.emplace_back(std::move(int_field_result.value()));

auto struct_spec_result{StructSpec::create(
Identifier::create(std::string{cTestStructName}),
std::move(fields)
)};
REQUIRE_FALSE(struct_spec_result.has_error());
REQUIRE(nullptr != dynamic_cast<StructSpec const*>(struct_spec_result.value().get()));

SECTION("Struct with StructSpec") {
auto struct_result{Struct::create(Identifier::create(std::string{cTestStructName}))};
REQUIRE_FALSE(struct_result.has_error());
auto* struct_node{dynamic_cast<Struct*>(struct_result.value().get())};
REQUIRE(nullptr != struct_node);

REQUIRE(struct_node->get_num_children() == 1);
REQUIRE(cTestStructName == struct_node->get_name());
REQUIRE(nullptr == struct_node->get_spec());

constexpr std::string_view cExpectedSerializedResult{"[Type[Struct]]:\n"
" Name:\n"
" [Identifier]:TestStruct"};
auto const serialized_result{struct_node->serialize_to_str(0)};
REQUIRE_FALSE(serialized_result.has_error());
REQUIRE(serialized_result.value() == cExpectedSerializedResult);

// Ensure nullptr can't be set as `StructSpec`
auto const null_set_spec{struct_node->set_spec({})};
REQUIRE(null_set_spec.has_error());
REQUIRE(null_set_spec.error()
== Struct::ErrorCode{Struct::ErrorCodeEnum::NullStructSpec});
REQUIRE(nullptr == struct_node->get_spec());

// Set the `StructSpec` to the `Struct`
REQUIRE_FALSE(struct_node->set_spec(struct_spec_result.value()).has_error());
REQUIRE(nullptr != struct_node->get_spec());

// Ensure `StructSpec` can't be set again
auto const duplicated_set_spec{struct_node->set_spec(struct_spec_result.value())};
REQUIRE(duplicated_set_spec.has_error());
REQUIRE(duplicated_set_spec.error()
== Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecAlreadySet});
}

SECTION("Set spec to a wrong Struct") {
auto struct_result{Struct::create(Identifier::create("WrongStruct"))};
REQUIRE_FALSE(struct_result.has_error());
auto* struct_node{dynamic_cast<Struct*>(struct_result.value().get())};
REQUIRE(nullptr != struct_node);
auto const set_spec_result{struct_node->set_spec(struct_spec_result.value())};
REQUIRE(set_spec_result.has_error());
REQUIRE(set_spec_result.error()
== Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecNameMismatch});
}
Comment on lines +443 to +452

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Also assert struct state remains unchanged after a failed set_spec due to name mismatch.
Ensures no partial assignment occurs on error.

Apply this diff:

             auto const set_spec_result{struct_node->set_spec(struct_spec_result.value())};
             REQUIRE(set_spec_result.has_error());
             REQUIRE(set_spec_result.error()
                     == Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecNameMismatch});
+            // Ensure no spec was installed on error.
+            REQUIRE(struct_node->get_spec() == nullptr);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SECTION("Set spec to a wrong Struct") {
auto struct_result{Struct::create(Identifier::create("WrongStruct"))};
REQUIRE_FALSE(struct_result.has_error());
auto* struct_node{dynamic_cast<Struct*>(struct_result.value().get())};
REQUIRE(nullptr != struct_node);
auto const set_spec_result{struct_node->set_spec(struct_spec_result.value())};
REQUIRE(set_spec_result.has_error());
REQUIRE(set_spec_result.error()
== Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecNameMismatch});
}
SECTION("Set spec to a wrong Struct") {
auto struct_result{Struct::create(Identifier::create("WrongStruct"))};
REQUIRE_FALSE(struct_result.has_error());
auto* struct_node{dynamic_cast<Struct*>(struct_result.value().get())};
REQUIRE(nullptr != struct_node);
auto const set_spec_result{struct_node->set_spec(struct_spec_result.value())};
REQUIRE(set_spec_result.has_error());
REQUIRE(set_spec_result.error()
== Struct::ErrorCode{Struct::ErrorCodeEnum::StructSpecNameMismatch});
// Ensure no spec was installed on error.
REQUIRE(struct_node->get_spec() == nullptr);
}
🤖 Prompt for AI Agents
In tests/tdl/test-parser-ast.cpp around lines 442 to 451, after asserting that
set_spec returned a name-mismatch error, add assertions that the Struct object's
state was not mutated: confirm the struct's identifier/name is still
"WrongStruct" and that its spec pointer/member remains unset (or unchanged) so
no partial assignment occurred on error.

}
}
} // namespace

Expand Down