Re: [PATCH 01/36] coccinelle: misc: add cond_return_no_effect.cocci

[Sang-Heon Jeon]

On Fri, Jul 24, 2026 at 3:45 AM Sang-Heon Jeon <ekffu200098@xxxxxxxxx> wrote:
>
> Add a new Coccinelle script which removes a conditional return that
> has no effect:
>
>         if (ret)
>                 return ret;
>         return ret;
>
> Both branches return the same value, so the check can be removed.
> The condition can also be a negation or a comparison with a
> constant. Such code is usually a leftover from removing a
> statement between the two returns.
>
> When a local variable is assigned right before the check, the
> assignment and the two returns turn into a single return of the
> assigned expression, and the declaration is dropped if nothing
> else uses the variable. Otherwise only the check is removed.
>
> The fold can delete comments between the check and the final
> return, so the generated patch should be reviewed.
>
> Signed-off-by: Sang-Heon Jeon <ekffu200098@xxxxxxxxx>
> ---
>  .../misc/cond_return_no_effect.cocci          | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/cond_return_no_effect.cocci
>
> diff --git a/scripts/coccinelle/misc/cond_return_no_effect.cocci b/scripts/coccinelle/misc/cond_return_no_effect.cocci
> new file mode 100644
> index 000000000000..541aafdc32c6
> --- /dev/null
> +++ b/scripts/coccinelle/misc/cond_return_no_effect.cocci
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Remove a conditional return that has no effect:
> +///
> +///    if (ret)
> +///            return ret;
> +///    return ret;
> +///
> +/// Both branches return the same variable, so the check has no
> +/// effect. It can also be a negation or a comparison with a
> +/// constant.
> +///
> +/// When a local variable is assigned right before the check, the
> +/// assignment and the two returns turn into a single return of the
> +/// assigned expression, and the declaration is dropped if nothing
> +/// else uses the variable. Otherwise only the check is removed.
> +///
> +// Such code is usually a leftover from removing a statement between
> +// the two returns.
> +//
> +// Confidence: High
> +// Copyright: (C) 2026 Sang-Heon Jeon
> +// Comments: The fold can delete comments between the check and the
> +//           final return, so review the generated patch.
> +// Options: --no-includes --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +//----------------------------------------------------------
> +//  For patch mode
> +//----------------------------------------------------------
> +
> +@collect depends on patch@
> +identifier ret;
> +expression E;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +       ret = E;
> +       if (\(ret \| !ret \| ret cmp C\))
> +               return ret;
> +       return ret;
> +
> +@depends on patch@
> +local idexpression ret;
> +expression E;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +-      ret = E;
> +-      if (\(ret \| !ret \| ret cmp C\))
> +-              return ret;
> +-      return ret;
> ++      return E;
> +

If we can make these rules depend on an earlier rule, it should
improve the performance a bit. I will look into whether it is
worthwhile.

> +@depends on patch@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +-      if (\(ret \| !ret \| ret cmp C\))
> +-              return ret;
> +       return ret;
> +
> +@depends on patch@
> +type T;
> +identifier collect.ret;
> +@@
> +-      T ret;
> +       ... when != ret
> +           when strict
> +
> +@depends on patch@
> +type T;
> +identifier collect.ret;
> +constant C;
> +@@
> +-      T ret = C;
> +       ... when != ret
> +           when strict
> +

While testing the script, I found that this rule can unexpectedly
remove the declaration of a global or static variable in the same
file, even when it is still used elsewhere. There is no such case in
this series, but I will fix it in v2 by matching the declaration
inside a function.

fn(...) {
      ...
-     T ret;
      ... when != ret
          when strict
}

> +
> +//----------------------------------------------------------
> +//  For context mode
> +//----------------------------------------------------------
> +
> +@depends on context@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +*      if (\(ret \| !ret \| ret cmp C\))
> +*              return ret;
> +       return ret;
> +
> +//----------------------------------------------------------
> +//  For org and report mode
> +//----------------------------------------------------------
> +
> +@r depends on org || report@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +position p;
> +@@
> +       if@p (\(ret \| !ret \| ret cmp C\))
> +               return ret;
> +       return ret;
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +cocci.print_main("WARNING: conditional return with no effect (both branches return the same value)", p)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: conditional return with no effect (both branches return the same value)")
> --
> 2.43.0
>