Skip to content

FreeVariables leaks a set builder's %1 placeholder, and only a lambda counts as a binder #989

Description

@Rafael-SOWNet

Measured on master at 4ee698da.

1. A set builder leaks its internal placeholder

"{ k : k > 0 }".ToEntity().FreeVariables      // { %1 }

%1 is the fresh name ConditionalSet.InitDirectChildren invents so that the set's variable is not confused with a variable of the same name outside it. It is not in the expression the caller wrote, it cannot be typed, and Variable.CreateTemp will invent a different one for a different predicate — so this is neither the bound name nor a stable answer. FreeVariables recurses through DirectChildren and picks it up.

Whatever the definition of "free" is, this is not it.

2. Only a lambda is treated as a binder

"lambda(k, k)".ToEntity().FreeVariables       // { }        correct
"sum(k, k, 1, 3)".ToEntity().FreeVariables    // { k }
"integral(k, k)".ToEntity().FreeVariables     // { k }
"limit(k, k, 0)".ToEntity().FreeVariables     // { k }
"derivative(k, k)".ToEntity().FreeVariables   // { k }

The XML doc says so plainly — "We call a bound variable a variable which is a parameter of some outer lambda. Then, all other variables are free." — so this is a documented choice rather than an oversight, and I am not filing it as a bug so much as asking whether the choice still holds.

It reads oddly now for two reasons. sum(k, k, 1, 3) is 6: nothing about the answer depends on k, and a caller asking FreeVariables to find out what an expression depends on is told it depends on k. And #986 has just made "these are binders" explicit in the type system — CalculusOperator and ConditionalSet both bind Var, and that is now where the bound name is decided. Six binders exist and one of them is honoured here.

Vars has the same shape ("sum(k, k, 1, 3)".ToEntity().Vars is { k }), but Vars promises less: it is documented as the variables that occur, minus the named constants, and an occurrence is what it counts.

What I think

(1) is a defect on any definition and worth fixing on its own — the placeholder should not escape.

(2) is a design call. Extending "bound" to every binder makes FreeVariables mean what the name means, and it is a breaking change for anyone who reads it as "occurring variables" — which is what VarsAndConsts is for, so the migration exists. I would rather ask than assume; the property is public and old.

Found while restoring three probe cases (vars, freevars, varsconsts) and writing a one-line comment saying what each does. The comment was wrong, which is how the difference turned up.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions