Skip to content

Conversation

@cston
Copy link
Contributor

@cston cston commented Jun 27, 2023

Address PROTOTYPE comments.

@cston cston requested review from a team as code owners June 27, 2023 00:21
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 27, 2023
@cston cston changed the base branch from main to features/CollectionLiterals June 27, 2023 00:22
@cston cston removed request for a team June 27, 2023 00:22
}

var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(syntax, isNewInstance: true, targetType) { WasCompilerGenerated = true };
// PROTOTYPE: When generating a List<T>, should we use the well-known
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Added open issue to test plan #66418.

var result = element switch
{
BoundBadExpression => element,
// PROTOTYPE: Should spread elements support target type?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Added open issue to test plan #66418.

}
}
""";
// PROTOTYPE: Should compile and run successfully: expectedOutput: "[1, 2, 3], "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Natural type for collection literals is not supported, so [x] and [y] are target-typed only.

CompileAndVerify(new[] { source, s_collectionExtensions }, expectedOutput: "(System.Int32[]) [1, 2, 3], ");
}

// PROTOTYPE: Test other variance cases.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

See TypeInference_38, _39.

}
}
""";
// PROTOTYPE: Confirm we should generate ListBase<int> instances for both x and y.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Natural type is not supported.

[ConditionalTheory(typeof(CoreClrOnly))]
[CombinatorialData]
public void SpreadElement_01(
[CombinatorialValues("IEnumerable", "IEnumerable<int>", "int[]", "List<int>", "Span<int>", "ReadOnlySpan<int>")] string spreadType,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IEnumerable

Tested separate in SpreadElement_11.

}
""";
var comp = CreateCompilation(source);
// PROTOTYPE: Should spread elements support target type? (Should we infer IEnumerable<int>?)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Added open issue to test plan #66418.

}
}
""";
// PROTOTYPE: Should spread elements support target type? (Should we infer IEnumerable<string>?)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTOTYPE

Added open issue to test plan #66418.

}

[ConditionalFact(typeof(CoreClrOnly), AlwaysSkip = "PROTOTYPE: 'IAsyncEnumerable<int>' does not contain a definition for 'GetAwaiter'")]
public void Async_03()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Async_03

Added open issue to test plan #66418: support IAsyncEnumerable<T> for spread element?

@cston
Copy link
Contributor Author

cston commented Jun 27, 2023

    // PROTOTYPE: Should this compile successfully?

Added open issue to test plan [#66418](#66418).


Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/CollectionLiteralTests.cs:2036 in a207dec. [](commit_id = a207dec, deletion_comment = True)

@cston cston requested review from 333fred and RikkiGibson June 28, 2023 10:12
{
// PROTOTYPE: This is not a great diagnostic. Users could easily run into this and be confused. Can we do
// better. For example:
// This is not a great diagnostic. Users could easily run into this and be confused. Can we do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular case involves a dictionary element which is not currently supported. I've logged an issue to improve the diagnostic for the previous example (A)[1]: #68862.

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment, otherwise LGTM

return GetValEscape(switchExpr.SwitchArms.SelectAsArray(a => a.Value), scopeOfTheContainingExpression);

case BoundKind.CollectionLiteralExpression:
// PROTOTYPE: Revisit if spans may be optimized to avoid heap allocation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test plan #66418 item tracking this now.

@cston cston merged commit 77d3192 into dotnet:features/CollectionLiterals Jun 29, 2023
@cston cston deleted the collections-comments branch June 29, 2023 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants