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

Address some PROTOTYPE comments #72298

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 23 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -810,8 +811,10 @@ internal BoundExpression BindCollectionExpressionConstructor(SyntaxNode syntax,
return collectionCreation;
}

internal bool HasCollectionExpressionApplicableConstructor(SyntaxNode syntax, TypeSymbol targetType, out MethodSymbol? constructor, out bool isExpanded, BindingDiagnosticBag diagnostics)
internal bool HasCollectionExpressionApplicableConstructor(SyntaxNode syntax, TypeSymbol targetType, out MethodSymbol? constructor, out bool isExpanded, BindingDiagnosticBag diagnostics, bool isParamsModifierValidation = false)
{
Debug.Assert(!isParamsModifierValidation || syntax is ParameterSyntax);

// This is what BindClassCreationExpression is doing in terms of reporting diagnostics

constructor = null;
Expand Down Expand Up @@ -847,13 +850,14 @@ internal bool HasCollectionExpressionApplicableConstructor(SyntaxNode syntax, Ty
out MemberResolutionResult<MethodSymbol> memberResolutionResult,
candidateConstructors: out _,
allowProtectedConstructorsOfBaseType: false,
out CompoundUseSiteInfo<AssemblySymbol> overloadResolutionUseSiteInfo);
out CompoundUseSiteInfo<AssemblySymbol> overloadResolutionUseSiteInfo,
isParamsModifierValidation: isParamsModifierValidation);

analyzedArguments.Free();

if (overloadResolutionSucceeded)
{
bindClassCreationExpressionContinued(binder, syntax, memberResolutionResult, in overloadResolutionUseSiteInfo, diagnostics);
bindClassCreationExpressionContinued(binder, syntax, memberResolutionResult, in overloadResolutionUseSiteInfo, isParamsModifierValidation, diagnostics);
constructor = memberResolutionResult.Member;
isExpanded = memberResolutionResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm;
}
Expand All @@ -879,6 +883,7 @@ static void bindClassCreationExpressionContinued(
SyntaxNode node,
MemberResolutionResult<MethodSymbol> memberResolutionResult,
in CompoundUseSiteInfo<AssemblySymbol> overloadResolutionUseSiteInfo,
bool isParamsModifierValidation,
BindingDiagnosticBag diagnostics)
{
ReportConstructorUseSiteDiagnostics(node.Location, diagnostics, suppressUnsupportedRequiredMembersError: false, in overloadResolutionUseSiteInfo);
Expand All @@ -888,7 +893,21 @@ static void bindClassCreationExpressionContinued(
binder.ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver: false);
// NOTE: Use-site diagnostics were reported during overload resolution.

CheckRequiredMembersInObjectInitializer(method, initializers: default, node, diagnostics);
ImmutableSegmentedDictionary<string, Symbol> requiredMembers = GetMembersRequiringInitialization(method);
if (requiredMembers.Count != 0)
{
if (isParamsModifierValidation)
{
diagnostics.Add(
ErrorCode.ERR_ParamsCollectionConstructorDoesntInitializeRequiredMember,
((ParameterSyntax)node).Modifiers.First(static m => m.IsKind(SyntaxKind.ParamsKeyword)).GetLocation(),
method, requiredMembers.First().Value);
}
else
{
ReportMembersRequiringInitialization(node, requiredMembers.ToBuilder(), diagnostics);
}
}
}

// This is what CreateBadClassCreationExpression is doing in terms of reporting diagnostics
Expand Down
73 changes: 38 additions & 35 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5610,24 +5610,24 @@ private static void ReportDuplicateObjectMemberInitializers(BoundExpression boun
}

#nullable enable
private static ImmutableSegmentedDictionary<string, Symbol> GetMembersRequiringInitialization(MethodSymbol constructor)
{
if (!constructor.ShouldCheckRequiredMembers() ||
constructor.ContainingType.HasRequiredMembersError) // An error will be reported on the constructor if from source, or a use-site diagnostic will be reported on the use if from metadata.
{
return ImmutableSegmentedDictionary<string, Symbol>.Empty;
}

return constructor.ContainingType.AllRequiredMembers;
}

internal static void CheckRequiredMembersInObjectInitializer(
MethodSymbol constructor,
ImmutableArray<BoundExpression> initializers,
SyntaxNode creationSyntax,
BindingDiagnosticBag diagnostics)
{
if (!constructor.ShouldCheckRequiredMembers())
{
return;
}

if (constructor.ContainingType.HasRequiredMembersError)
{
// An error will be reported on the constructor if from source, or a use-site diagnostic will be reported on the use if from metadata.
return;
}

var requiredMembers = constructor.ContainingType.AllRequiredMembers;
ImmutableSegmentedDictionary<string, Symbol> requiredMembers = GetMembersRequiringInitialization(constructor);

if (requiredMembers.Count == 0)
{
Expand All @@ -5638,7 +5638,7 @@ internal static void CheckRequiredMembersInObjectInitializer(

if (initializers.IsDefaultOrEmpty)
{
reportMembers();
ReportMembersRequiringInitialization(creationSyntax, requiredMembersBuilder, diagnostics);
return;
}

Expand Down Expand Up @@ -5684,29 +5684,29 @@ internal static void CheckRequiredMembersInObjectInitializer(
}
}

reportMembers();
ReportMembersRequiringInitialization(creationSyntax, requiredMembersBuilder, diagnostics);
}

void reportMembers()
private static void ReportMembersRequiringInitialization(SyntaxNode creationSyntax, ImmutableSegmentedDictionary<string, Symbol>.Builder requiredMembersBuilder, BindingDiagnosticBag diagnostics)
{
if (requiredMembersBuilder.Count == 0)
{
if (requiredMembersBuilder.Count == 0)
{
// Avoid Location allocation.
return;
}
// Avoid Location allocation.
return;
}

Location location = creationSyntax switch
{
ObjectCreationExpressionSyntax { Type: { } type } => type.Location,
BaseObjectCreationExpressionSyntax { NewKeyword: { } newKeyword } => newKeyword.GetLocation(),
AttributeSyntax { Name: { } name } => name.Location,
_ => creationSyntax.Location
};
Location location = creationSyntax switch
{
ObjectCreationExpressionSyntax { Type: { } type } => type.Location,
BaseObjectCreationExpressionSyntax { NewKeyword: { } newKeyword } => newKeyword.GetLocation(),
AttributeSyntax { Name: { } name } => name.Location,
_ => creationSyntax.Location
};

foreach (var (_, member) in requiredMembersBuilder)
{
// Required member '{0}' must be set in the object initializer or attribute constructor.
diagnostics.Add(ErrorCode.ERR_RequiredMemberMustBeSet, location, member);
}
foreach (var (_, member) in requiredMembersBuilder)
{
// Required member '{0}' must be set in the object initializer or attribute constructor.
diagnostics.Add(ErrorCode.ERR_RequiredMemberMustBeSet, location, member);
}
}
#nullable disable
Expand Down Expand Up @@ -6661,7 +6661,8 @@ internal bool TryPerformConstructorOverloadResolution(
out MemberResolutionResult<MethodSymbol> memberResolutionResult,
out ImmutableArray<MethodSymbol> candidateConstructors,
bool allowProtectedConstructorsOfBaseType,
out CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
out CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
bool isParamsModifierValidation = false)
{
// Get accessible constructors for performing overload resolution.
ImmutableArray<MethodSymbol> allInstanceConstructors;
Expand Down Expand Up @@ -6746,7 +6747,8 @@ internal bool TryPerformConstructorOverloadResolution(
result.ReportDiagnostics(
binder: this, location: errorLocation, nodeOpt: null, diagnostics,
name: errorName, receiver: null, invokedExpression: null, analyzedArguments,
memberGroup: candidateConstructors, typeContainingConstructors, delegateTypeBeingInvoked: null);
memberGroup: candidateConstructors, typeContainingConstructors, delegateTypeBeingInvoked: null,
isParamsModifierValidation: isParamsModifierValidation);
}
}

