Conversation
…ameterless constructor exists Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Extra.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Extra.cs
Show resolved
Hide resolved
…st, add Debug.Assert Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Extra.cs
Outdated
Show resolved
Hide resolved
…fy test with expectedOutput Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
| Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "with()").WithArguments("x", "MyCollection<int>.MyCollection(int, params MyCollection<int>)").WithLocation(3, 6), | ||
| // (4,6): error CS9223: Creation of params collection 'MyCollection<int>' results in an infinite chain of invocation of constructor 'MyCollection<T>.MyCollection(T, params MyCollection<T>)'. | ||
| // c = [with(1)]; |
There was a problem hiding this comment.
As I can understand here is also won't be any infinite recursion, because we cannot instantiate MyCollection<T> without at least one argument.
But I expect here an error of unresolved constructor call of MyCollection<T> due to arguments to parameters mismatch, because the params MyCollection<T> y parameter will be treated as a regular non-params parameter (because there is no default .ctor on MyCollection<T>), therefore we didn't provided the suitable argument for it.
@jjonescz #Closed
There was a problem hiding this comment.
It is required that a params collection has a parameterless constructor, that's why we have the error below for this (8,30): error CS9228: Non-array params collection type must have an applicable constructor that can be called with no arguments.
Then the compiler can assume it can create the params collection without arguments and the cycle error makes sense. Sure we could omit this error but it wouldn't solve much, just slightly better error recovery.
| Debug.Assert(inProgressConstructor is not null); | ||
| _diagnostics.Add(ErrorCode.ERR_ParamsCollectionInfiniteChainOfConstructorCalls, syntax, inProgress, inProgressConstructor.OriginalDefinition); | ||
| return null; | ||
| if (constructor is null) |
| if (!hasWithElement) | ||
| { | ||
| // We are in a cycle, but without a with-element, a parameterless constructor can break the | ||
| // cycle since it won't recursively invoke the params constructor. Check if one is accessible. |
There was a problem hiding this comment.
This logic feels fragile and suspicious.
| @@ -979,9 +979,17 @@ CollectionExpressionTypeKind.Array or CollectionExpressionTypeKind.Span or Colle | |||
| if (_targetType is NamedTypeSymbol namedType && | |||
| _binder.HasParamsCollectionTypeInProgress(namedType, out NamedTypeSymbol? inProgress, out MethodSymbol? inProgressConstructor)) | |||
Is this line doing the right thing when And, if I suppress creation of this binder in presence of Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs:1058 in fe0b802. [](commit_id = fe0b802, deletion_comment = False) |
|
Done with review pass (commit 4) |
MyCollectionhas both a parameterless constructor and a params constructorHasCollectionExpressionApplicableConstructorto find the parameterless constructor when in cycle with no with-elementTryConvertCollectionExpressionImplementsIEnumerableTypeto only report CS9223 when constructor is null (true cycle)ParamsCycle_MultipleConstructorsto remove false positive expectation + add[WorkItem]attributeParamsCycle_MultipleConstructors_ClassContexttest (same coverage as existing test)Debug.Assert(constructor.Parameters.IsEmpty)and improve comment to clarify constructor propagationParamsCycle_MultipleConstructors_ClassContextas aCompileAndVerifytest withexpectedOutput: "1"to verify code executionOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.