Skip to content

Add initial support for specifying explicit base type in the base access. #33281

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

Merged
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
31 changes: 28 additions & 3 deletions docs/features/DefaultInterfaceImplementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,41 @@ class Test1 : I1

- Declaring types within interfaces. **Protected** and **protected internal** accessibility is not supported.
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

accessibility is not supported [](start = 78, length = 30)

These constraints are not correct. #Closed


- Implementing interface methods in derived interfaces by using explicit implementation syntax, accessibility is private, allowed modifiers: **extern** and **async**.
- Implementing interface methods in derived interfaces by using explicit implementation syntax, accessibility matches accessibility of the implemented member, allowed modifiers: **extern** and **async**.
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

matches accessibility of the implemented member [](start = 110, length = 47)

This doesn't ensure that the member is accessible to the caller. For example, an internal implementation in one assembly can be implemented in another assembly with internals-visible-to access to the first. A third assembly with internal-visible-to the original interface but not the override's assembly would not have access to that override. To ensure the override is accessible to the caller (and nowhere else), explicit override methods should be given protected access. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't ensure that the member is accessible to the caller.

I am not claiming that it is.

To ensure the override is accessible to the caller (and nowhere else), explicit override methods should be given protected access.

  1. In general, giving protected access would be incorrect because that might expose otherwise inaccessible types.
  2. So far, we disallow declaring protected members in interfaces and haven't defined what protected access means for interface members both for the language and for the runtime.

That is why I am starting with the same accessibility and we will refine this once we discuss design issues at LDM


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

Copy link
Member

Choose a reason for hiding this comment

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

Can you please explain how declaring a protected member in IL (that has an unutterable name) could "expose" an inaccesible type? I'm assuming we would not change the accessibility of types just because we emit symbols that use them.


In reply to: 255762670 [](ancestors = 255762670,255757955)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please explain how declaring a protected member in IL (that has an unutterable name) could "expose" an inaccesible type? I'm assuming we would not change the accessibility of types just because we emit symbols that use them.

The name doesn't matter in my opinion. Original member declaration can be internal and, therefore, refer to internal types. It would be illegal to use those types in a protected member.


In reply to: 255770151 [](ancestors = 255770151,255762670,255757955)

Copy link
Contributor Author

@AlekseyTs AlekseyTs Feb 12, 2019

Choose a reason for hiding this comment

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

It is possible that protected access would work, if we knew what that means. As I said, this is a topic for an LDM.


In reply to: 255770872 [](ancestors = 255770872,255770151,255762670,255757955)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this PR updates the speclet and the implementation to match each other, though it produces cases that require that we generate illegal IL.

Could you elaborate, give an example please?


In reply to: 256257520 [](ancestors = 256257520,256257340,256177815,255803908,255803140,255772556,255770872,255770151,255762670,255757955)

Copy link
Member

@gafter gafter Feb 13, 2019

Choose a reason for hiding this comment

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

Yes, this PR updates the speclet and the implementation to match each other, though it produces cases that require that we generate illegal IL.

Could you elaborate, give an example please?

An example is

// assembly1
[InternalsVisibleTo("assembly2")]
[InternalsVisibleTo("assembly3")]
public interface I1
{
    internal void M() {}
}
// assembly2
public interface I2 : i1
{
    void I1.M() {}
}
// assembly3
public interface I3: I1, I2
{
    void I1.M()
    {
        // this call must emit an invocation of I2.M
        // but that is not CLR-accessible in the speclet's translation scheme
        base(I2).M();
    }
}

In reply to: 256265573 [](ancestors = 256265573,256257520,256257340,256177815,255803908,255803140,255772556,255770872,255770151,255762670,255757955)

Copy link
Contributor Author

@AlekseyTs AlekseyTs Feb 13, 2019

Choose a reason for hiding this comment

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

An example is
...

 // this call must emit an invocation of I2.M
 // but that is not CLR-accessible in the speclet's translation scheme
  base(I2).M();

I think this is misinterpretation of the speclet and misinterpretation of the implementation:

  • The speclet says: "The most specific implementation must be accessible at the call site." This is a requirement for the base access that should be met, not a guarantee.
  • According to the above, compiler shouldn't emit a call to the inaccessible method in the example, but should report an error. Unit-test ExplicitBase_152 clearly demonstrates this behavior.

Do you still believe that the speclet "produces cases that require that we generate illegal IL"?


In reply to: 256314854 [](ancestors = 256314854,256265573,256257520,256257340,256177815,255803908,255803140,255772556,255770872,255770151,255762670,255757955)

Copy link
Member

Choose a reason for hiding this comment

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

Do you still believe that the speclet "produces cases that require that we generate illegal IL"?

No, I now believe that the speclet describes an implementation that fails to implement the language specification. The only accessibility requirement on a member accessed through an explicit base is the existing spec constraint that a member found through lookup is accessible. In this case the member found (the original interface definition) is indeed accessible. The constraint "The most specific implementation must be accessible at the call site." is one that doesn't appear in the language specification. It wouldn't even make sense there, as explicit overrides do not have accessibility in the language spec and are not found during name lookup.

In short, unit test ExplicitBase_152 has errors that are not supported by the language.


In reply to: 256494259 [](ancestors = 256494259,256314854,256265573,256257520,256257340,256177815,255803908,255803140,255772556,255770872,255770151,255762670,255757955)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am glad this is resolved now. And thank you for sharing your thoughts about the feature design.


In reply to: 256640387 [](ancestors = 256640387,256494259,256314854,256265573,256257520,256257340,256177815,255803908,255803140,255772556,255770872,255770151,255762670,255757955)


- Implementing interface properties and indexers in derived interfaces by using explicit implementation syntax, accessibility is private, allowed modifiers: **extern**.
- Implementing interface properties and indexers in derived interfaces by using explicit implementation syntax, accessibility matches accessibility of the implemented member, allowed modifiers: **extern**.

- Implementing interface events in derived interfaces by using explicit implementation syntax, accessibility is private, no allowed modifiers.
- Implementing interface events in derived interfaces by using explicit implementation syntax, accessibility matches accessibility of the implemented member, no allowed modifiers.

- Declaring static fields, auto-properties and field-like events (**protected** modifier is not allowed).
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

protected modifier is not allowed [](start = 66, length = 37)

This constraint is not correct. Just as protected static fields are permitted in classes, if fields et al are permitted at all, then protected versions of them should be permitted in interfaces. However the current spec forbids these categories of symbols. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This constraint is not correct. Just as protected static fields are permitted in classes, if fields et al are permitted at all, then protected versions of them should be permitted in interfaces. However the current spec forbids these categories of symbols.

So far, we didn't define what protected access means for interface members. This document accurately reflects that. We can revisit this at LDM.


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


- Declaring operators ```+ - ! ~ ++ -- true false * / % & | ^ << >> > < >= <=``` in interfaces.

- Base access
The following forms of base-access are added (https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-11-14.md)
```
base ( <type-syntax> ) . identifier
base ( <type-syntax> ) [ argument-list ]
```

The type-syntax can refer to one of the base classes of the containing type, or one of the interfaces implemented or inherited by the containing type.

When the type-syntax refers to a class, the member lookup rules, overload resolution rules and IL emit match the rules for the 7.3 supported
forms of base-access. The only difference is the specified base class is used instead of the immediate base class, the most derived implementation is
found in that class, etc.

When the type-syntax refers to an interface:
1. The member lookup is performed in that interface, using the regular member lookup rules within interfaces, with an exception that members of
System.Object do not participate in the lookup.
2. Regular overload resolution is performed for members returned by the lookup process, virtual or abstract members are not replaced with most
derived implementations at this step (unlike the case when the type-syntax refers to a class). If result of overload resolution is a virtual
or abstract method, it must have a most specific implementation within the interface type, an error is reported otherwise. The most specific
implementation must be accessible at the call site.
3. During IL emit a **call** (non-virtual call) instruction is used to invoke methods. If result of overload resolution on the previous step is
virtual or abstract method, the most specific implementation of the method is used as the target for the instruction.

Given the accessibility requirements for the most specific interface implementation, accessibility of implementations provided in derived interfaces
is changed to match accessibility of implemented members, prior to this change the accessibility was private.

**Open issues and work items** are tracked in https://github.com/dotnet/roslyn/issues/17952.

Expand Down
69 changes: 48 additions & 21 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -773,10 +773,17 @@ private bool CheckEventValueKind(BoundEventAccess boundEvent, BindValueKind valu

if (receiver?.Kind == BoundKind.BaseReference && eventSymbol.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, boundEvent.Syntax, eventSymbol);
return false;
var baseReference = (BoundBaseReference)receiver;

if (!eventSymbol.IsImplementableInterfaceMember() ||
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

IsImplementableInterfaceMember [](start = 37, length = 30)

Just as you can use a base invocation to a non-virtual member in a class, base invocations to an interface do not require that the symbol found is virtual or abstract. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just as you can use a base invocation to a non-virtual member in a class, base invocations to an interface do not require that the symbol found is virtual or abstract.

Completely agree and tests confirm that.


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

baseReference.ExplicitBaseReferenceOpt?.Type.IsInterfaceType() != true)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, boundEvent.Syntax, eventSymbol);
return false;
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

I don't think this error is triggered (though it should be) for an abstract event in an interface that is base(I) invoked from a derived interface. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this error is triggered (though it should be) for an abstract event in an interface that is base(I) invoked from a derived interface.

This code path is for legacy scenarios, not when an abstract event is in an interface. Interface case is handled in BindEventAssignment, can be observed in unit-tests ExplicitBase_122 and ExplicitBase_145, for example.


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

}
}
else if (ReportUseSiteDiagnostics(eventSymbol, diagnostics, eventSyntax))

if (ReportUseSiteDiagnostics(eventSymbol, diagnostics, eventSyntax))
{
// NOTE: BindEventAssignment checks use site errors on the specific accessor
// (since we don't know which is being used).
Expand Down Expand Up @@ -952,15 +959,6 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
return false;
}
}
else if (receiver?.Kind == BoundKind.BaseReference && setMethod.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, propertySymbol);
return false;
}
else if (!object.Equals(setMethod.GetUseSiteDiagnostic(), propertySymbol.GetUseSiteDiagnostic()) && ReportUseSiteDiagnostics(setMethod, diagnostics, propertySyntax))
{
return false;
}
else
{
var accessThroughType = this.GetAccessThroughType(receiver);
Expand Down Expand Up @@ -988,6 +986,12 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
{
return false;
}

if (IsBadBaseAccess(node, receiver, setMethod, diagnostics, propertySymbol) ||
(!object.Equals(setMethod.GetUseSiteDiagnostic(), propertySymbol.GetUseSiteDiagnostic()) && ReportUseSiteDiagnostics(setMethod, diagnostics, propertySyntax)))
{
return false;
}
}
}

Expand All @@ -1001,15 +1005,6 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
Error(diagnostics, ErrorCode.ERR_PropertyLacksGet, node, propertySymbol);
return false;
}
else if (receiver?.Kind == BoundKind.BaseReference && getMethod.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, propertySymbol);
return false;
}
else if (!object.Equals(getMethod.GetUseSiteDiagnostic(), propertySymbol.GetUseSiteDiagnostic()) && ReportUseSiteDiagnostics(getMethod, diagnostics, propertySyntax))
{
return false;
}
else
{
var accessThroughType = this.GetAccessThroughType(receiver);
Expand All @@ -1032,6 +1027,12 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
}

ReportDiagnosticsIfObsolete(diagnostics, getMethod, node, receiver?.Kind == BoundKind.BaseReference);

if (IsBadBaseAccess(node, receiver, getMethod, diagnostics, propertySymbol) ||
(!object.Equals(getMethod.GetUseSiteDiagnostic(), propertySymbol.GetUseSiteDiagnostic()) && ReportUseSiteDiagnostics(getMethod, diagnostics, propertySyntax)))
{
return false;
}
}
}

Expand All @@ -1044,6 +1045,32 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
return true;
}

private bool IsBadBaseAccess(SyntaxNode node, BoundExpression receiverOpt, MethodSymbol method, DiagnosticBag diagnostics,
PropertySymbol propertySymbolOpt = null, bool checkOnlyAccessThroughInterface = false)
{
if (receiverOpt?.Kind == BoundKind.BaseReference)
{
var baseReference = (BoundBaseReference)receiverOpt;

if (method.IsImplementableInterfaceMember() &&
baseReference.ExplicitBaseReferenceOpt?.Type.IsInterfaceType() == true)
{
if (TypeSymbol.TryFindBaseImplementationInInterface((NamedTypeSymbol)baseReference.ExplicitBaseReferenceOpt.Type, method, diagnostics, node, this) is null)
{
Copy link
Member

@jaredpar jaredpar Feb 13, 2019

Choose a reason for hiding this comment

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

Is it worth asserting that diagnostics is non empty in this case? #Resolved

Debug.Assert(!diagnostics.IsEmptyWithoutResolution);
return true;
Copy link
Member

@gafter gafter Feb 12, 2019

Choose a reason for hiding this comment

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

return true [](start = 24, length = 11)

Should an error be reported before returning true? #Closed

Copy link
Member

Choose a reason for hiding this comment

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

I see that is done in TryFindBaseImplementationInInterface


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

}
}
else if (!checkOnlyAccessThroughInterface && method.IsAbstract)
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, propertySymbolOpt ?? (Symbol)method);
return true;
}
}

return false;
}

/// <summary>
/// Computes the scope to which the given invocation can escape
/// NOTE: the escape scope for ref and val escapes is the same for invocations except for trivial cases (ordinary type returned by val)
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ private BoundExpression CreateMethodGroupConversion(SyntaxNode syntax, BoundExpr
BoundExpression receiverOpt = group.ReceiverOpt;
MethodSymbol method = conversion.Method;
bool hasErrors = false;
if (receiverOpt != null && receiverOpt.Kind == BoundKind.BaseReference && method.IsAbstract)

if (IsBadBaseAccess(syntax, receiverOpt, method, diagnostics))
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, syntax, method);
hasErrors = true;
}

Expand Down Expand Up @@ -514,7 +514,7 @@ private BoundMethodGroup FixMethodGroupWithTypeOrValue(BoundMethodGroup group, C
/// </returns>
private bool MemberGroupFinalValidation(BoundExpression receiverOpt, MethodSymbol methodSymbol, SyntaxNode node, DiagnosticBag diagnostics, bool invokedAsExtensionMethod)
{
CheckRuntimeSupportForSymbolAccess(node, methodSymbol, diagnostics);
CheckRuntimeSupportForSymbolAccess(node, receiverOpt, methodSymbol, diagnostics);

if (MemberGroupFinalValidationAccessibilityChecks(receiverOpt, methodSymbol, node, diagnostics, invokedAsExtensionMethod))
{
Expand Down
70 changes: 49 additions & 21 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,7 @@ private BoundThisReference BindThis(ThisExpressionSyntax node, DiagnosticBag dia
}
else
{
hasErrors = IsRefOrOutThisParameterCaptured(node, diagnostics);
hasErrors = IsRefOrOutThisParameterCaptured(node.Token, diagnostics);
}

return ThisReference(node, this.ContainingType, hasErrors);
Expand All @@ -1848,14 +1848,14 @@ private BoundThisReference ThisReference(SyntaxNode node, NamedTypeSymbol thisTy
return new BoundThisReference(node, thisTypeOpt ?? CreateErrorType(), hasErrors) { WasCompilerGenerated = wasCompilerGenerated };
}

private bool IsRefOrOutThisParameterCaptured(SyntaxNode node, DiagnosticBag diagnostics)
private bool IsRefOrOutThisParameterCaptured(SyntaxNodeOrToken thisOrBaseToken, DiagnosticBag diagnostics)
{
ParameterSymbol thisSymbol = this.ContainingMemberOrLambda.EnclosingThisSymbol();
// If there is no this parameter, then it is definitely not captured and
// any diagnostic would be cascading.
if ((object)thisSymbol != null && thisSymbol.ContainingSymbol != ContainingMemberOrLambda && thisSymbol.RefKind != RefKind.None)
{
Error(diagnostics, ErrorCode.ERR_ThisStructNotInAnonMeth, node);
Error(diagnostics, ErrorCode.ERR_ThisStructNotInAnonMeth, thisOrBaseToken);
return true;
}

Expand All @@ -1864,33 +1864,58 @@ private bool IsRefOrOutThisParameterCaptured(SyntaxNode node, DiagnosticBag diag

private BoundBaseReference BindBase(BaseExpressionSyntax node, DiagnosticBag diagnostics)
{
NamedTypeSymbol baseType = (object)this.ContainingType == null ? null : this.ContainingType.BaseTypeNoUseSiteDiagnostics;
bool hasErrors = true;
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).TypeSymbol;
hasErrors = baseType.IsErrorType();

if (!hasErrors && !(this.ContainingType is null))
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
if (!this.ContainingType.IsDerivedFrom(baseType, TypeCompareKind.ConsiderEverything, ref useSiteDiagnostics) &&
!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);
}

bool inStaticContext;
if (!HasThis(isExplicit: true, inStaticContext: out inStaticContext))
{
//this error is returned in the field initializer case
Error(diagnostics, inStaticContext ? ErrorCode.ERR_BaseInStaticMeth : ErrorCode.ERR_BaseInBadContext, node);
Error(diagnostics, inStaticContext ? ErrorCode.ERR_BaseInStaticMeth : ErrorCode.ERR_BaseInBadContext, node.Token);
hasErrors = true;
}
else if ((object)baseType == null) // e.g. in System.Object
{
Error(diagnostics, ErrorCode.ERR_NoBaseClass, node);
hasErrors = true;
}
else if (node.Parent.Kind() != SyntaxKind.SimpleMemberAccessExpression && node.Parent.Kind() != SyntaxKind.ElementAccessExpression)
{
Error(diagnostics, ErrorCode.ERR_BaseIllegal, node);
}
else if (IsRefOrOutThisParameterCaptured(node, diagnostics))
else if (this.ContainingType is null || (node.Parent.Kind() != SyntaxKind.SimpleMemberAccessExpression && node.Parent.Kind() != SyntaxKind.ElementAccessExpression))
{
// error has been reported by CheckThisReference
Error(diagnostics, ErrorCode.ERR_BaseIllegal, node.Token);
hasErrors = true;
}
else
else if (IsRefOrOutThisParameterCaptured(node.Token, diagnostics))
{
hasErrors = false;
// error has been reported by IsRefOrOutThisParameterCaptured
hasErrors = true;
}

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

private BoundExpression BindCast(CastExpressionSyntax node, DiagnosticBag diagnostics)
Expand Down Expand Up @@ -5809,7 +5834,7 @@ private BoundExpression BindInstanceMemberAccess(
bool leftIsBaseReference = boundLeft.Kind == BoundKind.BaseReference;
if (leftIsBaseReference)
{
options |= LookupOptions.UseBaseReferenceAccessibility;
options |= (LookupOptions.UseBaseReferenceAccessibility | LookupOptions.NoObjectMembersOnInterfaces);
}

HashSet<DiagnosticInfo> useSiteDiagnostics = null;
Expand Down Expand Up @@ -6474,14 +6499,15 @@ private BoundExpression BindPropertyAccess(
WarnOnAccessOfOffDefault(node, receiver, diagnostics);
}

CheckRuntimeSupportForSymbolAccess(node, propertySymbol, diagnostics);
CheckRuntimeSupportForSymbolAccess(node, receiver, propertySymbol, diagnostics);

return new BoundPropertyAccess(node, receiver, propertySymbol, lookupResult, propertySymbol.Type.TypeSymbol, hasErrors: (hasErrors || hasError));
}

private void CheckRuntimeSupportForSymbolAccess(SyntaxNode node, Symbol symbol, DiagnosticBag diagnostics)
private void CheckRuntimeSupportForSymbolAccess(SyntaxNode node, BoundExpression receiverOpt, Symbol symbol, DiagnosticBag diagnostics)
{
if (!symbol.IsStatic && symbol.ContainingType.IsInterface && !symbol.IsImplementableInterfaceMember() &&
if (!symbol.IsStatic && symbol.ContainingType.IsInterface &&
(!symbol.IsImplementableInterfaceMember() || (receiverOpt as BoundBaseReference)?.ExplicitBaseReferenceOpt?.Type.IsInterfaceType() == true) &&
!Compilation.Assembly.RuntimeSupportsDefaultInterfaceImplementation)
{
Error(diagnostics, ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, node);
Expand All @@ -6507,7 +6533,7 @@ private BoundExpression BindEventAccess(
WarnOnAccessOfOffDefault(node, receiver, diagnostics);
}

CheckRuntimeSupportForSymbolAccess(node, eventSymbol, diagnostics);
CheckRuntimeSupportForSymbolAccess(node, receiver, eventSymbol, diagnostics);

return new BoundEventAccess(node, receiver, eventSymbol, isUsableAsField, lookupResult, eventSymbol.Type.TypeSymbol, hasErrors: (hasErrors || hasError));
}
Expand Down Expand Up @@ -7035,7 +7061,7 @@ private BoundExpression BindIndexerAccess(ExpressionSyntax node, BoundExpression
Debug.Assert(analyzedArguments != null);

LookupResult lookupResult = LookupResult.GetInstance();
LookupOptions lookupOptions = expr.Kind == BoundKind.BaseReference ? LookupOptions.UseBaseReferenceAccessibility : LookupOptions.Default;
LookupOptions lookupOptions = expr.Kind == BoundKind.BaseReference ? (LookupOptions.UseBaseReferenceAccessibility | LookupOptions.NoObjectMembersOnInterfaces) : LookupOptions.Default;
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
this.LookupMembersWithFallback(lookupResult, expr.Type, WellKnownMemberNames.Indexer, arity: 0, useSiteDiagnostics: ref useSiteDiagnostics, options: lookupOptions);
diagnostics.Add(node, useSiteDiagnostics);
Expand Down Expand Up @@ -7313,6 +7339,8 @@ private BoundExpression BindIndexerOrIndexedPropertyAccess(
diagnostics);
}

CheckRuntimeSupportForSymbolAccess(syntax, receiver, property, diagnostics);

propertyAccess = new BoundIndexerAccess(
syntax,
receiver,
Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1112,9 +1112,8 @@ private BoundCall BindInvocationExpressionContinued(
bool isDelegateCall = (object)delegateTypeOpt != null;
if (!isDelegateCall)
{
if ((object)receiver != null && receiver.Kind == BoundKind.BaseReference && method.IsAbstract)
if (IsBadBaseAccess(node, receiver, method, diagnostics))
{
Error(diagnostics, ErrorCode.ERR_AbstractBaseCall, node, method);
gotError = true;
}

Expand Down
Loading