Skip to content

Comments

refactor(rd_gen_to_dag): removed assert#654

Merged
atsushi421 merged 2 commits intomainfrom
fix_build_dag
Sep 24, 2025
Merged

refactor(rd_gen_to_dag): removed assert#654
atsushi421 merged 2 commits intomainfrom
fix_build_dag

Conversation

@kobayu858
Copy link
Contributor

Description

Removed duplicate error checks. Added new error checks.

Related links

How was this PR tested?

Notes for reviewers

Signed-off-by: kobayu858 <yutaro.kobayashi.2@tier4.jp>
Signed-off-by: kobayu858 <yutaro.kobayashi.2@tier4.jp>
@atsushi421
Copy link
Contributor

@kobayu858
Could you explain where the duplicate part is?

@kobayu858
Copy link
Contributor Author

@atsushi421

My explanation was insufficient. I apologize.
The existing assertions had incorrect conditions to begin with.

For example, the following section should be
in_links_num > 0 instead of in_links_num == 0.
Considering this premise, when in_links_num is 0, it can be detected by the match statement error LinkNumError::Input.

This occurred in three places, so I fixed it.

    assert!(
        in_links_num == 0,
        "LinkNumError::NoInput: dag_id={dag_id}, node_id={node_id}"
    );

    match in_links_num {
        1 => register_sink!(dag, node_data, sched_type, u64),
        2 => register_sink!(dag, node_data, sched_type, u64, u64),
        3 => register_sink!(dag, node_data, sched_type, u64, u64, u64),
        _ => Err(LinkNumError::Input(dag_id, node_id)),
    }
            LinkNumError::Input(dag_id, node_id) => {
                write!(f, "DAG#{dag_id} Node#{node_id} has no input links")
            }

Copy link
Contributor

@atsushi421 atsushi421 left a comment

Choose a reason for hiding this comment

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

I got it. LGTM.

@atsushi421 atsushi421 merged commit f6f0988 into main Sep 24, 2025
1 check passed
@atsushi421 atsushi421 deleted the fix_build_dag branch September 24, 2025 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants