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

Record-structs: Address some PROTOTYPE markers #52256

Merged
merged 8 commits into from
Apr 2, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ private static void ComputeDeclarations(

return;
}

// PROTOTYPE(record-structs): update for record structs
case SyntaxKind.RecordDeclaration:
{
if (associatedSymbol is IMethodSymbol ctor)
Expand All @@ -117,6 +115,20 @@ private static void ComputeDeclarations(
return;
}

goto case SyntaxKind.ClassDeclaration;
}
case SyntaxKind.RecordStructDeclaration:
{
if (associatedSymbol is IMethodSymbol ctor)
{
Copy link
Member Author

@jcouv jcouv Mar 30, 2021

Choose a reason for hiding this comment

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

📝 I didn't manage to cover this if and couldn't find coverage for above if either. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't manage to cover this if and couldn't find coverage for above if either.

Really? I acn easily hit code paths in the previous case by running existing test in src\Compilers\CSharp\Test\Semantic\Semantics\RecordTests.cs


In reply to: 604290986 [](ancestors = 604290986)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. You're right. Both if blocks are visited by the AnalyzerActions_XX tests in RecordTests and RecordStructTests.


In reply to: 605097075 [](ancestors = 605097075,604290986)

var recordDeclaration = (RecordStructDeclarationSyntax)node;
Debug.Assert(ctor.MethodKind == MethodKind.Constructor && recordDeclaration.ParameterList is object);

var codeBlocks = GetParameterListInitializersAndAttributes(recordDeclaration.ParameterList);
builder.Add(GetDeclarationInfo(node, associatedSymbol, codeBlocks));
return;
}

goto case SyntaxKind.ClassDeclaration;
}
case SyntaxKind.ClassDeclaration:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ internal override IOperation GetOperationWorker(CSharpSyntaxNode node, Cancellat
case AccessorDeclarationSyntax accessor:
model = (accessor.Body != null || accessor.ExpressionBody != null) ? GetOrAddModel(node) : null;
break;
// PROTOTYPE(record-structs): update for record structs
case RecordDeclarationSyntax { ParameterList: { }, PrimaryConstructorBaseType: { } } recordDeclaration when TryGetSynthesizedRecordConstructor(recordDeclaration) is SynthesizedRecordConstructor:
model = GetOrAddModel(recordDeclaration);
break;
Expand Down Expand Up @@ -805,7 +804,6 @@ private MemberSemanticModel GetMemberModel(int position)
!LookupPosition.IsInConstructorParameterScope(position, constructorDecl) &&
!LookupPosition.IsInParameterList(position, constructorDecl);
break;
// PROTOTYPE(record-structs): update for record structs
case SyntaxKind.RecordDeclaration:
{
var recordDecl = (RecordDeclarationSyntax)memberDecl;
Expand Down Expand Up @@ -875,7 +873,6 @@ internal override MemberSemanticModel GetMemberModel(SyntaxNode node)
GetOrAddModel(constructorDecl) : null;
}

// PROTOTYPE(record-structs): update for record structs
case SyntaxKind.RecordDeclaration:
{
var recordDecl = (RecordDeclarationSyntax)memberDecl;
Expand Down Expand Up @@ -1094,7 +1091,6 @@ private MemberSemanticModel CreateMemberModel(CSharpSyntaxNode node)
return createMethodBodySemanticModel(memberDecl, symbol);
}

// PROTOTYPE(record-structs): update for record structs
case SyntaxKind.RecordDeclaration:
{
SynthesizedRecordConstructor symbol = TryGetSynthesizedRecordConstructor((RecordDeclarationSyntax)node);
Expand Down Expand Up @@ -1259,8 +1255,9 @@ MemberSemanticModel createMethodBodySemanticModel(CSharpSyntaxNode memberDecl, S
}
}

private SynthesizedRecordConstructor TryGetSynthesizedRecordConstructor(RecordDeclarationSyntax node)
private SynthesizedRecordConstructor TryGetSynthesizedRecordConstructor(TypeDeclarationSyntax node)
{
Debug.Assert(node is RecordDeclarationSyntax or RecordStructDeclarationSyntax);
NamedTypeSymbol recordType = GetDeclaredType(node);
var symbol = recordType.GetMembersUnordered().OfType<SynthesizedRecordConstructor>().SingleOrDefault();

Expand Down Expand Up @@ -2026,11 +2023,14 @@ private ParameterSymbol GetMethodParameterSymbol(

MethodSymbol method;

// PROTOTYPE(record-structs): update for record structs
if (memberDecl is RecordDeclarationSyntax recordDecl && recordDecl.ParameterList == paramList)
{
method = TryGetSynthesizedRecordConstructor(recordDecl);
}
else if (memberDecl is RecordStructDeclarationSyntax recordStructDecl && recordStructDecl.ParameterList == paramList)
{
method = TryGetSynthesizedRecordConstructor(recordStructDecl);
}
else
{
method = (GetDeclaredSymbol(memberDecl, cancellationToken) as IMethodSymbol).GetSymbol();
Expand Down Expand Up @@ -2374,7 +2374,6 @@ internal override Symbol RemapSymbolIfNecessaryCore(Symbol symbol)

internal override Func<SyntaxNode, bool> GetSyntaxNodesToAnalyzeFilter(SyntaxNode declaredNode, ISymbol declaredSymbol)
{
// PROTOTYPE(record-structs): update for record structs
switch (declaredNode)
{
case CompilationUnitSyntax unit when SimpleProgramNamedTypeSymbol.GetSimpleProgramEntryPoint(Compilation, unit, fallbackToMainEntryPoint: false) is SynthesizedSimpleProgramEntryPointSymbol entryPoint:
Expand Down Expand Up @@ -2436,6 +2435,33 @@ internal override Func<SyntaxNode, bool> GetSyntaxNodesToAnalyzeFilter(SyntaxNod
}
break;

case RecordStructDeclarationSyntax recordStructDeclaration when TryGetSynthesizedRecordConstructor(recordStructDeclaration) is SynthesizedRecordConstructor ctor:
switch (declaredSymbol.Kind)
{
case SymbolKind.Method:
Debug.Assert((object)declaredSymbol.GetSymbol() == (object)ctor);
return (node) =>
{
// Accept only nodes that either match, or above/below of a 'parameter list'.
if (node.Parent == recordStructDeclaration)
{
return node == recordStructDeclaration.ParameterList;
}
return true;
};

case SymbolKind.NamedType:
Debug.Assert((object)declaredSymbol.GetSymbol() == (object)ctor.ContainingSymbol);
// Accept nodes that do not match a 'parameter list'.
return (node) => node != recordStructDeclaration.ParameterList;

default:
ExceptionUtilities.UnexpectedValue(declaredSymbol.Kind);
break;
}
break;

case PrimaryConstructorBaseTypeSyntax { Parent: BaseListSyntax { Parent: RecordDeclarationSyntax recordDeclaration } } baseType
when recordDeclaration.PrimaryConstructorBaseType == declaredNode && TryGetSynthesizedRecordConstructor(recordDeclaration) is SynthesizedRecordConstructor ctor:
if ((object)declaredSymbol.GetSymbol() == (object)ctor)
Expand All @@ -2448,6 +2474,10 @@ internal override Func<SyntaxNode, bool> GetSyntaxNodesToAnalyzeFilter(SyntaxNod
case ParameterSyntax param when declaredSymbol.Kind == SymbolKind.Property && param.Parent?.Parent is RecordDeclarationSyntax recordDeclaration && recordDeclaration.ParameterList == param.Parent:
Debug.Assert(declaredSymbol.GetSymbol() is SynthesizedRecordPropertySymbol);
return (node) => false;

case ParameterSyntax param when declaredSymbol.Kind == SymbolKind.Property && param.Parent?.Parent is RecordStructDeclarationSyntax recordStructDeclaration && recordStructDeclaration.ParameterList == param.Parent:
Debug.Assert(declaredSymbol.GetSymbol() is SynthesizedRecordPropertySymbol);
return (node) => false;
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ internal enum DeclarationKind : byte
Submission,
ImplicitClass,
SimpleProgram,
// PROTOTYPE(record-structs): rename to RecordClass?
Record,
RecordStruct
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ internal override bool IsRecord
}
}

// PROTOTYPE(record-structs): update for record structs (is there a way to recognize a record struct from PE?)
// Record structs get erased when emitted to metadata
internal override bool IsRecordStruct => false;

public override Accessibility DeclaredAccessibility
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,6 @@ ImmutableArray<SymbolDisplayPart> ITypeSymbol.ToMinimalDisplayParts(SemanticMode

bool ITypeSymbol.IsReadOnly => UnderlyingTypeSymbol.IsReadOnly;

bool ITypeSymbol.IsRecord => UnderlyingTypeSymbol.IsRecord;
bool ITypeSymbol.IsRecord => UnderlyingTypeSymbol.IsRecord || UnderlyingTypeSymbol.IsRecordStruct;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,6 @@ public sealed override ImmutableArray<Symbol> GetMembers(string name)
return ImmutableArray<Symbol>.Empty;
}

// PROTOTYPE(record-structs): update for record structs
/// <remarks>
/// For source symbols, there can only be a valid clone method if this is a record, which is a
/// simple syntax check. This will need to change when we generalize cloning, but it's a good
Expand Down Expand Up @@ -1512,7 +1511,6 @@ internal void AssertMemberExposure(Symbol member, bool forDiagnostics = false)
var declared = Volatile.Read(ref _lazyDeclaredMembersAndInitializers);
Debug.Assert(declared != DeclaredMembersAndInitializers.UninitializedSentinel);

// PROTOTYPE(record-structs): update for record structs
if ((declared is object && (declared.NonTypeMembers.Contains(m => m == (object)member) || declared.RecordPrimaryConstructor == (object)member)) ||
Volatile.Read(ref _lazyMembersAndInitializers)?.NonTypeMembers.Contains(m => m == (object)member) == true)
{
Expand Down Expand Up @@ -2969,7 +2967,8 @@ private void AddDeclaredNontypeMembers(DeclaredMembersAndInitializersBuilder bui
noteRecordParameters(recordStructDecl, parameterList, builder, diagnostics);
AddNonTypeMembers(builder, recordStructDecl.Members, diagnostics);

// PROTOTYPE(record-structs): we will allow declaring parameterless constructors
// We will allow declaring parameterless constructors
// Tracking issue https://github.com/dotnet/roslyn/issues/52240
if (parameterList?.ParameterCount == 0)
{
diagnostics.Add(ErrorCode.ERR_StructsCantContainDefaultConstructor, parameterList.Location);
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2253,7 +2253,6 @@ ITypeSymbol ITypeSymbolInternal.GetITypeSymbol()
return GetITypeSymbol(DefaultNullableAnnotation);
}

// PROTOTYPE(record-structs): rename to IsRecordClass?
Copy link
Member Author

@jcouv jcouv Mar 30, 2021

Choose a reason for hiding this comment

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

📝 not planning to rename the internal API #Resolved

internal abstract bool IsRecord { get; }

internal abstract bool IsRecordStruct { get; }
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Syntax/SyntaxKindFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,6 @@ public static SyntaxKind GetTypeDeclarationKind(SyntaxKind kind)
case SyntaxKind.InterfaceKeyword:
return SyntaxKind.InterfaceDeclaration;
case SyntaxKind.RecordKeyword:
// PROTOTYPE(record-structs): anything we can do?
Copy link
Member Author

@jcouv jcouv Mar 30, 2021

Choose a reason for hiding this comment

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

📝 I don't see anything we could do given the shape of the API. We'll see if that causes problems during IDE work #Resolved

return SyntaxKind.RecordDeclaration;
default:
return SyntaxKind.None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ private static SyntaxKind GetTypeDeclarationKeywordKind(SyntaxKind kind)
return SyntaxKind.InterfaceKeyword;
case SyntaxKind.RecordDeclaration:
return SyntaxKind.RecordKeyword;
// PROTOTYPE(record-structs): update for record structs
Copy link
Member Author

@jcouv jcouv Mar 30, 2021

Choose a reason for hiding this comment

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

📝 We'll see if something is needed here during IDE work #Resolved

default:
throw ExceptionUtilities.UnexpectedValue(kind);
}
Expand Down
Loading