Proposal: Support out/pattern variables in LINQ with query-scoping

[HaloFour]

See dotnet/roslyn#15619

TL;DR:

Extend LINQ query syntax to support the use of out declarations and variable patterns in queries so that the introduced identifiers are promoted to range variables that may be referenced in subsequent query clauses. The scope would be identical to how range variables introduced with the let clause today.

Examples:

IEnumerable<string> strings = ...;

var numbers1 = from value in strings
    where int.TryParse(value, out var number)
    select number;

IEnumerable<object> objects = ...;

var numbers2 = from value in objects
    where value is int number || (value is string s && int.TryParse(s, out number))
    select number;

Suggested translations, using anonymous types:

let:

from s in strings
let res = int.TryParse(s, out var i)
where res
select i;

// translated into:
strings.Select(s => new { res = int.TryParse(s, out var i), i = i))
    .Where($projected => $projected.res)
    .Select($projected => $projected.i);

where:

from s in strings
where int.TryParse(s, out var i)
select i;

// translated into:
strings.Select(s => new { $condition = int.TryParse(s, out var i), i = i))
    .Where($projected => $projected.$condition)
    .Select($projected => $projected.i);

from:

from department in departments
from employee in department.GetEmployees(out var manager)
select new { employee, manager };

// translated into:
departments.Select(department => new {
        department = department,
        $collection = department.GetEmployees(out var manager),
        manager = manager
    })
    .SelectMany($projected => $projected.$collection,
        ($projected, $collection) => new {
             employee = $projected.employee,
             manager = $projected.manager
        }
    );

[Thaina]

For functionality like this I would write extension method

static class Ext
{
    public static int? TryParseInt(this string text)
    {
        return int.TryParse(text,out var value) ? value : null;
    }

    public static IEnumerable<V> SelectNotNull<T,V>(this IEnumerable<T> items,Func<T,V?> selector) where V : struct
    {
         foreach(var item in items)
         {
             var value = selector(item);
             if(value != null)
                 yield return value.Value;
         }
    }
}

IEnumerable<string> strings = ...;
var numbers1 = strings.SelectNotNull((text) => text.TryParseInt());

I think this API should be in corefx. There are also talking about dictionary.GetValueOrNull(key) in there

[orthoxerox]

@HaloFour could you perhaps copy the suggested translations into your proposal?

[jnm2]

This syntax is brilliant. I've had many opportunities this year where I was sad that this syntax wasn't already available.

The case I ran into today is one of those awkward things that is hard to follow with method syntax but straightforward using let- if only I was able to use out var in the same way as let.

[jnm2]

😢 /~https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-01-03.md#scoping-of-expression-variables-in-query-clauses

If they go ahead with this, doesn't that make it a breaking change to ever make them available as range variables?

[gafter]

@jnm2 Yes. But we hope to make it possible for you to write

from s in e
let (b, i) = int.TryParse(s, out int parsed) ? (true, parsed) : default
where b
select i

or something better

[HaloFour]

@jnm2

The last sentence seems to imply that they might consider it in the context of the let clause.

@gafter

I'd love to see what you might have in mind for "something better". While that use of tuples in the let clause does scratch the itch it's not exactly the nicest looking code in the world.

🍝 Maybe? It's marginally better.

from s in e
let (b = int.TryParse(s, out int parsed), i = parsed)
where b
select i

Or the following?

from s in e
let (b, i) = (int.TryParse(s, out int parsed), parsed)
where b
select i

[gafter]

from s in e
let (b, i) = (int.TryParse(s, out int parsed), parsed)
where b
select i

[jnm2]

from s in e
let (b, i) = (int.TryParse(s, out var parsed), parsed)
where b
select i

Versus:

from s in e
where int.TryParse(s, out var parsed)
select parsed

From the notes it seems the only issue is motivation to do the work. Would you consider accepting work from the community? @HaloFour's projections seem intuitive.

[gafter]

I would want to see a specification before even considering taking this kind of work from the community. It would have to happen very quickly, as in C# 7.3 you can use pattern variables in query clauses, so it would be a breaking change.

[gafter]

Note that the current specification is syntactic, so this would require a major overhaul to the specification.

[HaloFour]

@gafter

What's the timeframe on 7.3 being locked down and would the team consider delaying that one feature if the community could manage to pull together "enough"* of the specification changes to demonstrate that its on the "correct"* path?

* As determined by the design team

[gafter]

I don't think it is realistic. The LDM spent a couple of meetings on this direction to decide which way to go and landed with what is in 7.3. 7.3 is locked down approximately yesterday.

The specification problem is that you're going to end up capturing variables even when they're not needed in subsequent clauses, which means there will be a substantial performance penalty for anyone who uses an expression variable but doesn't need it to be part of the query state. Fixing that is hard and probably not worth the complexity.

[HaloFour]

@gafter

I don't think it is realistic. The LDM spent a couple of meetings on this direction to decide which way to go and landed with what is in 7.3. 7.3 is locked down approximately yesterday.