Expand Down Expand Up @@ -7617,7 +7619,8 @@ private void BindMemberAccessReportError(
}
else if (boundLeft.Kind == BoundKind.TypeExpression ||
boundLeft.Kind == BoundKind.BaseReference ||
node.Kind() == SyntaxKind.AwaitExpression && plainName == WellKnownMemberNames.GetResult)
(node.Kind() == SyntaxKind.AwaitExpression && plainName == WellKnownMemberNames.GetResult) ||
(Flags.Includes(BinderFlags.CollectionExpressionConversionValidation | BinderFlags.CollectionInitializerAddMethod) && name is ParameterSyntax))
{
Error(diagnostics, ErrorCode.ERR_NoSuchMember, name, boundLeft.Type, plainName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ internal void ReportDiagnostics<T>(
CSharpSyntaxNode queryClause = null,
bool isMethodGroupConversion = false,
RefKind? returnRefKind = null,
TypeSymbol delegateOrFunctionPointerType = null) where T : Symbol
TypeSymbol delegateOrFunctionPointerType = null,
bool isParamsModifierValidation = false) where T : Symbol
{
Debug.Assert(!this.Succeeded, "Don't ask for diagnostic info on a successful overload resolution result.");

Expand Down Expand Up @@ -509,7 +510,11 @@ internal void ReportDiagnostics<T>(
if (receiver is null)
{
Debug.Assert(firstSupported.Member is MethodSymbol { MethodKind: MethodKind.Constructor });
diagnostics.Add(ErrorCode.ERR_CollectionExpressionMissingConstructor, location);
diagnostics.Add(
isParamsModifierValidation ?
ErrorCode.ERR_ParamsCollectionMissingConstructor :
ErrorCode.ERR_CollectionExpressionMissingConstructor,
location);
}
else
{
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7878,4 +7878,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ParamsMemberCannotBeLessVisibleThanDeclaringMember" xml:space="preserve">
<value>Method '{0}' cannot be less visible than the member with params collection '{1}'.</value>
</data>
<data name="ERR_ParamsCollectionConstructorDoesntInitializeRequiredMember" xml:space="preserve">
<value>Constructor '{0}' leaves required member '{1}' uninitialized.</value>
</data>
<data name="ERR_ParamsCollectionExpressionTree" xml:space="preserve">
<value>An expression tree may not contain an expanded form of non-array params collection parameter.</value>
</data>
<data name="ERR_ParamsCollectionExtensionAddMethod" xml:space="preserve">
<value>'{0}' does not contain a definition for a suitable instance 'Add' method</value>
</data>
<data name="ERR_ParamsCollectionMissingConstructor" xml:space="preserve">
<value>Non-array params collection type must have an applicable constructor that can be called with no arguments.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,7 @@ internal SynthesizedAttributeData SynthesizeParamCollectionAttribute(ParameterSy
if ((object)Compilation.SourceModule != symbol.ContainingModule)
{
// For symbols that are not defined in the same compilation (like NoPia), don't synthesize this attribute.
return null; // PROTOTYPE(ParamsCollections): Test this code path
return null;
}

return TrySynthesizeParamCollectionAttribute();
Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,10 @@ internal enum ErrorCode
WRN_DynamicDispatchToParamsCollectionConstructor = 9505,
ERR_ParamsCollectionInfiniteChainOfConstructorCalls = 9506,
ERR_ParamsMemberCannotBeLessVisibleThanDeclaringMember = 9507,
ERR_ParamsCollectionConstructorDoesntInitializeRequiredMember = 9508,
ERR_ParamsCollectionExpressionTree = 9509,
ERR_ParamsCollectionExtensionAddMethod = 9510,
ERR_ParamsCollectionMissingConstructor = 9511,

#endregion

Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,10 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.WRN_DynamicDispatchToParamsCollectionConstructor:
case ErrorCode.ERR_ParamsCollectionInfiniteChainOfConstructorCalls:
case ErrorCode.ERR_ParamsMemberCannotBeLessVisibleThanDeclaringMember:
case ErrorCode.ERR_ParamsCollectionConstructorDoesntInitializeRequiredMember:
case ErrorCode.ERR_ParamsCollectionExpressionTree:
case ErrorCode.ERR_ParamsCollectionExtensionAddMethod:
case ErrorCode.ERR_ParamsCollectionMissingConstructor:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6833,7 +6833,6 @@ static void expandParamsCollection(ref ImmutableArray<BoundExpression> arguments
}
else
{
// PROTOTYPE(ParamsCollections): Add meaningful tests to verify that conversions are handled properly etc.
elements = ((BoundCollectionExpression)((BoundConversion)argument).Operand).UnconvertedCollectionExpression.Elements.CastArray<BoundExpression>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,11 @@ public override BoundNode VisitCollectionExpression(BoundCollectionExpression no
{
if (_inExpressionLambda)
{
Error(ErrorCode.ERR_ExpressionTreeContainsCollectionExpression, node);
Error(
node.IsParamsArrayOrCollection ?
ErrorCode.ERR_ParamsCollectionExpressionTree :
ErrorCode.ERR_ExpressionTreeContainsCollectionExpression,
node);
}

return base.VisitCollectionExpression(node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1569,7 +1569,7 @@ void validateParamsType(BindingDiagnosticBag diagnostics)
return;
}

if (!binder.HasCollectionExpressionApplicableConstructor(syntax, Type, out MethodSymbol? constructor, isExpanded: out _, diagnostics))
if (!binder.HasCollectionExpressionApplicableConstructor(syntax, Type, out MethodSymbol? constructor, isExpanded: out _, diagnostics, isParamsModifierValidation: true))
{
return;
}
Expand All @@ -1589,7 +1589,7 @@ void validateParamsType(BindingDiagnosticBag diagnostics)
if (addMethods[0].IsStatic) // No need to check other methods, extensions are never mixed with instance methods
{
Debug.Assert(addMethods[0].IsExtensionMethod);
diagnostics.Add(ErrorCode.ERR_NoSuchMember, syntax, Type, WellKnownMemberNames.CollectionInitializerAddMethodName);
diagnostics.Add(ErrorCode.ERR_ParamsCollectionExtensionAddMethod, syntax, Type);
return;
}

Expand Down
20 changes: 20 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading