Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend Out Variable support to LINQ #15619

Closed
scalablecory opened this issue Nov 30, 2016 · 20 comments
Closed

Extend Out Variable support to LINQ #15619

scalablecory opened this issue Nov 30, 2016 · 20 comments

Comments

@scalablecory
Copy link

scalablecory commented Nov 30, 2016

Consider this case for out variable use:

var strings = new[] { "1", "2" };

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

This won't work due to how "out int" is transformed -- it is scoped to the delegate passed to Where(). Out variables should be extended for more natural LINQ integration.

@scalablecory
Copy link
Author

I'll note that implementation of this is non-trivial. It would need to be broken down into a Select/Where pair if it were to be kept within the existing selection of LINQ methods.

@HaloFour
Copy link

Somewhat relevant:

#6400
#6877

Personally I think I'd prefer for out declarations to simply "leak" into the wider scope of the query as a range variable. To me that's the most intuitive and would "just work".

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

// translated into:

strings.Select(s => new { res = int.TryParse(s, out int i), i = i })
    .Where(anon => anon.res)
    .Select(anon => anon.i);

// or using tuples:
        
strings.Select(s => (res: int.TryParse(s, out int i), i: i))
    .Where(tuple => tuple.res)
    .Select(tuple => tuple.i);

Of course if C# 7.0 ships without that behavior then it would be too late to consider it and additional syntax would be required to expand the scope.

The same behavior could probably apply for pattern variables although it might be trickier to tease out definite assignment. It's a shame that the fate of let deconstruction statements is in limbo as I thought that keyword would fit in perfectly with LINQ, particularly if it supported continue or break clauses in the else clause for fallible patterns which would translate to Where and SkipWhile respectively.

@controlflow
Copy link

LINQ query syntax looks totally abandoned from the new language features perspective. Deconstruction is not available, out variables won't have "nice" scope in query expression (I guess because of complex transformations involved).

I think I'm actually happy with this (I hope query syntax will die eventually), but this tells something about C# design process and the "composability" of language features introduced in previous C# versions :\

@alrz
Copy link
Member

alrz commented Dec 1, 2016

Also,

var q = from item in list
        where item is int x
        select x;

I don't think this would be a breaking change if done later.

@scalablecory
Copy link
Author

@HaloFour indeed, perhaps just leaving it the normal "out" syntax is sufficient. I hadn't considered that a sufficiently optimizing compiler could simply "do the right thing" in the case that the out variable didn't break the scope of the delegate.

@HaloFour
Copy link

HaloFour commented Dec 1, 2016

@alrz

It would be a breaking change to expand the scope of x later as the query could introduce additional range variables with the name x:

var q = from item in list
    where item is int x
    let x = 123
    select x;

With narrow scoping this is legal. With wider scoping this is a compiler error.

That's why I think it's important to consider now. /cc @CyrusNajmabadi

@CyrusNajmabadi
Copy link
Member

i'm actually not sure what happens here. As linq is just supposed to be syntactic transformations using lambdas, i'm not sure what the interplay is with that and pattern/out variables. Based on the syntactic transformation, my intuition is that they're not in scope. But i need @gafter @MadsTorgersen to weigh in.

@alrz
Copy link
Member

alrz commented Dec 1, 2016

@CyrusNajmabadi I think the only reason I'd use linq over extension methods is that it manages these variables (for example, let transformation is something that extension methods don't have), therefore I'd expect it to work, otherwise, you should write it manually which wouldn't be that straightforward.

@HaloFour
Copy link

HaloFour commented Dec 1, 2016

@CyrusNajmabadi

Agreed, this increases the amount of transformation required. A projection including the variable patterns and out declarations must be performed first via Select. I think that the experience is worth it, though. It will "just work". Not considering this now removes it permanently from the table and you won't have another opportunity to explore the combination of LINQ and pattern matching without having to consider new syntax.

@HaloFour
Copy link

HaloFour commented Dec 1, 2016

Alternatively, if you need to punt this until post C# 7.0 you could make it illegal to include an out declaration or variable pattern in a LINQ query, just as it is with queries translated into expression trees. It would at least buy time to consider things.

@CyrusNajmabadi
Copy link
Member

