Skip to content

Extensions: relax inferrability rule #78758

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

Merged
jcouv merged 8 commits into from
Jun 6, 2025
Merged

Extensions: relax inferrability rule #78758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ace0477
Extensions: relax inferrability rule for methods
May 29, 2025
46636d7
Address feedback
Jun 2, 2025
4996484
Tweak
Jun 3, 2025
39dd249
Extension type parameters must be used by parameters, except for methods
Jun 5, 2025
129813b
Tweaks
Jun 5, 2025
3c27068
Address feedback
Jun 5, 2025
f00db0c
Check within loop
Jun 5, 2025
d158ed3
Address feedback
Jun 6, 2025
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
View file
Open in desktop
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>The extended type '{0}' must reference all the type parameters declared by the extension, since the extension block contains a non-method member, but type parameter '{1}' is not referenced.</value>
</data>
<data name="ERR_ExpressionTreeContainsExtensionPropertyAccess" xml:space="preserve">
<value>An expression tree may not contain an extension property access</value>
Expand Down
30 changes: 0 additions & 30 deletions src/Compilers/CSharp/Portable/Symbols/Extensions/SynthesizedExtensionMarker.cs
View file
Open in desktop
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 •
edited
Loading

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
39 changes: 39 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,45 @@ protected override void AfterMembersCompletedChecks(BindingDiagnosticBag diagnos
var syntax = (ExtensionBlockDeclarationSyntax)this.GetNonNullSyntaxNode();
diagnostics.Add(ErrorCode.ERR_BadExtensionContainingType, syntax.Keyword);
}

if (TryGetOrCreateExtensionMarker() is { Parameters: [var extensionParameter] })
{
checkUnderspecifiedGenericExtension(extensionParameter, TypeParameters, diagnostics);
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

checkUnderspecifiedGenericExtension

I think that long term it would be better to perform this check per member and report an error per member. For example, once we enable indexers, I can imagine that some indexer declarations (referencing remaining type arguments in their parameters) will be allowed, and others won't be allowed. #Closed

}
}

return;

void checkUnderspecifiedGenericExtension(ParameterSymbol parameter, ImmutableArray<TypeParameterSymbol> typeParameters, BindingDiagnosticBag diagnostics)
{
if (GetMembers().All((m, a) => m is MethodSymbol { MethodKind: MethodKind.Ordinary }, arg: (object?)null))
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All

Is there an overload that doesn't need unused additional argument? Perhaps we have Any instead? #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

MethodKind.Ordinary

I think operators should be allowed too. At least, consider leaving a comment to follow up, the one that starts with a link to the test plan. Operators are being worked on as we speak. BTW, similar to indexers, I think that some operator declarations would be allowed and other operator declarations wouldn't be allowed. This is even stronger reason to moving to per member check/reporting. #Closed

Copy link
Member Author

@jcouv jcouv Jun 2, 2025

Choose a reason for hiding this comment

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

The criteria we last discussed in WG is whether it's possible to use the member with explicit type arguments.
So I don't think we'll allow either indexers or operators in non-inferrable extension blocks.

FWIW, here's the update I captured in the spec (to be reviewed in LDM this week):

__Inferrability:__ All the type parameters of an extension block must be used in the receiver type when the extension block
contains a non-method member. 
This makes it always possible to infer the type arguments when applied to a receiver of the given receiver type and
the member doesn't allow explicit type arguments.

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 3, 2025

Choose a reason for hiding this comment

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

I do see that in the summary, but, to be honest, I do not remember us having a serious discussion about the criteria to the point that I must have forgotten us deciding on having the restriction. At the same time, from our "Tensors and extension operators" meeting I vaguely remember us expressing a desire to support the following scenario from #78472 (comment):

extension<TTensor, TScalar>(TTensor)
    where TTensor : IReadOnlyTensor<TTensor, TScalar>
    where TScalar : IAdditionOperators<TScalar, TScalar, TScalar>
{
    public static TTensor operator +(TTensor left, TScalar right);
}

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here is a link to the specific summary message from Tanner specifically mentioning the following shape as planned for .NET 10:

extension<TTensor, TScalar>(TTensor)
    where TTensor : IReadOnlyTensor<TTensor, TScalar>
    where TScalar : IAdditionOperators<TScalar, TScalar, TScalar>
{
    public static TTensor operator +(TTensor left, TScalar right);
}

{
return;
}

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
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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.zh-Hans.xlf
View file
Open in desktop

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.zh-Hant.xlf
View file
Open in desktop

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

Loading