Skip to content

Quotations matching "" produce incorrect representation #19873

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...)

Activity

  1. added theissue type on Jun 1, 2026
  2. added this to the Backlog milestone on Jun 1, 2026
  3. T-Gro commented on Jun 1, 2026

    @T-Gro
    Member

    Hi @pezipink , I understand the angle you are coming from.
    The "" matching was done as part of PatternMatchCompilation, together with rest of the pattern matching logic.

    I will try to assess the consequences of moving this to the Optimizer - keeping both better codegen for consumer of .NET IL, as well as reverting the change to users of quoted expressions.

  4. added
    Area-QuotationsQuotations (compiler support or library). See also "queries"
    and removed on Jun 1, 2026
  5. pezipink commented on Jun 2, 2026

    @pezipink
    ContributorAuthor

    Thanks, is there a reason this was only applied to match?

    <@ fun s -> if s = "" then 1 else 0 @>

     Lambda (s,
           IfThenElse (Call (None, op_Equality, [s, Value ("")]), Value (1),
                       Value (0)))
    

    I'm pretty sure the non quoted version of this used to produce the same IL as the equivalent match? I imagine you'd want the new check here as well, which moving it into the optimizer should also give you.

    Seems a bit strange to have two different representations of it.

  6. T-Gro commented on Jun 2, 2026

    @T-Gro
    Member

    How compiler decides to turn pattern matching syntax into exact instructions (if chains, jump tables, range checks for consecutive integers) is a compiler implementation detail.

    However, your issue shows that quotations are a leaky abstraction in that case - already receiving the representation after pattern matching is turned into lower-level language features. Even before the referred change, quotations received an IfThenElse, no longer representing the match clause.

  7. pezipink commented on Jun 2, 2026

    @pezipink
    ContributorAuthor

    Hi @T-Gro . Thanks for your reply, but I'm not sure I understand the argument you are making. I don't think it is a 'leaky abstraction'.

    Since F# is not LISP, supporting a (limited) quotation system required a suitable runtime abstraction to be selected. By design it excludes many syntactic language elements and presents a simplified view of others. This is why I said

    to provide a slightly abstracted but direct representation of the code

    A tradeoff selected was for IfThenElse to represent all quoted conditional logic, and not to have a richer set of higher level forms like 'match' or even 'and'. In this context, translating match to IfThenElse is not surfacing a compiler implementation detail, but instead a direct result of translating the code to the chosen abstraction level. It just so happens that the compiler also uses a similar scheme internally.

    Essentially, quoted match -> IfThenElse is re-arranging and duplicating the syntax to result in a logically equivalent representation of the code at a different abstraction level. It does not attempt to bring additional semantic meaning to the code at this stage, it only guarantees the logic will hold - which is why it will happily produce very large trees where it repeats the same redundant logic over and over again. Quote some active patterns to see this in action!

    In my view this is fundamentally different to rewriting s = "" to s <> null && s.Length = 0. The new version is now a different piece of code no longer representative of the original. It can only be argued to be equivalent when considering the properties of a string in the target system.

    Jump tables and range checks you mentioned are excellent examples of compiler details that should not (and do not) appear in quotations.

    Philosophical and historical musings aside however, it is still a breaking change.

  8. T-Gro commented on Jun 12, 2026

    @T-Gro
    Member

    @pezipink :

    When fixing the behavior, I added baselines for basic quoted representations to guard any future changes.
    Which got me thinking - do you have any OSS repository with heavy reliance on quoted representation?

    If yes, we could add it to our matrix of OSS projects that go trough regression testing with every PR to the compiler.
    Quoted representations are now underrepresented, and this would be a valuable addition 👍 /

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-QuotationsQuotations (compiler support or library). See also "queries"BugRegression

    Type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions