Prevent removal of inlined mutable vars from optimized AST

[ncave]

This prevents inlined mutable vars to be removed from optimized AST.

[TIHan]

Can you give more context on this? What is this solving?

[dsyme]

Can you give more context on this? What is this solving?

I do get why this is needed.

The compiler inserts various compiler-generated bindings during type checking, but we make an effort to remove these when revealing the TAST via quotations and FCS expression trees, to make the public form of the expression trees less sensitive to compiler changes and less revealing of compiler internals. We do this by simply substituting let compgen = e1 in e2 away replacing all compgen in e2 with copies of e1.

However, this may be incorrect when a compiler-generated intermediate value is mutable. I'd imagine the mutability is relevant in some situation related to Fable

@ncave two things

1. Can you measure the impact of making the corresponding change in quotations.fs please?
2. Can you add some tests please? I'd like to understand the situations where this matters, particularly w.r.t. quotations.

[ncave]

Can you give more context on this? What is this solving?

@TIHan
This prevents incorrect removal of mutable variables after inlining functions that use them (in optimized AST), e.g. List.average, etc.

[dsyme]

@TIHan This prevents incorrect removal of mutable variables after inlining functions that use them (in optimized AST), e.g. List.average, etc.

That is, in the context of FCS clients like Fable who look at optimized expression trees.

It sounds like this doesn't affect quotations as yet, since they don't reveal expression trees after inlining.

[ncave]

@dsyme That's right, as stated, in optimized AST only. And it's perfectly fine to keep it downstream (Fable-only), I just wanted to bring it up to see if anything can be done here.

Basically, is that optimization (extra variable removal) worth applying unconditionally, if there are (not so edge) cases where it impacts (breaks) simple code that just uses FSharp.Core functions (e.g. List.average etc.). Again, just in the optimized AST, not the generated IL, but still.

[dsyme]

@dsyme That's right, as stated, in optimized AST only. And it's perfectly fine to keep it downstream (Fable-only), I just wanted to bring it up to see if anything can be done here.

Your fix is good. Can you add a test case please?

thanks

[TIHan]

Thank you both. It makes sense to me.

[ncave]

@dsyme Added some tests.