Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -8135,7 +8135,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>'value': an automatically-generated parameter name conflicts with an extension type parameter name</value>
</data>
<data name="ERR_UnderspecifiedExtension" xml:space="preserve">
<value>The extended type '{0}' must reference all the type parameters declared by the extension, but type parameter '{1}' is not referenced.</value>
<value>Every type parameter from the extension block must be referenced by the extension parameter or a parameter of this member. But type parameter `{0}` is not referenced.</value>
Copy link
Member

Choose a reason for hiding this comment

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

I think this is far too wordy.

Suggested change
<value>Every type parameter from the extension block must be referenced by the extension parameter or a parameter of this member. But type parameter `{0}` is not referenced.</value>
<value>The type parameter `{0}` is not referenced by either the extension parameter or a parameter of this member.</value>

</data>
<data name="ERR_ExpressionTreeContainsExtensionPropertyAccess" xml:space="preserve">
<value>An expression tree may not contain an extension property access</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymb

if (parameter is { })
{
checkUnderspecifiedGenericExtension(parameter, ContainingType.TypeParameters, diagnostics);

TypeSymbol parameterType = parameter.TypeWithAnnotations.Type;
RefKind parameterRefKind = parameter.RefKind;
SyntaxNode? parameterTypeSyntax = parameterList.Parameters[0].Type;
Expand Down Expand Up @@ -112,34 +110,6 @@ protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymb

return parameter;
}

static void checkUnderspecifiedGenericExtension(ParameterSymbol parameter, ImmutableArray<TypeParameterSymbol> typeParameters, BindingDiagnosticBag diagnostics)
Copy link
Member Author

@jcouv jcouv May 30, 2025

Choose a reason for hiding this comment

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

📝 moved this logic to SourceNamedTypeSymbol.AfterMembersCompletedChecks as the added GetMembers call was causing a circularity issue (out-of-date after new LDM decision)

{
var underlyingType = parameter.Type;
var usedTypeParameters = PooledHashSet<TypeParameterSymbol>.GetInstance();
underlyingType.VisitType(collectTypeParameters, arg: usedTypeParameters);

foreach (var typeParameter in typeParameters)
{
if (!usedTypeParameters.Contains(typeParameter))
{
diagnostics.Add(ErrorCode.ERR_UnderspecifiedExtension, parameter.GetFirstLocation(), underlyingType, typeParameter);
}
}

usedTypeParameters.Free();
}

static bool collectTypeParameters(TypeSymbol type, PooledHashSet<TypeParameterSymbol> typeParameters, bool ignored)
{
if (type is TypeParameterSymbol typeParameter)
{
typeParameters.Add(typeParameter);
}

return false;
}

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,44 @@ private static void EnsureNullableAttributeExists(CSharpCompilation compilation,
}
}

internal static void CheckUnderspecifiedGenericExtension(Symbol extensionMember, ImmutableArray<ParameterSymbol> parameters, BindingDiagnosticBag diagnostics)
{
Debug.Assert(extensionMember.GetIsNewExtensionMember());

NamedTypeSymbol extension = extensionMember.ContainingType;
if (extension.ExtensionParameter is not { } extensionParameter)
{
return;
}

var usedTypeParameters = PooledHashSet<TypeParameterSymbol>.GetInstance();
extensionParameter.Type.VisitType(collectTypeParameters, arg: usedTypeParameters);
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 5, 2025

Choose a reason for hiding this comment

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

extensionParameter.Type.VisitType(collectTypeParameters, arg: usedTypeParameters);

Consider returning early if we already recorded extension.TypeParameters.Length type parameters after this. #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 5, 2025

Choose a reason for hiding this comment

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

extensionParameter.Type.VisitType(collectTypeParameters, arg: usedTypeParameters);

It feels unfortunate that we have to repeat this for every member. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I could leave an follow-up comment in "optimization/MQ" bucket, but I think I'm fine to leave as-is

foreach (var parameter in parameters)
{
parameter.Type.VisitType(collectTypeParameters, arg: usedTypeParameters);
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 5, 2025

Choose a reason for hiding this comment

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

parameter.Type.VisitType(collectTypeParameters, arg: usedTypeParameters);

Consider breaking out early if we already recorded extension.TypeParameters.Length type parameters. #Closed

}

foreach (var typeParameter in extension.TypeParameters)
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 5, 2025

Choose a reason for hiding this comment

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

extension.TypeParameters

Consider adding an early return if we have no type parameters #Closed

{
if (!usedTypeParameters.Contains(typeParameter))
{
diagnostics.Add(ErrorCode.ERR_UnderspecifiedExtension, extensionMember.GetFirstLocation(), typeParameter);
}
}

usedTypeParameters.Free();

static bool collectTypeParameters(TypeSymbol type, PooledHashSet<TypeParameterSymbol> typeParameters, bool ignored)
{
if (type is TypeParameterSymbol typeParameter)
{
typeParameters.Add(typeParameter);
}

return false;
}
}

internal static Location GetParameterLocation(ParameterSymbol parameter) => parameter.GetNonNullSyntaxNode().Location;

internal static void CheckParameterModifiers(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,

ParameterHelpers.EnsureNullableAttributeExists(compilation, this, Parameters, diagnostics, modifyCompilation: true);

if (this.GetIsNewExtensionMember() && MethodKind != MethodKind.Ordinary)
{
ParameterHelpers.CheckUnderspecifiedGenericExtension(this, Parameters, diagnostics);
}

Location getReturnTypeLocation()
{
returnTypeLocation ??= this.ReturnTypeLocation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,11 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,
}

ParameterHelpers.EnsureNullableAttributeExists(compilation, this, Parameters, diagnostics, modifyCompilation: true);

if (this.GetIsNewExtensionMember())
{
ParameterHelpers.CheckUnderspecifiedGenericExtension(this, Parameters, diagnostics);
}
}

private void CheckAccessibility(Location location, BindingDiagnosticBag diagnostics, bool isExplicitInterfaceImplementation)
Expand Down
4 changes: 2 additions & 2 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.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

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

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

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

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

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

Loading