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

Tuple conversion implementation appears confused #11013

Closed
gafter opened this issue May 2, 2016 · 4 comments · Fixed by #16659
Closed

Tuple conversion implementation appears confused #11013

gafter opened this issue May 2, 2016 · 4 comments · Fixed by #16659

Comments

@gafter
Copy link
Member

gafter commented May 2, 2016

The tuple conversion implementation appears either confused or confusing. A method

        private BoundExpression CreateTupleConversion(CSharpSyntaxNode syntax, BoundTupleLiteral sourceTuple, Conversion conversion, bool isCast, TypeSymbol destination, DiagnosticBag diagnostics)

results in a bound node that does not have the destination type (but instead a type that is a combination of the destination type with names from the source tuple). I don't think that's right.

@VSadov
Copy link
Member

VSadov commented May 5, 2016

That is for cases like

( (long a, long b) ) ( c: 1, d: 2 );
The literal is target-typed to (long c, long d), because target-typing of elements affects only types, not names. I think that was the idea when we discussed this. We can change that.

Then there is a conversion node that identity-converts to (long a, long b).

The result is (long a, long b), the conversion is not confused.
It just goes through two steps - target typing of the literal and then name-conversion.

Since semantic info exposes the top conversion, it will look as
IdentityConversion: (long c, long d) --> (long a, long b)

@AlekseyTs
Copy link
Contributor

If that code survives, we need to decide what to do with location for the tuple symbol.

@VSadov
Copy link
Member

VSadov commented May 5, 2016

I think location of a thing named "c" should be where we have "c" introduced in the source.
The symbol for (long c, long d) will have location "( c: 1, d: 2 );"

The natural type (int c, int d) would also have that location, but I do not think the type will be observable in APIs, so that should not be confusing.

@VSadov
Copy link
Member

VSadov commented May 5, 2016

WithElementNames should probably take locations in addition to names.

Original locations are almost certainly incorrect when names are substituted. - How can it be in the same place when names are different?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants