From bb29ba3bbb397f51cd50a5f5cae58331eb1e48e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 17 Jun 2024 08:11:40 +0200
Subject: [PATCH] Refactor type annotation handling for clarity

In #8564, Frej Drejhammar noticed that the following comment
was misleading:

    %% One or more arguments have been simplified to literal values.

It turns out that the entire clause in which the comment is located is
not needed. Removing the `arg_types` annotation is unnecessary, since
it will be added back in the next iteration of the `beam_ssa_type`
pass.

This commit removes the unnecessary clause, as well as strengthening
and explaining the invariants maintained by `opt_anno_types/2`.
---
 lib/compiler/src/beam_ssa_type.erl | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/compiler/src/beam_ssa_type.erl b/lib/compiler/src/beam_ssa_type.erl
index 790d4ed364b3..fdc5d06dd9b9 100644
--- a/lib/compiler/src/beam_ssa_type.erl
+++ b/lib/compiler/src/beam_ssa_type.erl
@@ -589,10 +589,30 @@ opt_is([I0 | Is], Ts0, Ds0, Ls, Fdb, Sub0, Meta, Acc) ->
 opt_is([], Ts, Ds, _Ls, Fdb, Sub, _Meta, Acc) ->
     {reverse(Acc), Ts, Ds, Fdb, Sub}.
 
-opt_anno_types(#b_set{op=Op,args=Args}=I, Ts) ->
+%% opt_anno_types(Instruction0, Types) -> Instruction.
+%%  Maintain the invariant that the `arg_types` annotation only
+%%  contains type annotations for variable arguments whose types are
+%%  more specific than `any`. Literal arguments must not have any type
+%%  annotation.
+%%
+%%  Also, ensure that the `arg_types` annotation is only present
+%%  in instructions that can benefit from it.
+opt_anno_types(#b_set{anno=Anno0,op=Op,args=Args}=I, Ts) ->
     case benefits_from_type_anno(Op, Args) of
-        true -> opt_anno_types_1(I, Args, Ts, 0, #{});
-        false -> I
+        true ->
+            opt_anno_types_1(I, Args, Ts, 0, #{});
+        false ->
+            case Anno0 of
+                #{arg_types := _} ->
+                    %% Remove `arg_types` from operations that don't
+                    %% need them. This can happen, for example, when
+                    %% the operation has been changed from
+                    %% `{bif,is_list}` to `is_nonempty_list`.
+                    Anno = maps:remove(arg_types, Anno0),
+                    I#b_set{anno=Anno};
+                _  ->
+                    I
+            end
     end;
 opt_anno_types(#b_switch{anno=Anno0,arg=Arg}=I, Ts) ->
     case concrete_type(Arg, Ts) of
@@ -621,10 +641,6 @@ opt_anno_types_1(#b_set{anno=Anno0}=I, [], _Ts, _Index, Acc) ->
     case Anno0 of
         #{ arg_types := Acc } ->
             I;
-        #{ arg_types := _ } when Acc =:= #{} ->
-            %% One or more arguments have been simplified to literal values.
-            Anno = maps:remove(arg_types, Anno0),
-            I#b_set{anno=Anno};
         #{} ->
             Anno = Anno0#{ arg_types => Acc },
             I#b_set{anno=Anno}