Dictionary expressions: support concrete target types that do not use [CollectionBuilder]

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -5346,7 +5346,13 @@
             MessageID.IDS_FeatureDictionaryExpressions.CheckFeatureAvailability(diagnostics, syntax, syntax.ColonToken.GetLocation());
             var key = BindValue(syntax.KeyExpression, diagnostics, BindValueKind.RValue);
             var value = BindValue(syntax.ValueExpression, diagnostics, BindValueKind.RValue);
-            return new BoundKeyValuePairElement(syntax, key, value);
+            return new BoundKeyValuePairElement(
+                syntax,
+                key,
+                value,
+                keyPlaceholder: null,
+                valuePlaceholder: null,
+                indexerAssignment: null);
         }
 #nullable disable
 
@@ -6582,11 +6588,99 @@
         }
 
 #nullable enable
-        private BoundCollectionExpressionSpreadElement BindCollectionExpressionSpreadElementAddMethod(
+        private BoundExpression BindDictionaryItemAssignment(
+            SyntaxNode syntax,
+            BoundObjectOrCollectionValuePlaceholder implicitReceiver,
+            MethodSymbol setMethod,
+            BoundExpression expr,
+            Conversion keyConversion,
+            TypeSymbol destinationKeyType,
+            Conversion valueConversion,
+            TypeSymbol destinationValueType,
+            BindingDiagnosticBag diagnostics)
+        {
+            Debug.Assert(expr.Type is { });
+            Debug.Assert(ConversionsBase.IsKeyValuePairType(Compilation, expr.Type, WellKnownType.System_Collections_Generic_KeyValuePair_KV, out _, out _));
+
+            var exprType = (NamedTypeSymbol)expr.Type;
+            return BindDictionaryItemAssignment(
+                syntax,
+                implicitReceiver,
+                setMethod,
+                CreateConversion(
+                    bindKeyOrValue(
+                        syntax,
+                        expr,
+                        ((MethodSymbol)GetWellKnownTypeMember(WellKnownMember.System_Collections_Generic_KeyValuePair_KV__get_Key, diagnostics, syntax: syntax)).AsMember(exprType)),
+                    keyConversion,
+                    destinationKeyType,
+                    diagnostics),
+                CreateConversion(
+                    bindKeyOrValue(
+                        syntax,
+                        expr,
+                        ((MethodSymbol)GetWellKnownTypeMember(WellKnownMember.System_Collections_Generic_KeyValuePair_KV__get_Value, diagnostics, syntax: syntax)).AsMember(exprType)),
+                    valueConversion,
+                    destinationValueType,
+                    diagnostics),
+                diagnostics);
+
+            static BoundCall bindKeyOrValue(SyntaxNode syntax, BoundExpression expr, MethodSymbol getMethod)
+            {
+                return new BoundCall(
+                    syntax,
+                    receiverOpt: expr,
+                    // PROTOTYPE: What is the correct value for receiver cloning? Test with expression element
+                    // and spread element where the item is ref KVP<,> or ref readonly KVP<,>.
+                    initialBindingReceiverIsSubjectToCloning: ThreeState.False,
+                    method: getMethod,
+                    arguments: [],
+                    argumentNamesOpt: default,
+                    argumentRefKindsOpt: default,
+                    isDelegateCall: false,
+                    expanded: false,
+                    invokedAsExtensionMethod: false,
+                    argsToParamsOpt: default,
+                    defaultArguments: default,
+                    resultKind: LookupResultKind.Viable,
+                    type: getMethod.ReturnType).MakeCompilerGenerated();
+            }
+        }
+
+        // PROTOTYPE: Test when there is a more applicable indexer. We should stick with this one.
+        // PROTOTYPE: Test when there is no conversion from value to indexer type, since value is not included in AnalyzedArguments.
+        private BoundExpression BindDictionaryItemAssignment(
+            SyntaxNode syntax,
+            BoundObjectOrCollectionValuePlaceholder implicitReceiver,
+            MethodSymbol setMethod,
+            BoundExpression key,
+            BoundExpression value,
+            BindingDiagnosticBag diagnostics)
+        {
+            // PROTOTYPE: What about argument conversions?
+            return new BoundCall(
+                syntax,
+                receiverOpt: implicitReceiver,
+                initialBindingReceiverIsSubjectToCloning: ThreeState.False,
+                method: setMethod,
+                arguments: [key, value],
+                argumentNamesOpt: default,
+                argumentRefKindsOpt: default,
+                isDelegateCall: false,
+                expanded: false,
+                invokedAsExtensionMethod: false,
+                argsToParamsOpt: default,
+                defaultArguments: default,
+                resultKind: LookupResultKind.Viable,
+                type: setMethod.ReturnType).MakeCompilerGenerated();
+        }
+
+        private BoundCollectionExpressionSpreadElement BindCollectionExpressionSpreadElement<TArg>(
             SpreadElementSyntax syntax,
             BoundCollectionExpressionSpreadElement element,
-            Binder collectionInitializerAddMethodBinder,
             BoundObjectOrCollectionValuePlaceholder implicitReceiver,
+            Func<Binder, ExpressionSyntax, BoundValuePlaceholder, BoundObjectOrCollectionValuePlaceholder, TArg, BindingDiagnosticBag, BoundExpression> bindItem,
+            TArg arg,
             BindingDiagnosticBag diagnostics)
         {
             var enumeratorInfo = element.EnumeratorInfoOpt;
@@ -6603,22 +6697,24 @@
             }
 
             Debug.Assert(enumeratorInfo.ElementType is { }); // ElementType is set always, even for IEnumerable.
-            var addElementPlaceholder = new BoundValuePlaceholder(syntax, enumeratorInfo.ElementType);
-            var addMethodInvocation = BindCollectionInitializerElementAddMethod(
+            // PROTOTYPE: See ConvertCollectionExpression().bindSpread() which uses syntax.Expression rather
+            // than syntax for the item placeholder and also uses .WithSuppression(syntax.Expression.IsSuppressed).
+            var itemPlaceholder = new BoundValuePlaceholder(syntax, enumeratorInfo.ElementType);
+            var item = bindItem(
+                this,
                 syntax.Expression,
-                ImmutableArray.Create((BoundExpression)addElementPlaceholder),
-                hasEnumerableInitializerType: true,
-                collectionInitializerAddMethodBinder,
-                diagnostics,
-                implicitReceiver);
+                itemPlaceholder,
+                implicitReceiver,
+                arg,
+                diagnostics);
             return element.Update(
                 element.Expression,
                 expressionPlaceholder: element.ExpressionPlaceholder,
                 conversion: element.Conversion,
                 enumeratorInfo,
                 lengthOrCount: element.LengthOrCount,
-                elementPlaceholder: addElementPlaceholder,
-                iteratorBody: new BoundExpressionStatement(syntax, addMethodInvocation) { WasCompilerGenerated = true });
+                elementPlaceholder: itemPlaceholder,
+                iteratorBody: new BoundExpressionStatement(syntax, item) { WasCompilerGenerated = true });
         }
 #nullable disable
 

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/CollectionExpressionTypeKind.cs

@@ -12,6 +12,7 @@ internal enum CollectionExpressionTypeKind
         ReadOnlySpan,
         CollectionBuilder,
         ImplementsIEnumerable,
+        ImplementsIEnumerableWithIndexer,
         ArrayInterface,
         DictionaryInterface,
     }

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs

@@ -188,68 +188,154 @@ protected override Conversion GetCollectionExpressionConversion(
 
             if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable)
             {
+                // PROTOTYPE: Test type with indexer but no constructor callable with no arguments. Should be a conversion error rather than subsequent binding error.
                 if (!_binder.HasCollectionExpressionApplicableConstructor(syntax, targetType, out constructor, out isExpanded, BindingDiagnosticBag.Discarded))
                 {
                     return Conversion.NoConversion;
                 }
 
-                if (elements.Length > 0 &&
+                // PROTOTYPE: Test collection type that does not include an indexer, and where element type is some KeyValuePair<,>.
+                if (object.ReferenceEquals(elementType.OriginalDefinition, Compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_KeyValuePair_KV)) &&
+                    Compilation.IsFeatureEnabled(MessageID.IDS_FeatureDictionaryExpressions) &&
+                    _binder.GetCollectionExpressionApplicableIndexer(syntax, targetType, BindingDiagnosticBag.Discarded) is { })
+                {
+                    collectionTypeKind = CollectionExpressionTypeKind.ImplementsIEnumerableWithIndexer;
+                }
+                else if (elements.Length > 0 &&
                     !_binder.HasCollectionExpressionApplicableAddMethod(syntax, targetType, addMethods: out _, BindingDiagnosticBag.Discarded))
                 {
                     return Conversion.NoConversion;
                 }
             }
 
