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.

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

Jump to bottom

Remove support for base access with explicit base type. #35456

Merged
merged 2 commits into from
May 3, 2019
Merged

Remove support for base access with explicit base type. #35456

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
b7ee9c8
Remove support for base access with explicit base type.
AlekseyTs May 2, 2019
c08420e
More adjustments for tests.
AlekseyTs May 3, 2019
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
61 changes: 3 additions & 58 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,65 +1074,10 @@ private bool IsBadBaseAccess(SyntaxNode node, BoundExpression receiverOpt, Symbo
Debug.Assert(member.Kind != SymbolKind.Property);
Debug.Assert(member.Kind != SymbolKind.Event);

if (receiverOpt?.Kind == BoundKind.BaseReference)
if (receiverOpt?.Kind == BoundKind.BaseReference && member.IsAbstract)
{
var baseReference = (BoundBaseReference)receiverOpt;

if (baseReference.ExplicitBaseReferenceOpt != null)
{
if (baseReference.HasErrors)
{
return true;
}

TypeSymbol baseType = baseReference.ExplicitBaseReferenceOpt.Type;

if (baseType.IsInterfaceType() && (member as MethodSymbol)?.IsImplementable() == true)
{
MultiDictionary<Symbol, Symbol>.ValueSet set = TypeSymbol.FindImplementationInInterface(member, (NamedTypeSymbol)baseType);

if (set.Count == 1)
{
member = set.Single();

HashSet<DiagnosticInfo> useSiteDiagnostics = null;
if (!IsAccessible(member, ref useSiteDiagnostics, accessThroughType: null))
{
diagnostics.Add(node, useSiteDiagnostics);
Error(diagnostics, ErrorCode.ERR_BadAccess, node, member);
return true;
}
else if (member.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, member);
return true;
}
}
else if (!member.ContainingType.Equals(baseType, TypeCompareKind.AllIgnoreOptions))
{
Error(diagnostics, ErrorCode.ERR_NotImplementedInBase, node, member, baseType);
return true;
}
else
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, member);
return true;
}

return false;
}
else if (!member.ContainingType.Equals(baseType, TypeCompareKind.AllIgnoreOptions))
{
Error(diagnostics, ErrorCode.ERR_NotDeclaredInBase, node, member, baseType);
return true;
}
}

if (member.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, propertyOrEventSymbolOpt ?? member);
return true;
}
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, propertyOrEventSymbolOpt ?? member);
return true;
}

return false;
Expand Down
34 changes: 4 additions & 30 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1887,36 +1887,10 @@ private bool IsRefOrOutThisParameterCaptured(SyntaxNodeOrToken thisOrBaseToken,

private BoundBaseReference BindBase(BaseExpressionSyntax node, DiagnosticBag diagnostics)
{
TypeSymbol baseType;
BoundTypeExpression boundType = null;
bool hasErrors = false;

if (node.TypeClause is null)
{
baseType = this.ContainingType is null ? null : this.ContainingType.BaseTypeNoUseSiteDiagnostics;
}
else
{
baseType = this.BindType(node.TypeClause.BaseType, diagnostics, out AliasSymbol alias).Type;
hasErrors = baseType.IsErrorType();

if (!hasErrors && !(this.ContainingType is null))
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
if (!this.ContainingType.IsDerivedFrom(baseType, TypeCompareKind.ConsiderEverything, ref useSiteDiagnostics) &&
!(this.ContainingType.IsInterface && baseType.IsObjectType()) &&
!this.ContainingType.ImplementsInterface(baseType, ref useSiteDiagnostics))
{
Error(diagnostics, ErrorCode.ERR_NotBaseOrImplementedInterface, node.TypeClause.BaseType, baseType, this.ContainingType);
diagnostics.Add(node.TypeClause.BaseType, useSiteDiagnostics);
hasErrors = true;
}
}

boundType = new BoundTypeExpression(node.TypeClause.BaseType, alias, baseType, hasErrors);
}

TypeSymbol baseType = this.ContainingType is null ? null : this.ContainingType.BaseTypeNoUseSiteDiagnostics;
bool inStaticContext;
Copy link
Member

Choose a reason for hiding this comment

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

inStaticContext [](start = 17, length = 15)

nit: consider inlining declaration


if (!HasThis(isExplicit: true, inStaticContext: out inStaticContext))
{
//this error is returned in the field initializer case
Expand All @@ -1939,7 +1913,7 @@ private BoundBaseReference BindBase(BaseExpressionSyntax node, DiagnosticBag dia
hasErrors = true;
}

return new BoundBaseReference(node, boundType, baseType, hasErrors);
return new BoundBaseReference(node, baseType, hasErrors);
}

private BoundExpression BindCast(CastExpressionSyntax node, DiagnosticBag diagnostics)
Expand Down Expand Up @@ -6602,7 +6576,7 @@ private void CheckRuntimeSupportForSymbolAccess(SyntaxNode node, BoundExpression
if (symbol.ContainingType?.IsInterface == true && !Compilation.Assembly.RuntimeSupportsDefaultInterfaceImplementation && Compilation.SourceModule != symbol.ContainingModule)
{
if (!symbol.IsStatic && !(symbol is TypeSymbol) &&
(!symbol.IsImplementableInterfaceMember() || (receiverOpt as BoundBaseReference)?.ExplicitBaseReferenceOpt?.Type.IsInterfaceType() == true))
!symbol.IsImplementableInterfaceMember())
{
Error(diagnostics, ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, node);
}
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,6 @@
<Node Name="BoundBaseReference" Base="BoundExpression">
<!-- Type is not significant for this node type; always null -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="allow"/>
<Field Name="ExplicitBaseReferenceOpt" Type="BoundTypeExpression" Null="allow"/>
</Node>

<Node Name="BoundLocal" Base="BoundExpression">
Expand Down
36 changes: 0 additions & 36 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
View file
Open in desktop

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

12 changes: 0 additions & 12 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5835,9 +5835,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_DefaultInterfaceImplementation" xml:space="preserve">
<value>default interface implementation</value>
</data>
<data name="IDS_BaseTypeInBaseExpression" xml:space="preserve">
<value>specifying base type in base expression</value>
</data>
<data name="ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation" xml:space="preserve">
<value>Target runtime doesn't support default interface implementation.</value>
</data>
Expand All @@ -5856,18 +5853,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_LanguageVersionDoesNotSupportDefaultInterfaceImplementationForMember" xml:space="preserve">
<value>'{0}' cannot implement interface member '{1}' in type '{2}' because feature '{3}' is not available in C# {4}. Please use language version '{5}' or greater.</value>
</data>
<data name="ERR_NotBaseOrImplementedInterface" xml:space="preserve">
<value>'{0}' is not base type or interface of {1}.</value>
</data>
<data name="ERR_RuntimeDoesNotSupportProtectedAccessForInterfaceMember" xml:space="preserve">
<value>Target runtime doesn't support 'protected', 'protected internal', or 'private protected' accessibility for a member of an interface.</value>
</data>
<data name="ERR_NotImplementedInBase" xml:space="preserve">
<value>'{0}' is not implemented in base interface {1}.</value>
</data>
<data name="ERR_NotDeclaredInBase" xml:space="preserve">
<value>'{0}' is not an immediate member of {1}.</value>
</data>
<data name="ERR_DefaultInterfaceImplementationInNoPIAType" xml:space="preserve">
<value>Type '{0}' cannot be embedded because it has a non-abstract member. Consider setting the 'Embed Interop Types' property to false.</value>
</data>
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,6 @@ internal static BoundBlock ConstructDestructorBody(MethodSymbol method, BoundBlo
syntax,
new BoundBaseReference(
syntax,
explicitBaseReferenceOpt: null,
method.ContainingType)
{ WasCompilerGenerated = true },
baseTypeFinalize))
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1713,9 +1713,9 @@ internal enum ErrorCode
ERR_LanguageVersionDoesNotSupportDefaultInterfaceImplementationForMember = 8706,

ERR_RuntimeDoesNotSupportProtectedAccessForInterfaceMember = 8707,
ERR_NotBaseOrImplementedInterface = 8708,
ERR_NotImplementedInBase = 8709,
ERR_NotDeclaredInBase = 8710,
//ERR_NotBaseOrImplementedInterface = 8708,
//ERR_NotImplementedInBase = 8709,
//ERR_NotDeclaredInBase = 8710,

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 it may be better to just delete these, since it will likely be inappropriate to include these diagnostics in the C# 8.0 range of codes if it is introduced in a later language version.

Copy link
Member

Choose a reason for hiding this comment

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

One possible reason to keep these error codes is to make it easier to revert this PR later on (to re-introduce the feature). We can reserve those error codes.

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

ERR_DefaultInterfaceImplementationInNoPIAType = 8711,

Expand Down
6 changes: 2 additions & 4 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,8 @@ internal enum MessageID
IDS_FeatureObsoleteOnPropertyAccessor = MessageBase + 12758,
IDS_FeatureReadOnlyMembers = MessageBase + 12759,
IDS_DefaultInterfaceImplementation = MessageBase + 12760,
IDS_BaseTypeInBaseExpression = MessageBase + 12761,
IDS_OverrideWithConstraints = MessageBase + 12762,
IDS_FeatureNestedStackalloc = MessageBase + 12763,
IDS_OverrideWithConstraints = MessageBase + 12761,
IDS_FeatureNestedStackalloc = MessageBase + 12762,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -288,7 +287,6 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureObsoleteOnPropertyAccessor:
case MessageID.IDS_FeatureReadOnlyMembers:
case MessageID.IDS_DefaultInterfaceImplementation: // semantic check
case MessageID.IDS_BaseTypeInBaseExpression:
case MessageID.IDS_OverrideWithConstraints: // semantic check
case MessageID.IDS_FeatureNestedStackalloc: // semantic check
return LanguageVersion.CSharp8;
Expand Down
43 changes: 17 additions & 26 deletions src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs
View file
Open in desktop

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

Loading