Our general intuition on variables is that if an existing construct can already introduce variables, then sub-expression-introduced-variables in that construct should go into that same variable scope.

So, given that, my intuition tells me that if you use an out-var in a 'let clause' then it would go in the same scope as the let-variable.

Open questiosn though on things like where/select. But, intuitively, i can see why they'd behave like 'let'.

@HaloFour
Copy link

HaloFour commented Dec 1, 2016

@CyrusNajmabadi

And to note, I've seen this pattern used more than once:

int num;

var query = from s in strings
    where int.TryParse(s, out num)
    select num;

foreach (var i in query) { ... }

And I've seen that go pear-shaped when parallelism is introduced.

@scalablecory scalablecory changed the title Extend "let" for Out Variable support Extend Out Variable support to LINQ Dec 1, 2016
@HaloFour
Copy link

HaloFour commented Dec 2, 2016

@CyrusNajmabadi

Looking through the LINQ query clauses I'm not sure that it would need to apply to any of them beyond let, where and maybe from. For the rest I'm not sure it makes a lot of sense to introduce range variables. With a terminal select there wouldn't seem to be anywhere for that range variable to go since the result of the expression is what is projected. For select into maybe you could argue that a range variable could be introduced alongside the projection, but normally all of the other range variables are discarded at that point.

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
        }
    );

@gafter
Copy link
Member

gafter commented Dec 2, 2016

@scalablecory Do you have a proposed specification, or modification to the current specification? Or is this just a request that someone else figure it out?

@HaloFour
Copy link

HaloFour commented Dec 2, 2016

@gafter

What do you need beyond the query-to-method translations above to at least get the concept considered? I'd be willing to take a crack at specification amendments. I'm more interested in the reaction from the LDM at this point.

@gafter
Copy link
Member

gafter commented Dec 2, 2016

@HaloFour The current specification is completely local and compositional. The proposed translation isn't. I'll let you know what the LDM says.

@gafter
Copy link
Member

gafter commented Dec 2, 2016

@HaloFour Also, the suggested translation only handles expression variables in a let, where and from clause, but there are other kinds of clauses.

@HaloFour
Copy link

HaloFour commented Dec 2, 2016

@gafter

The current specification is completely local and compositional.

Of course, but previously C# had precious few ways to introduce new variables so let perfectly fit the bill.

Also, the suggested translation only handles expression variables in a where clause, but there are lots of other kinds of clauses.

Don't forget let and from (see this comment). I'm not sure that projecting range variables makes a lot of sense in the other clauses but I'm far from opposed to considering something that would apply to any and all LINQ query clauses. For example a blanket amendment to the specification could be that if a LINQ query clause is followed by an expression that contains variable patterns or out declarations that the translation is preceded by a call to Select which projects the result of that expression plus all of the introduced variables into a transparent identifier.

In my opinion supporting the wider scope in let would scratch the itch. Adding where to the mix would make me ecstatic and I think covers the majority of use cases where one might want to mix out declarations or pattern matching with LINQ. Beyond that is icing on the cake that might make some edge cases easier without resorting to let.

I leave it to the LDM to consider.

// C# 6.0 (seen in the wild, unsafe for parallelism)
int i;
from s in strings
where int.TryParse(s, out i)
select i;

// C# 7.0 (as of now, safe but awkward)
from s in strings
let projected = (condition = int.TryParse(s, out var i), result = i)
where projected.condition
select projected.i;

// C# 7.0? (just let, safe)
from s in strings
let condition = int.TryParse(s, out var i)
where condition
select i;

// C# 7.0? (let and where, safe and quite concise)
from s in strings
where int.TryParse(s, out var i)
select i;

@gafter
Copy link
Member

gafter commented Dec 14, 2016

See #15910. We're blocking expression variables from being used in query clauses, so that we can do this later without it being a breaking change. To work around the restriction, use a local function.

@gafter
Copy link
Member

gafter commented Jan 8, 2018

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https:/dotnet/csharplang/issues, and there is a draft spec at https:/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https:/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https:/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


This may be close enough to dotnet/csharplang#159 that discussion can continue there.

@gafter gafter closed this as completed Jan 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants