Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/be effects: Use DFA for fz -effects and avoid panic in mutate, fix #2293 #2407

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

fridis
Copy link
Member

@fridis fridis commented Jan 2, 2024

Before this patch, the effects reported for HelloWorld.fz were useless:

   > ./build/bin/fz -effects build/tests/hello/HelloWorld.fz
  ((i32.infix ..).as_stream.#anonymous9.map_to_stream String).#anonymous1.as_array.m
  ((list String).as_stream.map_to_stream String).#anonymous1.as_array.m
  ((list i32).as_stream.map_to_stream String).#anonymous1.as_array.m
  ((list u8).as_stream.map_to_stream String).#anonymous1.as_array.m
  (i32.infix ..).as_stream.#anonymous9.as_array.m
  (list String).as_stream.as_array.m
  (list i32).as_stream.as_array.m
  (list u8).as_stream.as_array.m
  exit
  io.err
  io.out
  mutate
  panic

This patch fixes this by switching the effects backend used by fz -effects to use DFA instead for CFG, which give higher analysis accuracy and avoids the many ...as_array.m effects.

Then, effect.abortable was added as a feature that can be redefined to indicate that a given effect must not support abort. This helps removing the reported panic effect and consequently also io.err and exit.

Finally, the use of mutate effect was removed from exit (which is an effect already any may as well use set and list.as_array (which used mut in a local mutate instance, which made the local mutate instance useless). Also, the use of panic in mutate.check_and_replace was replaced by a precondition.

A minor cleanup in the code using a local mutate instance in Stream.as_array is also part of this patch.

fz -effects now supports -verbose=1, which will print the reason and a sample call chain why the reported effects are there, this facilitates understanding the results significantly. To reduce noise, verbose output of DFA was moved one verbosity level up.

The resulting output of fz -effects HelloWorld.fz now is

   > ./build/bin/fz -effects build/tests/hello/HelloWorld.fz
  io.out

While -verbose adds a nice explanation (plus some unrelated warnings that need to be fixed).

   > ./build/bin/fz -effects -verbose build/tests/hello/HelloWorld.fz
  []
  EFFECT type io.out default used is io.out
  program entry point
      |
      +- performs call HelloWorld => HelloWorld
        |
        +- performs call say a0=boxed(Const_String):Const_String => UNIT
  /home/fridi/fuzion/work/build/tests/hello/HelloWorld.fz:27:3:
    say "Hello World!"
  --^^^
          |
          +- performs call io.out => io.out embedded in io.out
  $FUZION/lib/say.fz:31:25:
  public say(s Any) => io.out.println s
  ------------------------^
            |
            +- performs call ((io.#type io).out.#type io.out).install_default => UNIT
  $FUZION/lib/io/out.fz:42:12:
    out.type.install_default
  -----------^
              |
              +- performs call io.out.default target=io.out => UNIT
  $FUZION/lib/io/out.fz:32:36:
      (io.out default_print_handler).default
  -----------------------------------^

  Elapsed time for phases: prep 66ms, fe 104ms, createMIR 1ms, me 91ms, ir 603ms, effectsCheck 104ms, be 256ms
  6 warnings.

 #2293

Before this patch, the effects reported for HelloWorld.fz were useless:

   > ./build/bin/fz -effects build/tests/hello/HelloWorld.fz
  ((i32.infix ..).as_stream.#anonymous9.map_to_stream String).#anonymous1.as_array.m
  ((list String).as_stream.map_to_stream String).#anonymous1.as_array.m
  ((list i32).as_stream.map_to_stream String).#anonymous1.as_array.m
  ((list u8).as_stream.map_to_stream String).#anonymous1.as_array.m
  (i32.infix ..).as_stream.#anonymous9.as_array.m
  (list String).as_stream.as_array.m
  (list i32).as_stream.as_array.m
  (list u8).as_stream.as_array.m
  exit
  io.err
  io.out
  mutate
  panic

This patch fixes this by switching the effects backend used by `fz -effects` to
use DFA instead for CFG, which give higher analysis accuracy and avoids the many
`...as_array.m` effects.

Then, `effect.abortable` was added as a feature that can be redefined to
indicate that a given effect must not support `abort`.  This helps removing the
reported `panic` effect and consequently also `io.err` and `exit`.

Finally, the use of `mutate` effect was removed from `exit` (which is an effect
already any may as well use `set` and `list.as_array` (which used `mut` in a
local mutate instance, which made the local mutate instance useless). Also, the
use of `panic` in `mutate.check_and_replace` was replaced by a precondition.

A minor cleanup in the code using a local mutate instance in `Stream.as_array`
is also part of this patch.

`fz -effects` now supports `-verbose=1`, which will print the reason and a
sample call chain why the reported effects are there, this facilitates
understanding the results significantly.  To reduce noise, verbose output of DFA
was moved one verbosity level up.

The resulting output of `fz -effects HelloWorld.fz` now is

   > ./build/bin/fz -effects build/tests/hello/HelloWorld.fz
  io.out

While `-verbose` adds a nice explanation (plus some unrelated warnings that need
to be fixed).

   > ./build/bin/fz -effects -verbose build/tests/hello/HelloWorld.fz
  []
  EFFECT type io.out default used is io.out
  program entry point
      |
      +- performs call HelloWorld => HelloWorld
        |
        +- performs call say a0=boxed(Const_String):Const_String => UNIT
  /home/fridi/fuzion/work/build/tests/hello/HelloWorld.fz:27:3:
    say "Hello World!"
  --^^^
          |
          +- performs call io.out => io.out embedded in io.out
  $FUZION/lib/say.fz:31:25:
  public say(s Any) => io.out.println s
  ------------------------^
            |
            +- performs call ((io.#type io).out.#type io.out).install_default => UNIT
  $FUZION/lib/io/out.fz:42:12:
    out.type.install_default
  -----------^
              |
              +- performs call io.out.default target=io.out => UNIT
  $FUZION/lib/io/out.fz:32:36:
      (io.out default_print_handler).default
  -----------------------------------^

  Elapsed time for phases: prep 66ms, fe 104ms, createMIR 1ms, me 91ms, ir 603ms, effectsCheck 104ms, be 256ms
  6 warnings.
@fridis fridis requested a review from maxteufel January 2, 2024 11:45
The previous patch die not install a non-anbortable effect on a call to `go`
resulting in check failures when the effect was used.  Also, a local mutate was
used directly and not via `.env`.
There are two problems:

First, tests/local_mutate checks that mutating a locally mutable variable
outside of the scope of the local mutate effect by catching the resulting panic,
so the previous change to use a precondition instead of panic breaks this test.
Using panic, however, adds the panic effect to small examples like HelloWorld,
which is not desirable. So this patch implements a workaround that enables the
use for fuzion.std.panic in the base lib.

Second, intrinsics may currently not have preconditions, so I had to split up
`effect.abort` into one feature with precondition that calls a second `abort0`
that is intrinsic.
lib/Stream.fz Show resolved Hide resolved
lib/exit.fz Show resolved Hide resolved
@fridis fridis requested a review from maxteufel January 3, 2024 23:11
@maxteufel maxteufel merged commit 181c813 into main Jan 4, 2024
5 checks passed
@maxteufel maxteufel deleted the effects_use_dfa_and_avoid_panic_effect branch January 4, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants