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.

Sign up for GitHub

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

Jump to bottom

Collection literals: type inference #68703

Merged
merged 11 commits into from
Jun 23, 2023
Merged

Collection literals: type inference #68703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d3f0043
Collection literals: type inference
cston Jun 20, 2023
183999a
Address feedback
cston Jun 22, 2023
c4e3e1a
More tests
cston Jun 22, 2023
cad8374
Merge remote-tracking branch 'upstream/features/CollectionLiterals' i…
cston Jun 22, 2023
dd075f8
Merge remote-tracking branch 'upstream/features/CollectionLiterals' i…
cston Jun 22, 2023
dc92b4f
Update tests
cston Jun 23, 2023
2b012fa
Recurse through elements for parameter and output type inferences
cston Jun 22, 2023
f8673ba
Address feedback
cston Jun 23, 2023
5f1e143
Add parens around type name
cston Jun 23, 2023
72c7946
Merge tests
cston Jun 23, 2023
69f6b5a
Add existing scenario to test
cston Jun 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ protected BoundExpression ConvertForEachCollection(
return convertedCollectionExpression;
}

protected bool GetEnumeratorInfoAndInferCollectionElementType(
internal bool GetEnumeratorInfoAndInferCollectionElementType(
SyntaxNode syntax,
ExpressionSyntax collectionSyntax,
ref ForEachEnumeratorInfo.Builder builder,
Expand Down
85 changes: 78 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

Expand Down Expand Up @@ -558,7 +559,7 @@ private MethodTypeInferenceResult InferTypeArgs(Binder binder, ref CompoundUseSi
// SPEC: phase. The first phase makes some initial inferences of bounds, whereas
// SPEC: the second phase fixes type parameters to specific types and infers further
// SPEC: bounds. The second phase may have to be repeated a number of times.
InferTypeArgsFirstPhase(ref useSiteInfo);
InferTypeArgsFirstPhase(binder, ref useSiteInfo);
bool success = InferTypeArgsSecondPhase(binder, ref useSiteInfo);
var inferredTypeArguments = GetResults(out bool inferredFromFunctionType);
return new MethodTypeInferenceResult(success, inferredTypeArguments, inferredFromFunctionType);
Expand All @@ -569,7 +570,7 @@ private MethodTypeInferenceResult InferTypeArgs(Binder binder, ref CompoundUseSi
// The first phase
//

private void InferTypeArgsFirstPhase(ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private void InferTypeArgsFirstPhase(Binder binder, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert(!_formalParameterTypes.IsDefault);
Debug.Assert(!_arguments.IsDefault);
Expand All @@ -585,11 +586,12 @@ private void InferTypeArgsFirstPhase(ref CompoundUseSiteInfo<AssemblySymbol> use
TypeWithAnnotations target = _formalParameterTypes[arg];
ExactOrBoundsKind kind = GetRefKind(arg).IsManagedReference() || target.Type.IsPointerType() ? ExactOrBoundsKind.Exact : ExactOrBoundsKind.LowerBound;

MakeExplicitParameterTypeInferences(argument, target, kind, ref useSiteInfo);
MakeExplicitParameterTypeInferences(binder, argument, target, kind, ref useSiteInfo);
}
}

private void MakeExplicitParameterTypeInferences(BoundExpression argument, TypeWithAnnotations target, ExactOrBoundsKind kind, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
#nullable enable
private void MakeExplicitParameterTypeInferences(Binder binder, BoundExpression argument, TypeWithAnnotations target, ExactOrBoundsKind kind, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
// SPEC: * If Ei is an anonymous function, and Ti is a delegate type or expression tree type,
// SPEC: an explicit type parameter inference is made from Ei to Ti and
Expand All @@ -609,8 +611,12 @@ private void MakeExplicitParameterTypeInferences(BoundExpression argument, TypeW
ExplicitParameterTypeInference(argument, target, ref useSiteInfo);
ExplicitReturnTypeInference(argument, target, ref useSiteInfo);
}
else if (argument.Kind == BoundKind.UnconvertedCollectionLiteralExpression)
Copy link
Member

Choose a reason for hiding this comment

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

Once we have natural typing, this (and the corresponding elements in output type inference) are going to need to change, as we are not making any type inferences from the entire collection type to T at the moment. Future work though.

{
MakeCollectionLiteralTypeInferences(binder, (BoundUnconvertedCollectionLiteralExpression)argument, target, kind, ref useSiteInfo);
}
else if (argument.Kind != BoundKind.TupleLiteral ||
!MakeExplicitParameterTypeInferences((BoundTupleLiteral)argument, target, kind, ref useSiteInfo))
!MakeExplicitParameterTypeInferences(binder, (BoundTupleLiteral)argument, target, kind, ref useSiteInfo))
{
// Either the argument is not a tuple literal, or we were unable to do the inference from its elements, let's try to infer from argument type
var argumentType = _extensions.GetTypeWithAnnotations(argument);
Expand All @@ -626,7 +632,50 @@ private void MakeExplicitParameterTypeInferences(BoundExpression argument, TypeW
}
}

private bool MakeExplicitParameterTypeInferences(BoundTupleLiteral argument, TypeWithAnnotations target, ExactOrBoundsKind kind, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private void MakeCollectionLiteralTypeInferences(
Binder binder,
BoundUnconvertedCollectionLiteralExpression argument,
TypeWithAnnotations target,
ExactOrBoundsKind kind,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (target.Type is null)
{
return;
}

if (argument.Elements.Length == 0)
{
return;
}

if (!TryGetCollectionIterationType(binder, (ExpressionSyntax)argument.Syntax, target.Type, out TypeWithAnnotations targetElementType))
{
return;
}

foreach (var element in argument.Elements)
{
MakeExplicitParameterTypeInferences(binder, element, targetElementType, kind, ref useSiteInfo);
}
}

private static bool TryGetCollectionIterationType(Binder binder, ExpressionSyntax syntax, TypeSymbol collectionType, out TypeWithAnnotations iterationType)
{
var builder = new ForEachEnumeratorInfo.Builder();
BoundExpression collectionExpr = new BoundValuePlaceholder(syntax, collectionType);
return binder.GetEnumeratorInfoAndInferCollectionElementType(
syntax,
syntax,
ref builder,
ref collectionExpr,
isAsync: false,
BindingDiagnosticBag.Discarded,
out iterationType);
}
#nullable disable

private bool MakeExplicitParameterTypeInferences(Binder binder, BoundTupleLiteral argument, TypeWithAnnotations target, ExactOrBoundsKind kind, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
// try match up element-wise to the destination tuple (or underlying type)
// Example:
Expand Down Expand Up @@ -659,7 +708,7 @@ private bool MakeExplicitParameterTypeInferences(BoundTupleLiteral argument, Typ
{
var sourceArgument = sourceArguments[i];
var destType = destTypes[i];
MakeExplicitParameterTypeInferences(sourceArgument, destType, kind, ref useSiteInfo);
MakeExplicitParameterTypeInferences(binder, sourceArgument, destType, kind, ref useSiteInfo);
}

return true;
Expand Down Expand Up @@ -788,6 +837,10 @@ private void MakeOutputTypeInferences(Binder binder, BoundExpression argument, T
{
MakeOutputTypeInferences(binder, (BoundTupleLiteral)argument, formalType, ref useSiteInfo);
}
else if (argument.Kind == BoundKind.UnconvertedCollectionLiteralExpression)
{
MakeOutputTypeInferences(binder, (BoundUnconvertedCollectionLiteralExpression)argument, formalType, ref useSiteInfo);
}
else
{
if (HasUnfixedParamInOutputType(argument, formalType.Type) && !HasUnfixedParamInInputType(argument, formalType.Type))
Expand All @@ -801,6 +854,24 @@ private void MakeOutputTypeInferences(Binder binder, BoundExpression argument, T
}
}

private void MakeOutputTypeInferences(Binder binder, BoundUnconvertedCollectionLiteralExpression argument, TypeWithAnnotations formalType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (argument.Elements.Length == 0)
{
return;
}

if (!TryGetCollectionIterationType(binder, (ExpressionSyntax)argument.Syntax, formalType.Type, out TypeWithAnnotations targetElementType))
{
return;
}

foreach (var element in argument.Elements)
{
MakeOutputTypeInferences(binder, element, targetElementType, ref useSiteInfo);
}
}

private void MakeOutputTypeInferences(Binder binder, BoundTupleLiteral argument, TypeWithAnnotations formalType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (formalType.Type.Kind != SymbolKind.NamedType)
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7228,9 +7228,11 @@ private static NullableAnnotation GetNullableAnnotation(BoundExpression expr)
case BoundKind.UnboundLambda:
case BoundKind.UnconvertedObjectCreationExpression:
case BoundKind.ConvertedTupleLiteral:
case BoundKind.UnconvertedCollectionLiteralExpression:
Copy link
Member

Choose a reason for hiding this comment

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

Note that this will crash IOperation

Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't clear to me why this is OK for UnconvertedObjectCreationExpression, but not for UnconvertedCollectionLiteralExpression.

Copy link
Member

Choose a reason for hiding this comment

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

It's not. That codepath is either never hit, or only hit during initial nullable analysis of something like an unbound lambda (and if so, we really should have left a comment). If CSharpOperationFactory sees an UnconvertedObjectCreationExpression, it will crash.

return NullableAnnotation.NotAnnotated;
default:
Debug.Assert(false); // unexpected value
// PROTOTYPE: Re-enable
//Debug.Assert(false); // unexpected value
return NullableAnnotation.Oblivious;
}
}
Expand Down
Loading