UPLC Inliner: effect safety

[Unisay]

Added test coverage before changing this functionality for #1492

[Unisay]

I have disabled logs preservation in Marlowe validators and witnessed Ex Budget improvement:

    +--------------------++-------------+--------------+--------+
    |                    ||   CPU units | Memory units |   Size |
    +====================++=============+==============+========+
    | Baseline (average) || 540 460 151 |    1 784 424 |  7 945 |
    |   Actual (average) || 533 611 472 |    1 742 839 |  7 831 |
    |              Delta ||      -1.27% |       -2.33% | -1.43% |
    +--------------------++-------------+--------------+--------+

[zliu41]

The title of the PR should mention UPLC, as you are only updating the UPLC inliner, not the PIR inliner.

[zliu41]

Second, this is a strange way of getting unique names. In most if not every other API (e.g., template haskell API, GHC API), getting uinque names is a monadic action. This way the names are truly unique. In your method, calling uniqueNames2 "a" "b" the second time returns the same names, which are no longer unique.

[Unisay]

This is by design: reducing a uniqueness scope allows to get rid of the monadic context.

calling uniqueNames2 "a" "b" the second time returns the same names

We never call freshName "x" more than once in tests anyway, so this is not a problem in practice.
OTOH the monadic contexts that requires to declare unique names before they are used is a (readability) problem that this approach solves.

[zliu41]

Also: the heavy use of infix functions like `app` looks visually unappealing.

[Unisay]

I guess its a matter of taste 🤷🏼‍♂️
I was thinking of having a variation of $ operator for infix applications, something like:

  term
    $. m
    $. xs
    $. i
    $. b

but in the end app is not so bad and doesn't require a creation/memoization of an extra operator.

[zliu41]

Have you verified that before your change, the inliner won't inline this? I think that should be the case but it's worth verifying to be certain.

[Unisay]

Yes, I did.

inlineImpure5:           FAIL
  Test output was different from 'untyped-plutus-core/test/Transform/inlineImpure5.uplc.golden'. 
  It was:
  \a -> (\b -> addInteger (addInteger 1 a) b) (addInteger 3 a)
  Use -p '/simplify/&&/inlineImpure5/' to rerun this test only.

[zliu41]

Also this should have a changelog entry. Please do not default to using the "no changelog required" tag. If you are in doubt about whether a changelog entry is needed, err on the side of adding one.

[zliu41]

👍

It occurred to me that the PIR equivalent of isVarEventuallyEvaluated will be useful for another optimization: if a non-strict var is eventually evaluated, then we can turn it into a strict var.