Skip to content

Adjust visibility of extension marker method in metadata once more #79617

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 2 commits into from
Jul 28, 2025
+83 −80
Merged

Adjust visibility of extension marker method in metadata once more #79617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a829c1
Adjust visibility of extension marker method in metadata once more
AlekseyTs Jul 25, 2025
ea90916
Formatting
AlekseyTs Jul 25, 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
9 changes: 4 additions & 5 deletions src/Compilers/CSharp/Portable/Emitter/Model/MethodSymbolAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ Cci.ITypeReference Cci.ITypeMemberReference.GetContainingType(EmitContext contex

NamedTypeSymbol containingType = AdaptedMethodSymbol.ContainingType;

if (AdaptedMethodSymbol is SynthesizedExtensionMarker)
if (AdaptedMethodSymbol is SynthesizedExtensionMarker marker)
{
return ((SourceMemberContainerTypeSymbol)containingType.ContainingType).GetExtensionGroupingInfo().GetCorrespondingMarkerType((SourceNamedTypeSymbol)containingType);
return ((SourceMemberContainerTypeSymbol)containingType.ContainingType).GetExtensionGroupingInfo().GetCorrespondingMarkerType(marker);
}
else if (AdaptedMethodSymbol.GetIsNewExtensionMember())
{
Expand Down Expand Up @@ -311,10 +311,9 @@ Cci.ITypeDefinition Cci.ITypeDefinitionMember.ContainingTypeDefinition
}

// PROTOTYPE: Share logic with Cci.ITypeMemberReference.GetContainingType implementation?
if (AdaptedMethodSymbol is SynthesizedExtensionMarker)
if (AdaptedMethodSymbol is SynthesizedExtensionMarker marker)
{
var containingType = AdaptedMethodSymbol.ContainingType;
return ((SourceMemberContainerTypeSymbol)containingType.ContainingType).GetExtensionGroupingInfo().GetCorrespondingMarkerType((SourceNamedTypeSymbol)containingType);
return ((SourceMemberContainerTypeSymbol)AdaptedMethodSymbol.ContainingType.ContainingType).GetExtensionGroupingInfo().GetCorrespondingMarkerType(marker);
}
else if (AdaptedMethodSymbol.GetIsNewExtensionMember())
{
Expand Down
34 changes: 1 addition & 33 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 @@ -19,8 +19,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
/// </summary>
internal sealed class SynthesizedExtensionMarker : SynthesizedSourceOrdinaryMethodSymbol
{
private byte _lazyMetadataVisibility = 0xFF;

internal SynthesizedExtensionMarker(SourceMemberContainerTypeSymbol extensionType, ParameterListSyntax parameterList)
: base(extensionType, WellKnownMemberNames.ExtensionMarkerMethodName, parameterList.OpenParenToken.GetLocation(), parameterList,
(GetDeclarationModifiers(), MakeFlags(
Expand All @@ -41,37 +39,7 @@ public override TypeMemberVisibility MetadataVisibility
{
get
{
if (_lazyMetadataVisibility == 0xFF)
{
_lazyMetadataVisibility = (byte)calculate();
Debug.Assert(_lazyMetadataVisibility != 0xFF);
}

return (TypeMemberVisibility)_lazyMetadataVisibility;

TypeMemberVisibility calculate()
{
if (Parameters is not [var parameter, ..])
{
return TypeMemberVisibility.Private;
}

var useSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;

var testField = new SynthesizedFieldSymbol(ContainingType, parameter.Type, WellKnownMemberNames.ExtensionMarkerMethodName, DeclarationModifiers.Public);
if (parameter.TypeWithAnnotations.IsAtLeastAsVisibleAs(testField, ref useSiteInfo))
{
return TypeMemberVisibility.Public;
}

testField = new SynthesizedFieldSymbol(ContainingType, parameter.Type, WellKnownMemberNames.ExtensionMarkerMethodName, DeclarationModifiers.Internal);
if (parameter.TypeWithAnnotations.IsAtLeastAsVisibleAs(testField, ref useSiteInfo))
{
return TypeMemberVisibility.Assembly;
}

return TypeMemberVisibility.Private;
}
return ((SourceMemberContainerTypeSymbol)ContainingType.ContainingType).GetExtensionGroupingInfo().GetCorrespondingMarkerMethodVisibility(this);
}
}

Expand Down
38 changes: 37 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/ExtensionGroupingInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ public ExtensionGroupingInfo(SourceMemberContainerTypeSymbol container)
return ImmutableArray<Cci.INestedTypeDefinition>.CastUp(_lazyGroupingTypes);
}

public Cci.ITypeDefinition GetCorrespondingMarkerType(SourceNamedTypeSymbol type)
public Cci.ITypeDefinition GetCorrespondingMarkerType(SynthesizedExtensionMarker markerMethod)
{
return GetCorrespondingMarkerType((SourceNamedTypeSymbol)markerMethod.ContainingType);
}

private ExtensionMarkerType GetCorrespondingMarkerType(SourceNamedTypeSymbol type)
{
GetGroupingTypes();

Expand Down Expand Up @@ -104,6 +109,37 @@ public Cci.ITypeDefinition GetCorrespondingMarkerType(SourceNamedTypeSymbol type
throw ExceptionUtilities.Unreachable();
}

public Cci.TypeMemberVisibility GetCorrespondingMarkerMethodVisibility(SynthesizedExtensionMarker marker)
{
Debug.Assert(Cci.TypeMemberVisibility.Public > Cci.TypeMemberVisibility.Assembly);
Debug.Assert(Cci.TypeMemberVisibility.Assembly > Cci.TypeMemberVisibility.Private);

var result = Cci.TypeMemberVisibility.Private;

foreach (var extension in GetCorrespondingMarkerType((SourceNamedTypeSymbol)marker.ContainingType).UnderlyingExtensions)
{
foreach (var symbol in extension.GetMembers())
Copy link
Member

@jjonescz jjonescz Jul 25, 2025 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

Why are we using members now to infer the visibility? #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs Jul 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why are we using members now to infer the visibility?

We discussed this in our working group and decided that doing this will be better. For example, empty extensions will not have impact on public surface. If all members are internal, there will be no impact on public surface even if extended type is public, etc.

{
var memberVisibility = symbol.MetadataVisibility;
Debug.Assert(memberVisibility is Cci.TypeMemberVisibility.Public or Cci.TypeMemberVisibility.Assembly or Cci.TypeMemberVisibility.Private);

if (memberVisibility == Cci.TypeMemberVisibility.Public)
{
return TypeMemberVisibility.Public;
}

if (result < memberVisibility)
{
// If we have a more visible member, use its visibility.
result = memberVisibility;
}
}
}

// PROTOTYPE: Is there a real need to cache this result for reuse?
return result;
}

public Cci.ITypeDefinition GetCorrespondingGroupingType(SourceNamedTypeSymbol type)
{
GetGroupingTypes();
Expand Down
Loading