File 4201-cover-Ensure-that-LC-filters-are-compiled-... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 4201-cover-Ensure-that-LC-filters-are-compiled-in-guard-c.patch of Package erlang (Revision 49aa26b943fcbed41dda0b0a8caee5fb)

Currently displaying revision 49aa26b943fcbed41dda0b0a8caee5fb , Show latest

From 2adb932784784d00ff68f1b8aa27ccc53dfddeb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 25 Feb 2021 05:51:53 +0100
Subject: [PATCH] cover: Ensure that LC filters are compiled in guard context

A subtle detail in the semantics of list comprehensions is that
filters will be compiled as guard expressions if possible. That means
that the following list comprehension will never raise an exception as
long as it is passed a proper list:

lc(L) ->
        [V || V <- L, V =:= a orelse element(1, V) =:= a].

Thus, the following call:

lc([a, {a,b}, {}, "ignore"])

will return `[a, {a,b}]`. When the calls `element(1, {}` and
`element(1, "ignore")` fail, no exception is raised and the
corresponding list elements are discarded.

The `cover` tools rewrites rewrites `andalso` and `orelse` to `case`
constructions in order to produce better coverage data. For this example,
that rewriting would change the error behavior of the list comprehension:

lc(L) ->
        [V || V <- L,
              case V =:= a of
                   true -> true;
                   false -> element(1, V) =:= a
              end].

Because of the `case`, the filter is no longer a guard, and the
`element(1, {}` call will now raise a `badarg` exception.

To avoid that problem, stop rewriting `andalso`/`orelse` in filters
that are guard tests.
---
 lib/tools/src/cover.erl        | 32 ++++++++++++++++++++++++++++++++
 lib/tools/test/cover_SUITE.erl | 25 ++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lib/tools/src/cover.erl b/lib/tools/src/cover.erl
index dd2366d7fb..9964d8ce19 100644
--- a/lib/tools/src/cover.erl
+++ b/lib/tools/src/cover.erl
@@ -1851,8 +1851,17 @@ expand(Expr) ->
     Expr1.
 
 expand({clause,Line,Pattern,Guards,Body}, Vs, N) ->
+    %% We must not expand andalso/orelse in guards.
     {ExpandedBody,N2} = expand(Body, Vs, N),
     {{clause,Line,Pattern,Guards,ExpandedBody},N2};
+expand({lc,Line,Expr,Qs}, Vs, N) ->
+    {ExpandedExpr,N2} = expand(Expr, Vs, N),
+    {ExpandedQs,N3} = expand_qualifiers(Qs, Vs, N2),
+    {{lc,Line,ExpandedExpr,ExpandedQs},N3};
+expand({bc,Line,Expr,Qs}, Vs, N) ->
+    {ExpandedExpr,N2} = expand(Expr, Vs, N),
+    {ExpandedQs,N3} = expand_qualifiers(Qs, Vs, N2),
+    {{bc,Line,ExpandedExpr,ExpandedQs},N3};
 expand({op,_Line,'andalso',ExprL,ExprR}, Vs, N) ->
     {ExpandedExprL,N2} = expand(ExprL, Vs, N),
     {ExpandedExprR,N3} = expand(ExprR, Vs, N2),
@@ -1881,6 +1890,29 @@ expand([E|Es], Vs, N) ->
 expand(T, _Vs, N) ->
     {T,N}.
 
+expand_qualifiers([Q|Qs], Vs, N) ->
+    {Q2,N2} = case erl_lint:is_guard_test(Q) of
+                  true ->
+                      %% This qualifier is a guard test and will be
+                      %% compiled as such. Don't expand andalso/orelse
+                      %% because that would turn it into a body
+                      %% expression that may raise an exception. Here
+                      %% is an example of a filter where the error
+                      %% behaviour would change:
+                      %%
+                      %%      V == a orelse element(1, V) == a
+                      %%
+                      {Q,N};
+                  false ->
+                      %% A generator or a filter that is not a guard
+                      %% test.
+                      expand(Q, Vs, N)
+              end,
+    {Qs2,N3} = expand_qualifiers(Qs, Vs, N2),
+    {[Q2|Qs2],N3};
+expand_qualifiers([], _Vs, N) ->
+    {[],N}.
+
 vars(A, {var,_,V}) when V =/= '_' ->
     [V|A];
 vars(A, T) when is_tuple(T) ->
diff --git a/lib/tools/test/cover_SUITE.erl b/lib/tools/test/cover_SUITE.erl
index 771f527e43..b9cd028546 100644
--- a/lib/tools/test/cover_SUITE.erl
+++ b/lib/tools/test/cover_SUITE.erl
@@ -32,7 +32,7 @@ all() ->
                    otp_8340,otp_8188,compile_beam_opts,eep37,
                    analyse_no_beam, line_0, compile_beam_no_file,
                    compile_beam_missing_backend,
-                   otp_13277, otp_13289, gh_4796],
+                   otp_13277, otp_13289, gh_4796, guard_in_lc],
     StartStop = [start, compile, analyse, misc, stop,
                  distribution, reconnect, die_and_reconnect,
                  dont_reconnect_after_stop, stop_node_after_disconnect,
@@ -1784,6 +1784,29 @@ otp_13289(Config) ->
     ok = file:delete(File),
     ok.
 
+guard_in_lc(Config) ->
+    Test = <<"-module(t).
+              -export([lc/1]).
+
+              lc(L) ->
+                  [V || V <- L, V == a orelse element(1, V) == a].
+             ">>,
+
+    %% The filter in the list comprehension must be compiled as a
+    %% guard expression. Therefore, `cover` must NOT rewrite the list
+    %% comprehension in the test code like this:
+    %%
+    %% [V || V <- L,
+    %%       case V == a of
+    %%           true -> true;
+    %%           false -> element(1, V) == a
+    %%       end].
+
+    File = cc_mod(t, Test, Config),
+    [a,{a,good}] = t:lc([a, b, {x,y}, {a,good}, "ignore"]),
+    ok = file:delete(File),
+    ok.
+
 local_only(Config) ->
     ok = file:set_cwd(proplists:get_value(data_dir, Config)),
 
-- 
2.26.2

Places

File 4201-cover-Ensure-that-LC-filters-are-compiled-in-guard-c.patch of Package erlang (Revision 49aa26b943fcbed41dda0b0a8caee5fb)

Places