Skip to content

Quotations matching "" produce incorrect representation #19873

@pezipink

Description

@pezipink

Related to this and this

I'm raising this as a bug as I believe it warrants further discussion and to bring clarity for future changes.

Modifying how code quotes are represented should be considered a breaking change in the language and treated accordingly.

Quotation compilers rely on the quoted representation of code as their foundation. If the representation can change then it has the potential to cause big problems at the very core of the compilers.

If such a change is unavoidable, it must be clearly communicated ahead of time and in the release notes. This change was all but silent as far as I can tell, and broke a production compiler of mine, which could have led to loss.

I strongly advocate that this change is reworked under the following premise;

The function of code quotations is to provide a slightly abstracted but direct representation of the code, for the primary purpose of translation into some other form, or to be compiled to some other language.

Runtime implementation specific details and optimisations should never be surfaced in the quoted code, since it defeats the purpose of the quote.

If I quote <@ 5 + 5 @> I do not expect or want the compiler to constant-fold this and give me the 10 literal back. Maybe I'm using a modular / clock arithmetic. <@ if true then true else false @> should not reduce to true. I might want to explicitly check for it. The quote must preserve the code as written - it is the function of the translator and target system / context to decide what the quote means.

In the case of <@ function "" -> true | _ -> false @> it now produces


Lambda (_arg1,
      IfThenElse (IfThenElse (Call (None, op_Inequality,
                                    [_arg1, Value (<null>)]),
                              Call (None, op_Equality,
                                    [PropertyGet (Some (_arg1), Length, []),
                                     Value (0)]), Value (false)), Value (true),
                  Value (false)))

The resulting quote is not representative of the original code at all. There is no equality to String.Empty or "". It has introduced a complex AND block that contains two checks that are not part of the quoted code in any way.

The null literal is especially a problem - unless you quote <@@ null @@> somewhere explicitly, there is no place I know of in the quote system that will produce this Value(<null>) node. My compiler affected by this change had never seen a null literal since it relies heavily on option types.

The length check is also problematic - you could argue that this new code is semantically equivalent, but that's only true within the context of the CLR runtime. It's quite possible my target doesn't understand the notion of a string's length, or, indeed, null itself. The rewriting of this quotation is conflating runtime specific details and optimisations with the representation of the actual code.

I haven't looked at the implementation details, but it seems to me this rewriting should happen after the quotes are generated, as other optimisations do.

Happy to debate, but I hope we can at least agree that changes like this in the future must be communicated loud and clear (apologies if it was, I found no mention of it anywhere...)

Metadata

Metadata

Assignees

No one assigned

    Type

    No fields configured for Bug.

    Projects

    Status

    New

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions