Re: [Cocci] [PATCH v2 RESEND] coccinelle: misc: add minmax script

[Julia Lawall]

On Fri, 19 Feb 2021, Denis Efremov wrote:

> Check for opencoded min(), max() implementations.
>
> Signed-off-by: Denis Efremov 
> ---
>
> Changes in v2:
>  - <... ...> instead of ... when any
>  - org mode reports fixed
>  - patch rule to drop excessive ()
>
>  scripts/coccinelle/misc/minmax.cocci | 224 +++
>  1 file changed, 224 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/minmax.cocci
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci 
> b/scripts/coccinelle/misc/minmax.cocci
> new file mode 100644
> index ..61d6b61fd82c
> --- /dev/null
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded min(), max() implementations.
> +/// Generated patches sometimes require adding a cast to fix compile warning.
> +/// Warnings/patches scope intentionally limited to a function body.
> +///
> +// Confidence: Medium
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: min, max
> +//
> +
> +
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
> +
> +@rmax depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*x cmp@p y ? x : y

The rule below indicated with FIXME is supposed to deal with the
possibility of () that are unnecessary when using min and max.  It doesn't
work, because <... P ...> allow P to match 0 or more times, and thus
func@p matches every function.

A simpler solution is to just allow arbitrary () in the pattern, eg:

  (x) cmp@p (y) ? (x) : (y)

That will allow each occurrence of x and y to occur with and without
parentheses.  In the submitted  semantic patch, the () issue was only
considered in the patch case.  But it actually affects the purely matching
cases too, because () can be used at one occurrence, but not the other.

> +@script:python depends on report@
> +p << rmax.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for max()")

p is an array because it can be bound to different positions on different
control-flow paths.  Notably this occurs with <... ...>.  If there are
multiple occurrences of the pattern, there will be one match that contains
all of them.  Thus the reporting code should be:

for p0 in p:
  coccilib.report.print_report(p0, "WARNING opportunity for max()")

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] [PATCH v2 RESEND] coccinelle: misc: add minmax script

[Denis Efremov]

Check for opencoded min(), max() implementations.

Signed-off-by: Denis Efremov 
---

Changes in v2:
 - <... ...> instead of ... when any
 - org mode reports fixed
 - patch rule to drop excessive ()

 scripts/coccinelle/misc/minmax.cocci | 224 +++
 1 file changed, 224 insertions(+)
 create mode 100644 scripts/coccinelle/misc/minmax.cocci

diff --git a/scripts/coccinelle/misc/minmax.cocci 
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..61d6b61fd82c
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+   <...
+*  x cmp@p y ? x : y
+   ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+   <...
+*  if (x cmp@p y) {
+*  max_val = x;
+*  } else {
+*  max_val = y;
+*  }
+   ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+   <...
+*  x cmp@p y ? x : y
+   ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+   <...
+*  if (x cmp@p y) {
+*  min_val = x;
+*  } else {
+*  min_val = y;
+*  }
+   ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+   <...
+-  x cmp y ? x : y
++  max(x, y)
+   ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+   <...
+-  if (x cmp y) {
+-  max_val = x;
+-  } else {
+-  max_val = y;
+-  }
++  max_val = max(x, y);
+   ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+   <...
+-  x cmp y ? x : y
++  min(x, y)
+   ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+   <...
+-  if (x cmp y) {
+-  min_val = x;
+-  } else {
+-  min_val = y;
+-  }
++  min_val = min(x, y);
+   ...>
+}
+
+@depends on (pmax || pmaxif || pmin || pminif)@
+identifier func;
+expression x, y;
+position p;
+// FIXME: Coccinelle consumes all available ram and
+// and timeouts on every file.
+// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
+@@
+
+func@p(...)
+{
+   <...
+(
+-  (min((x), (y)))
++  min(x, y)
+|
+-  (max((x), (y)))
++  max(x, y)
+)
+   ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
-- 
2.26.2

___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci