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

Fix HasFileLocalTypes check for constructed types #68386

Merged
merged 9 commits into from
Jun 9, 2023
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ internal static ImmutableArray<Symbol> GetCandidateMembers(NamespaceOrTypeSymbol

private bool IsInScopeOfAssociatedSyntaxTree(Symbol symbol)
{
while (symbol is not null and not NamedTypeSymbol { AssociatedFileIdentifier: not null })
while (symbol is not null and not NamedTypeSymbol { IsFileLocal: true })
{
symbol = symbol.ContainingType;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2480,7 +2480,7 @@ private BestSymbolInfo GetBestSymbolInfo(ArrayBuilder<Symbol> symbols, out BestS

private static BestSymbolLocation GetLocation(CSharpCompilation compilation, Symbol symbol)
{
if (symbol is SourceMemberContainerTypeSymbol { IsFileLocal: true })
if (symbol is NamedTypeSymbol { IsFileLocal: true })
{
return BestSymbolLocation.FromFile;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3392,7 +3392,7 @@ public override void VisitNamespace(NamespaceSymbol symbol)
public override void VisitNamedType(NamedTypeSymbol symbol)
{
Debug.Assert(symbol.ContainingSymbol.Kind == SymbolKind.Namespace); // avoid unnecessary traversal of nested types
if (symbol.AssociatedFileIdentifier is not null)
if (symbol.IsFileLocal)
Copy link
Member

Choose a reason for hiding this comment

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

Nice. I think this reads better/clearer :-)

{
var location = symbol.GetFirstLocation();
var filePath = location.SourceTree?.FilePath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ internal sealed override bool MangleName
get { return false; }
}

internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

public sealed override int Arity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ internal sealed override bool MangleName
get { return this.Arity > 0; }
}

internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

internal sealed override ImmutableArray<TypeWithAnnotations> TypeArgumentsWithAnnotationsNoUseSiteDiagnostics
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/ErrorTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,8 @@ internal override bool MangleName
get { return _originalDefinition.MangleName; }
}

internal override FileIdentifier? AssociatedFileIdentifier => _originalDefinition.AssociatedFileIdentifier;
internal sealed override bool IsFileLocal => _originalDefinition.IsFileLocal;
internal sealed override FileIdentifier? AssociatedFileIdentifier => _originalDefinition.AssociatedFileIdentifier;

internal override DiagnosticInfo? ErrorInfo
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ internal override bool MangleName
}
}

internal override bool IsFileLocal => false;
internal override FileIdentifier? AssociatedFileIdentifier => null;

public override Symbol? ContainingSymbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ internal override ModuleSymbol ContainingModule
get;
}

internal override FileIdentifier? AssociatedFileIdentifier
internal sealed override bool IsFileLocal => _lazyUncommonProperties is { lazyFilePathChecksum: { IsDefault: false }, lazyDisplayFileName: { } };
internal sealed override FileIdentifier? AssociatedFileIdentifier
{
get
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ internal override bool MangleName
}
}

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

/// <summary>
/// Get the arity of the missing type.
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@ public override string MetadataName
}
}

internal abstract bool IsFileLocal { get; }

/// <summary>
/// If this type is a file-local type, returns an identifier for the file this type was declared in. Otherwise, returns null.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ internal override UseSiteInfo<AssemblySymbol> GetUseSiteInfo()
internal sealed override NamedTypeSymbol NativeIntegerUnderlyingType => _underlyingType;

// note: there is no supported way to create a native integer type whose underlying type is file-local.
internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

internal sealed override bool IsRecord => false;
internal sealed override bool IsRecordStruct => false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ internal override bool MangleName
}
}

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

public AssemblySymbol EmbeddingAssembly
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ internal override bool MangleName
}
}

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

public NamedTypeSymbol UnderlyingSymbol
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ internal override bool MangleName
}
}

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

public string? Guid
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ internal override bool MangleName
}
}

internal override bool IsFileLocal => false;
internal override FileIdentifier? AssociatedFileIdentifier => null;

internal override DiagnosticInfo? ErrorInfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,10 @@ INamedTypeSymbol INamedTypeSymbol.TupleUnderlyingType
bool INamedTypeSymbol.IsSerializable => UnderlyingNamedTypeSymbol.IsSerializable;

bool INamedTypeSymbol.IsFileLocal =>
// Even though we round-trip the file identifier through metadata to support EE,
// we don't want public API to give the impression that the compiler considers a metadata type to be a file-local type.
// Internally we can treat a metadata type as being a file-local type for EE.
// For public API, only source types are considered file-local types.
UnderlyingNamedTypeSymbol.OriginalDefinition is SourceMemberContainerTypeSymbol
&& UnderlyingNamedTypeSymbol.AssociatedFileIdentifier is not null;
&& UnderlyingNamedTypeSymbol.IsFileLocal;

INamedTypeSymbol INamedTypeSymbol.NativeIntegerUnderlyingType => UnderlyingNamedTypeSymbol.NativeIntegerUnderlyingType.GetPublicSymbol();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ public sealed override bool AreLocalsZeroed
get { throw ExceptionUtilities.Unreachable(); }
}

internal override bool IsFileLocal => _underlyingType.IsFileLocal;
internal override FileIdentifier? AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;

internal sealed override NamedTypeSymbol AsNativeInteger() => throw ExceptionUtilities.Unreachable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ internal override ManagedKind GetManagedKind(ref CompoundUseSiteInfo<AssemblySym

internal bool IsNew => HasFlag(DeclarationModifiers.New);

internal bool IsFileLocal => HasFlag(DeclarationModifiers.File);
internal sealed override bool IsFileLocal => HasFlag(DeclarationModifiers.File);

internal bool IsUnsafe => HasFlag(DeclarationModifiers.Unsafe);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,10 @@ private static void CheckMembers(NamespaceSymbol @namespace, Dictionary<string,
Array.Clear(memberOfArity, 0, memberOfArity.Length);
foreach (var symbol in result[name])
{
var nts = symbol as NamedTypeSymbol;
var nts = symbol as SourceMemberContainerTypeSymbol;
// It should be impossible to have a type member of a source namespace symbol which is not a SourceMemberContainerTypeSymbol
Debug.Assert((object)nts != null || symbol is not TypeSymbol);

var arity = ((object)nts != null) ? nts.Arity : 0;
if (arity >= memberOfArity.Length)
{
Expand Down Expand Up @@ -335,15 +338,19 @@ private static void CheckMembers(NamespaceSymbol @namespace, Dictionary<string,

if ((object)other != null)
{
// To decide whether type declarations are duplicates, we need to access members only available on the source original definition symbols.
Debug.Assert((object)nts.OriginalDefinition == nts && (object)other.OriginalDefinition == other);
Debug.Assert(other is not TypeSymbol || other is SourceMemberContainerTypeSymbol);

switch (nts, other)
{
case (SourceNamedTypeSymbol left, SourceNamedTypeSymbol right) when isFileLocalTypeInSeparateFileFrom(left, right) || isFileLocalTypeInSeparateFileFrom(right, left):
case ({ } left, SourceMemberContainerTypeSymbol right) when isFileLocalTypeInSeparateFileFrom(left, right) || isFileLocalTypeInSeparateFileFrom(right, left):
// no error
break;
case (SourceNamedTypeSymbol { IsFileLocal: true }, _) or (_, SourceNamedTypeSymbol { IsFileLocal: true }):
case ({ IsFileLocal: true }, _) or (_, SourceMemberContainerTypeSymbol { IsFileLocal: true }):
diagnostics.Add(ErrorCode.ERR_FileLocalDuplicateNameInNS, symbol.GetFirstLocationOrNone(), name, @namespace);
break;
case (SourceNamedTypeSymbol { IsPartial: true }, SourceNamedTypeSymbol { IsPartial: true }):
case ({ IsPartial: true }, SourceMemberContainerTypeSymbol { IsPartial: true }):
diagnostics.Add(ErrorCode.ERR_PartialTypeKindConflict, symbol.GetFirstLocationOrNone(), symbol);
break;
default:
Expand All @@ -366,7 +373,7 @@ private static void CheckMembers(NamespaceSymbol @namespace, Dictionary<string,
}
}

static bool isFileLocalTypeInSeparateFileFrom(SourceNamedTypeSymbol possibleFileLocalType, SourceNamedTypeSymbol otherSymbol)
static bool isFileLocalTypeInSeparateFileFrom(SourceMemberContainerTypeSymbol possibleFileLocalType, SourceMemberContainerTypeSymbol otherSymbol)
{
if (!possibleFileLocalType.IsFileLocal)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ internal override IEnumerable<CSharpAttributeData> GetCustomAttributesToEmit(PEM
throw ExceptionUtilities.Unreachable();
}

internal sealed override bool IsFileLocal => _underlyingType.IsFileLocal;
internal sealed override FileIdentifier? AssociatedFileIdentifier => _underlyingType.AssociatedFileIdentifier;

internal sealed override NamedTypeSymbol AsNativeInteger() => throw ExceptionUtilities.Unreachable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ internal override IEnumerable<FieldSymbol> GetFieldsToEmit()
internal override bool MangleName => Arity > 0;

#nullable enable
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;
#nullable disable

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ internal abstract class SynthesizedEmbeddedAttributeSymbolBase : NamedTypeSymbol

internal override bool MangleName => false;

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

internal override bool HasCodeAnalysisEmbeddedAttribute => true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,7 @@ public static bool IsPartial(this TypeSymbol type)

public static bool HasFileLocalTypes(this TypeSymbol type)
{
var foundType = type.VisitType(predicate: (type, _, _) => type is SourceMemberContainerTypeSymbol { IsFileLocal: true }, arg: (object?)null);
var foundType = type.VisitType(predicate: (type, _, _) => type is NamedTypeSymbol { IsFileLocal: true }, arg: (object?)null);
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
return foundType is not null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ internal override bool MangleName
}
}

internal override bool IsFileLocal => false;
internal override FileIdentifier? AssociatedFileIdentifier => null;

internal override DiagnosticInfo ErrorInfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal override bool MangleName
}
}

internal override bool IsFileLocal => false;
internal override FileIdentifier? AssociatedFileIdentifier => null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ internal override bool MangleName
}
}

internal override FileIdentifier? AssociatedFileIdentifier => null;
internal sealed override bool IsFileLocal => false;
internal sealed override FileIdentifier? AssociatedFileIdentifier => null;

public override ImmutableArray<TypeParameterSymbol> TypeParameters
{
Expand Down
Loading
Loading