This is a failure of the LDM. How can there be any community involvement if the decisions are discussed and rendered long before they are posted? And what's the rush? Are you seeing a lot of feedback complaining about the inability to include out and pattern variables in query clauses? If so, how many of those people are actually expecting those variables to be promoted to range variables?

The specification problem is that you're going to end up capturing variables even when they're not needed in subsequent clauses, which means there will be a substantial performance penalty for anyone who uses an expression variable but doesn't need it to be part of the query state. Fixing that is hard and probably not worth the complexity.

That sounds like an implementation detail, not a concern for the specification. Why couldn't the specification simply state that the range variables are captured and available and allow for the compiler to optimize that away? The specification isn't that specific anyway and the compiler would be free to make other optimizations, such as replacing the anonymous types with tuples. What makes this case different?

Just double-checked the date on those meeting notes. You have no idea how many four-letter words I want to sling right now.

[jnm2]

While I'm not upset, this is very disappointing.

[HaloFour]

@jnm2

I'm more upset about the fact that the LDM has yet again gone through a design, discussion and decision with zero involvement of the community. We were quite literally given a time window of -1 days to have any input on what makes up 7.3 due to the fact that none of the design notes were posted. It's incredibly demoralizing and makes me question why I or anyone else would want to continue to participate here.

As for this feature specifically, I could live with deconstruction in the let clause. It's far from as nice as it could be, and it'd be entirely impossible to reverse that decision in the future, but apparently its critical to lift the compiler restriction on these variables in query clauses in the next minor release.

[svick]

@alrz I don't understand, which part of my proposal depends on the implementation of Where?

[gafter]

@svick You wrote

When a transparent identifier is checked to be not equal with null, the associated anonymous type object is checked to be not equal with null.

What associated anonymous type object?

[alrz]

@svick

So, if I understand correctly, from x in a where x is int i select i becomes

a.Select(x => x is int i ? new{x, i} : null).Where(t => t != null).Select(t => t.i);

There is no guarantee that Where ensures t != null is true. Ok that shouldn't really happen. But we may need all variables to be definitely assigned there, because you don't usually get those errors until you actually use them (which is what we will end up doing in the final translated code, but not in the source)

from item in list
where item is int i && (item is int j || item is int k)
select i;

PS: we may be able to do that if we only return those that are definitely assigned: ... ? new {item, i} but still those should be able to be assigned which with that translation, wouldn't be possible.

[gafter]

We generally specify scoping based on syntax, and then specify definite assignment based on the scoping and semantics.

This proposal flips things. It specifies scoping based on definite assignment. Since we do scoping very early and definite assignment later, this would be nontrivial to implement in the compiler. Moreover, if some variable happens to be defined in a previous query clause but not definitely assigned, you might be able to use the name in a later clause without an error - but you'd be using a field from the enclosing class. Definite assignment was designed to avoid this kind of problem.

In this sense, your proposed specification doesn't even make sense in the flow of the specification. It proposes a syntactic translation, but then throws in definite assignment as a decision criterion. We don't know how definite assignment works until we know what variables are in which scope, so we know what the names refer to, but we won't know any of that until we decide which syntactic translation we're using.

[svick]

@gafter

Thank you for the review.

You wrote

When a transparent identifier is checked to be not equal with null, the associated anonymous type object is checked to be not equal with null.

What associated anonymous type object?

I wanted to explain that .Where(* => * != null) should be translated into .Where(y => y != null). The above is my attempt to use the existing framework of transparent identifiers for that, but it's true that it's not well specified. I have changed the translation, so it doesn't use transparent identifier for this purpose any more.

In this sense, your proposed specification doesn't even make sense in the flow of the specification. It proposes a syntactic translation, but then throws in definite assignment as a decision criterion. We don't know how definite assignment works until we know what variables are in which scope, so we know what the names refer to, but we won't know any of that until we decide which syntactic translation we're using.

I have written a version of the spec that changes the translation based on whether the type of the expression in where is bool or not. But if I understand what you said above correctly, that wouldn't work either.

So I have updated the specification to always use the same translation for where with promoted expression variables. It means it won't work well for types other than bool, but I don't know how to write it in a way that supports both variables that are "definitely assigned after true" and types other than bool, if I can't use semantic information to differentiate the two cases.

@HaloFour

So, if I understand correctly, from x in a where x is int i select i becomes

a.Select(x => x is int i ? new{x, i} : null).Where(t => t != null).Select(t => t.i);

Correct.

There is no guarantee that Where ensures t != null is true.

Yes, but in that case, there is no possible value to give to i (except maybe default). This means such code will almost certainly fail at some point and I don't think it matters much at which point it will fail.

But we may need all variables to be definitely assigned there, because you don't usually get those errors until you actually use them (which is what we will end up doing in the final translated code, but not in the source)

from item in list
where item is int i && (item is int j || item is int k)
select i;

Those other variables are not referenced in following clauses. So they are not "promoted expression variables", which means you won't get any errors in that query.

So the above query would be translated to:

list
.Select(item => item is int i && (item is int j || item is int k) ? new { item, i } : null)
.Where(t => t != null)
.Select(t => t.i)

[alrz]

@svick See my PS. Unused variables should at least exist because they can still be assigned.

[alrz]

@gafter @svick

FWIW, I suggest we use a translation similar to what we do for async.

Considering:

from item in list
where item is int i && (true || item is int j)
select i;

We generate a struct of all expression variables, range variables and the result of the whole expression.

struct Env 
{
    public object item;
    public int i;
    public int j;
    public bool result;
}

then we do the regular lowering of patterns, except that we use those fields for pattern temps

list.Select(item => {
    Env env = new Env();
    env.item = item;
    bool result;
    if (item is int)
    {
        env.i = (int)item;
        result = true;
    }
    else
    {
        result = false;
    }
    env.result = result;
    return env;
}).Where(env => env.result)

EDIT: To make i definitely assigned in the following clauses, we should adjust flow analysis for where to make variables "definitely assigned when true"

[alrz]

On the second thought, I think we need eventually depend on Where e.g. we say that where is like if regarding definite assignment. I updated the comment with a better example.

Giving some context, check out this example to see what I mean by "similar to async".

[svick]

@alrz I don't think being able to assign to a variable declared in a previous clause is worth doing all of that.

[alrz]

@svick Thing is, that's how the compiler already works. Your translation is something totally new and doesn't necessarily do less. In fact, I think it's more complex because you're lowering variables based on their definite assignment state, we don't have anything like that in the compiler.

[alrz]

The equivalent of my example under your proposal would look like this:

list.Select(item => {
    int i;
    int j;
    if (item is int)
    {
        i = (int)item ;
        return new {item, i, /* j - omitted */};
    }
    else
    {
        return null;
    }
}).Where(env => env != null)

Some details worth mentioning:

• emitting field for j is now conditional based on definite assignment state
• you depend on the fact that anon type is a reference type. Imagine if we want to use tuples for that later (Use (value) tuples for query translation roslyn#8192), that would be another thing to be conditional.
• doesn't generate less code at all, there are additional locals and also the inevitable anon type.

So instead of doing that, we just make a struct for all variables from the start and directly use its fields for pattern temps. Seems to be a lot more straightforward.

[svick]

@alrz

Thing is, that's how the compiler already works.

It's not how it works for LINQ queries.

you're lowering variables based on their definite assignment state

The latest version of my spec does not depend on definite assignment at all. And no version of my spec did anything with variables that were not used in a following clause.

emitting field for j is now conditional based on definite assignment state

Like I said, it's not and never was, because j is not used in any following clause.

you depend on the fact that anon type is a reference type. Imagine if we want to use tuples for that later (dotnet/roslyn#8192), that would be another thing to be conditional.

I don't see how my spec limits the implementation from using nullable value tuples, or any other reasonable implementation.

doesn't generate less code at all, there are additional locals and also the inevitable anon type.

My goal wasn't to generate less code, it was to write a spec that is simple and also consistent with the existing spec for LINQ queries. If the implementation can achieve the same effect in a way that is more efficient, it is free to do so.

So instead of doing that, we just make a struct for all variables from the start and directly use its fields for pattern temps. Seems to be a lot more straightforward.

I still don't understand how this makes the spec simpler. But feel free to write your own version of the spec for this feature.

Though one thing that confused me was that your lowering example includes lowering of pattern matching and the && operator (and also removing unused code?), so I assumed your version of the specification would have to describe all that.

But it seems you don't want to do that, which still leaves me unclear about how your spec would look like.

[gafter]

The latest version of my spec does not depend on definite assignment at all. And no version of my spec did anything with variables that were not used in a following clause.

We won't know whether a variable is used in a following clause until we know where the variable is in scope. And we won't know where it is in scope until we know the translation selected for the query clauses. And we don't know which of your translations to use until we know whether a variable is used in a following clause. This is circular reasoning and you need to break it somewhere. We elected to break it at step one and say that such variables simply are not used in following clauses. You seem to want to break the circular reasoning somewhere else, but I don't know where.

[HaloFour]

I'd rather take the hit and always have the variable in scope as range variables. That eliminates the ambiguity. If there is concern about unused range variables causing performance issues (as if people who are concerned about performance issues use LINQ), the compiler (or an analyzer) can detect the fact that they are unused and warn, offering to convert to discards.

[alrz]

because j is not used in any following clause.

no version of my spec did anything with variables that were not used in a following clause.

@svick This is the "condition" I'm talking about. You can't define your translation based on variables being used or not.

Though one thing that confused me was that your lowering example includes lowering of pattern matching and the && operator (and also removing unused code?), so I assumed your version of the specification would have to describe all that.

My example in the prior comment doesn't represent the "translation", but the final lowered code.

The translation itself goes like this: we turn a "query operator on an expression with declaration expressions" to a Select prepended to that operator. But that's not the first thing we would do.

1. once we see a query operator with expression declarations, we turn all those locals (plus range variables and result) into struct fields and rewrite the expression.
2. then we wrap the result from (1) with a lambda, pass it to Select and prepend it to the actual operator.
3. finally we change the original query clause to use result from that struct.

Maybe I'm not explaining this well but this certainly is not what you call a specification.