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.

2 files changed, 56 insertions, 1 deletions

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,Anno,Pattern,Guards,Body}, Vs, N) ->
+    %% We must not expand andalso/orelse in guards.
     {ExpandedBody,N2} = expand(Body, Vs, N),
     {{clause,Anno,Pattern,Guards,ExpandedBody},N2};
+expand({lc,Anno,Expr,Qs}, Vs, N) ->
+    {ExpandedExpr,N2} = expand(Expr, Vs, N),
+    {ExpandedQs,N3} = expand_qualifiers(Qs, Vs, N2),
+    {{lc,Anno,ExpandedExpr,ExpandedQs},N3};
+expand({bc,Anno,Expr,Qs}, Vs, N) ->
+    {ExpandedExpr,N2} = expand(Expr, Vs, N),
+    {ExpandedQs,N3} = expand_qualifiers(Qs, Vs, N2),
+    {{bc,Anno,ExpandedExpr,ExpandedQs},N3};
 expand({op,_Anno,'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],
+                   otp_13277, otp_13289, 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)),