-            bool usesKeyValuePairs = CollectionUsesKeyValuePairs(Compilation, collectionTypeKind, elementType, out var keyType, out var valueType);
-            var builder = ArrayBuilder<Conversion>.GetInstance(elements.Length);
-            foreach (var element in elements)
+            ImmutableArray<Conversion> elementConversions;
+            if (CollectionUsesKeyValuePairs(Compilation, collectionTypeKind, elementType, out var keyType, out var valueType))
             {
-                switch (element)
+                var builder = ArrayBuilder<Conversion>.GetInstance(elements.Length * 2);
+                foreach (var element in elements)
                 {
-                    case BoundCollectionExpressionWithElement:
-                        // Collection arguments do not affect convertibility.
-                        continue;
-                    case BoundCollectionExpressionSpreadElement spreadElement:
-                        {
-                            var elementConversion = GetCollectionExpressionSpreadElementConversion(spreadElement, elementType, ref useSiteInfo);
-                            if (elementConversion.Exists)
+                    switch (element)
+                    {
+                        case BoundCollectionExpressionWithElement:
+                            // Collection arguments do not affect convertibility.
+                            continue;
+                        case BoundCollectionExpressionSpreadElement spreadElement:
                             {
-                                builder.Add(elementConversion);
-                                continue;
+                                var enumeratorInfo = spreadElement.EnumeratorInfoOpt;
+                                if (enumeratorInfo is { })
+                                {
+                                    // PROTOTYPE: Test case where elementType is null.
+                                    // PROTOTYPE: Test when element is not KeyValuePair<,>, or when there is no conversion between key or value types.
+                                    // PROTOTYPE: Test when element is not KeyValuePair<,> but there is an implicit conversion to KeyValuePair<K, V>. Should fail.
+                                    // PROTOTYPE: No dynamic support. Test that case, and update the spec accordingly.
+                                    if (IsKeyValuePairType(Compilation, enumeratorInfo.ElementType, out var itemKeyType, out var itemValueType))
+                                    {
+                                        var keyConversion = ClassifyImplicitConversionFromType(itemKeyType, keyType, ref useSiteInfo);
+                                        var valueConversion = ClassifyImplicitConversionFromType(itemValueType, valueType, ref useSiteInfo);
+                                        if (keyConversion.Exists && valueConversion.Exists)
+                                        {
+                                            builder.Add(keyConversion);
+                                            builder.Add(valueConversion);
+                                            continue;
+                                        }
+                                    }
+                                }
+
                             }
-                        }
-                        break;
-                    case BoundKeyValuePairElement keyValuePairElement:
-                        {
-                            if (usesKeyValuePairs)
+                            break;
+                        case BoundKeyValuePairElement keyValuePairElement:
                             {
-                                var keyConversion = ClassifyImplicitConversionFromExpression(keyValuePairElement.Key, keyType.Type, ref useSiteInfo);
-                                var valueConversion = ClassifyImplicitConversionFromExpression(keyValuePairElement.Value, valueType.Type, ref useSiteInfo);
+                                var keyConversion = ClassifyImplicitConversionFromExpression(keyValuePairElement.Key, keyType, ref useSiteInfo);
+                                var valueConversion = ClassifyImplicitConversionFromExpression(keyValuePairElement.Value, valueType, ref useSiteInfo);
                                 if (keyConversion.Exists && valueConversion.Exists)
                                 {
                                     builder.Add(keyConversion);
                                     builder.Add(valueConversion);
                                     continue;
                                 }
                             }
-                        }
-                        break;
-                    default:
-                        {
-                            var elementConversion = ClassifyImplicitConversionFromExpression((BoundExpression)element, elementType, ref useSiteInfo);
-                            if (elementConversion.Exists)
+                            break;
+                        case BoundExpression expressionElement:
                             {
-                                builder.Add(elementConversion);
-                                continue;
+                                // PROTOTYPE: dictionary-expressions.md currently states "If Ei is an expression element, there is an implicit conversion
+                                // from Ei to T." But that is incorrect since that allows conversions from types other than KeyValuePair<,>, and it disallows
+                                // conversions from KeyValuePair<Ke, Ve> to KeyValuePair<K, V> (if we remove "key-value pair conversions".) Update the
+                                // spec, removing "key-value pair conversions" and replacing the existing expression element rule with: "If Ei is an expression
+                                // element, and either: Ei has no type and there is an implicit conversion from Ei to T, or Ei has type KeyValuePair<Ke, Ve>
+                                // and there is an implicit conversion from Ke to K and an implicit conversion from Ve to V."
+
+                                // PROTOTYPE: Test when element is not KeyValuePair<,>, or when there is no conversion between key or value types.
+                                // PROTOTYPE: Test when element is not KeyValuePair<,> but there is an implicit conversion to KeyValuePair<K, V>. Should fail.
+                                // PROTOTYPE: No dynamic support. Test that case, and update the spec accordingly.
+                                if (expressionElement.Type is { })
+                                {
+                                    if (IsKeyValuePairType(Compilation, expressionElement.Type, out var elementKeyType, out var elementValueType))
+                                    {
+                                        var keyConversion = ClassifyImplicitConversionFromType(elementKeyType, keyType, ref useSiteInfo);
+                                        var valueConversion = ClassifyImplicitConversionFromType(elementValueType, valueType, ref useSiteInfo);
+                                        if (keyConversion.Exists && valueConversion.Exists)
+                                        {
+                                            builder.Add(keyConversion);
+                                            builder.Add(valueConversion);
+                                            continue;
+                                        }
+                                    }
+                                }
+                                else
+                                {
+                                    var expressionConversion = ClassifyImplicitConversionFromExpression(expressionElement, elementType, ref useSiteInfo);
+                                    if (expressionConversion.Exists)
+                                    {
+                                        builder.Add(expressionConversion);
+                                        continue;
+                                    }
+                                }
                             }
-                        }
-                        break;
+                            break;
+                    }
+                    builder.Free();
+                    return Conversion.NoConversion;
                 }
-                builder.Free();
-                return Conversion.NoConversion;
+                elementConversions = builder.ToImmutableAndFree();
+            }
+            else
+            {
+                var builder = ArrayBuilder<Conversion>.GetInstance(elements.Length);
+                foreach (var element in elements)
+                {
+                    switch (element)
+                    {
+                        case BoundCollectionExpressionWithElement:
+                            // Collection arguments do not affect convertibility.
+                            continue;
+                        case BoundCollectionExpressionSpreadElement spreadElement:
+                            {
+                                var elementConversion = GetCollectionExpressionSpreadElementConversion(spreadElement, elementType, ref useSiteInfo);
+                                if (elementConversion.Exists)
+                                {
+                                    builder.Add(elementConversion);
+                                    continue;
+                                }
+                            }
+                            break;
+                        case BoundExpression expressionElement:
+                            {
+                                var elementConversion = ClassifyImplicitConversionFromExpression(expressionElement, elementType, ref useSiteInfo);
+                                if (elementConversion.Exists)
+                                {
+                                    builder.Add(elementConversion);
+                                    continue;
+                                }
+                            }
+                            break;
+                    }
+                    builder.Free();
+                    return Conversion.NoConversion;
+                }
+                elementConversions = builder.ToImmutableAndFree();
             }
 
-            return Conversion.CreateCollectionExpressionConversion(collectionTypeKind, elementType, constructor, isExpanded, builder.ToImmutableAndFree());
+            return Conversion.CreateCollectionExpressionConversion(collectionTypeKind, elementType, constructor, isExpanded, elementConversions);
         }
 
         internal Conversion GetCollectionExpressionSpreadElementConversion(