Hello Kanishka.

> This patch removes redundant caller-side HAS_ROWS checks before calls to
> a68_low_dup.
>
> a68_low_dup now performs its own HAS_ROWS checks internally, making
> these caller-side checks unnecessary.

Thank you for the patch, which I see is your first contribution to GCC.
Welcome! :)

The patch looks good, but for a tiny nit, explained below.

> Thanks,
> Kanishka Solanki
> From 5652830ca1aea247de25d09a3a486231919b49d1 Mon Sep 17 00:00:00 2001
> From: Kanishka Solanki <[email protected]>
> Date: Mon, 15 Jun 2026 20:54:06 +0530
> Subject: [PATCH] remove redundant HAS_ROWS check from caller side of
>  a68_low_dup
>
> a68_low_dup originally always performed a deep copy, so callers
> avoided invoking it for values without rows by checking HAS_ROWS
> beforehand.
>
> a68_low_dup now performs its own HAS_ROWS check and returns the
> original expression unchanged when a deep copy is not required.
> Remove the remaining caller-side HAS_ROWS checks, as they are
> redundant.
>
> gcc/algol68/ChangeLog
>
>       * a68-low-clauses.cc (a68_lower_collateral_clause): Do not check
>       for rows before calling a68_low_dup.
>       * a68-low-units.cc (collect_call_arguments): Likewise.
>
> Signed-off-by: Kanishka Solanki <[email protected]>
> ---
>  gcc/algol68/a68-low-clauses.cc | 1 -
>  gcc/algol68/a68-low-units.cc   | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/gcc/algol68/a68-low-clauses.cc b/gcc/algol68/a68-low-clauses.cc
> index 9fca4b872bc..f59dbcbb318 100644
> --- a/gcc/algol68/a68-low-clauses.cc
> +++ b/gcc/algol68/a68-low-clauses.cc
> @@ -1132,7 +1132,6 @@ a68_lower_collateral_clause (NODE_T *p ATTRIBUTE_UNUSED,
>         for (tree_stmt_iterator si = tsi_start (units); !tsi_end_p (si); 
> tsi_next (&si))
>           {
>             tree unit = tsi_stmt (si);
> -           if (A68_TYPE_HAS_ROWS_P (TREE_TYPE (unit)))
>               unit = a68_low_dup (unit);

Please fix the indentation of the statement that used to be within the
conditional.  After the removal it must be aligned like the rest of the
statements in the block.

>             CONSTRUCTOR_APPEND_ELT (ve, size_int (num_units), unit);
>             num_units += 1;
> diff --git a/gcc/algol68/a68-low-units.cc b/gcc/algol68/a68-low-units.cc
> index f7bfe6f822f..054f7fb3d74 100644
> --- a/gcc/algol68/a68-low-units.cc
> +++ b/gcc/algol68/a68-low-units.cc
> @@ -1133,7 +1133,6 @@ collect_call_arguments (NODE_T *p, vec<tree, va_gc> 
> *args, LOW_CTX_T ctx)
>         /* In Algol 68 parameters are passed via an identity declaration, so
>            this must implement same semantics.  */
>         tree arg = a68_lower_tree (p, ctx);
> -       if (HAS_ROWS (MOID (p)))
>           arg = a68_low_dup (arg);

Likewise.

>         arg = a68_consolidate_ref (MOID (p), arg);
>         args->quick_push (arg);

Please fix the above problems and submit a V2 of the patch.  Make sure
that in the subject you specify it is the second iteration by preceding
it by [PATCH V2].

Thank you!

Reply via email to