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

Support public, internal, private, static, virtual, sealed, abstract, and extern modifiers with interface events. #19080

Merged
merged 1 commit into from
May 1, 2017
+3,438 −260
Merged

Support public, internal, private, static, virtual, sealed, abstract, and extern modifiers with interface events. #19080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions docs/features/DefaultInterfaceImplementation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@ class Test1 : I1

- Using **internal** and **private** modifiers with interface property/indexer accessors.

- Using **public**, **internal**, **private**, **static**, **virtual**, **sealed**, **abstract** and **extern** modifiers with interface events.

**Open issues and work items** are tracked in https://github.com/dotnet/roslyn/issues/17952.
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1511,14 +1511,11 @@ private static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationSta
return null;
}

var compilation = method.DeclaringCompilation;
var factory = compilation.GetBinderFactory(sourceMethod.SyntaxTree);

var bodySyntax = sourceMethod.BodySyntax;

if (bodySyntax != null)
{
var inMethodBinder = factory.GetBinder(bodySyntax);
var inMethodBinder = method.DeclaringCompilation.GetBinderFactory(sourceMethod.SyntaxTree).GetBinder(bodySyntax);
var binder = new ExecutableCodeBinder(bodySyntax, sourceMethod, inMethodBinder);
importChain = binder.ImportChain;

Expand Down
22 changes: 11 additions & 11 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceCustomEventAccessorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,17 @@ internal SourceCustomEventAccessorSymbol(
isExtensionMethod: false,
isMetadataVirtualIgnoringModifiers: explicitInterfaceImplementations.Any());

var bodyOpt = syntax.Body;
if (bodyOpt != null)
if (@event.ContainingType.IsInterface)
{
Binder.CheckFeatureAvailability(syntax, MessageID.IDS_DefaultInterfaceImplementation, diagnostics, this.Location);

if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, this.Location);
}
}

if (syntax.Body != null || syntax.ExpressionBody != null)
{
if (IsExtern && !IsAbstract)
{
Expand All @@ -73,15 +82,6 @@ internal SourceCustomEventAccessorSymbol(
{
diagnostics.Add(ErrorCode.ERR_AbstractHasBody, this.Location, this);
}
else if (@event.ContainingType.IsInterface)
{
Binder.CheckFeatureAvailability(syntax, MessageID.IDS_DefaultInterfaceImplementation, diagnostics, this.Location);

if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, this.Location);
}
}
// Do not report error for IsAbstract && IsExtern. Dev10 reports CS0180 only
// in that case ("member cannot be both extern and abstract").
}
Expand Down
36 changes: 28 additions & 8 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,23 +405,40 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool expli
{
bool isInterface = this.ContainingType.IsInterface;
var defaultAccess = isInterface ? DeclarationModifiers.Public : DeclarationModifiers.Private;
var defaultInterfaceImplementationModifiers = DeclarationModifiers.None;

// Check that the set of modifiers is allowed
var allowedModifiers = DeclarationModifiers.Unsafe;
if (!explicitInterfaceImplementation)
{
allowedModifiers |= DeclarationModifiers.New;
allowedModifiers |= DeclarationModifiers.New |
DeclarationModifiers.Sealed |
DeclarationModifiers.Abstract |
DeclarationModifiers.Static |
DeclarationModifiers.Virtual;

if (!isInterface)
{
allowedModifiers |=
DeclarationModifiers.AccessibilityMask |
DeclarationModifiers.Sealed |
DeclarationModifiers.Abstract |
DeclarationModifiers.Static |
DeclarationModifiers.Virtual |
DeclarationModifiers.Override;
}
else
{
const DeclarationModifiers allowedAccess = (DeclarationModifiers.AccessibilityMask & ~(DeclarationModifiers.Protected | DeclarationModifiers.ProtectedInternal));

// This is needed to make sure we can detect 'public' modifier specified explicitly and
// check it against language version below.
defaultAccess = DeclarationModifiers.None;

allowedModifiers |= allowedAccess | DeclarationModifiers.Extern;
defaultInterfaceImplementationModifiers |= DeclarationModifiers.Sealed |
DeclarationModifiers.Abstract |
DeclarationModifiers.Static |
DeclarationModifiers.Virtual |
DeclarationModifiers.Extern |
allowedAccess;
}
}

if (!isInterface)
Expand All @@ -433,12 +450,15 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool expli

this.CheckUnsafeModifier(mods, diagnostics);

// Let's overwrite modifiers for interface methods with what they are supposed to be.
ModifierUtils.ReportDefaultInterfaceImplementationModifiers(!isFieldLike, mods,
defaultInterfaceImplementationModifiers,
location, diagnostics);

// Let's overwrite modifiers for interface events with what they are supposed to be.
// Proper errors must have been reported by now.
if (isInterface)
{
mods = (mods & ~DeclarationModifiers.AccessibilityMask) | DeclarationModifiers.Public |
(isFieldLike ? DeclarationModifiers.Abstract : DeclarationModifiers.Virtual);
mods = ModifierUtils.AdjustModifiersForAnInterfaceMember(mods, !isFieldLike);
}

return mods;
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceFieldLikeEventSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ internal SourceFieldLikeEventSymbol(SourceMemberContainerTypeSymbol containingTy
// Don't initialize this.type - we'll just use the type of the field (which is lazy and handles var)
}

if (inInterfaceType && !this.IsAbstract && !this.IsExtern)
{
diagnostics.Add(ErrorCode.ERR_EventNeedsBothAccessors, this.Locations[0], this);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is this an error?

Because non-abstract, non-extern events should declare both accessors with implementation.

}

// Accessors will assume that Type is available.
_addMethod = new SynthesizedFieldLikeEventAccessorSymbol(this, isAdder: true);
_removeMethod = new SynthesizedFieldLikeEventAccessorSymbol(this, isAdder: false);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
const DeclarationModifiers allowedAccess = (DeclarationModifiers.AccessibilityMask & ~(DeclarationModifiers.Protected | DeclarationModifiers.ProtectedInternal));

// This is needed to make sure we can detect 'public' modifier specified explicitly and
// check it agains language version below.
// check it against language version below.
defaultAccess = DeclarationModifiers.None;

allowedModifiers |= allowedAccess | DeclarationModifiers.Extern | DeclarationModifiers.Async;
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ internal Accessibility LocalAccessibility
get { return ModifierUtils.EffectiveAccessibility(this.DeclarationModifiers); }
}

private DeclarationModifiers MakeModifiers(AccessorDeclarationSyntax syntax, bool isExplicitInterfaceImplementation, bool hasBody, Location location, DiagnosticBag diagnostics, out bool modifierErrors)
private DeclarationModifiers MakeModifiers(AccessorDeclarationSyntax syntax, bool isExplicitInterfaceImplementation,
bool hasBody, Location location, DiagnosticBag diagnostics, out bool modifierErrors)
{
// No default accessibility. If unset, accessibility
// will be inherited from the property.
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool isExp
const DeclarationModifiers allowedAccess = (DeclarationModifiers.AccessibilityMask & ~(DeclarationModifiers.Protected | DeclarationModifiers.ProtectedInternal));

// This is needed to make sure we can detect 'public' modifier specified explicitly and
// check it agains language version below.
// check it against language version below.
defaultAccess = DeclarationModifiers.None;

allowedModifiers |= allowedAccess | DeclarationModifiers.Extern;
Expand Down
14 changes: 13 additions & 1 deletion src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2011,16 +2011,28 @@ interface I
event System.Action E3 { add; remove; }
}
";
CreateStandardCompilation(text).VerifyDiagnostics(
CreateStandardCompilation(text, parseOptions: TestOptions.Regular7).VerifyDiagnostics(
// (4,30): error CS8107: Feature 'default interface implementation' is not available in C# 7. Please use language version 7.1 or greater.
// event System.Action E1 { add; }
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "add").WithArguments("default interface implementation", "7.1").WithLocation(4, 30),
// (4,33): error CS0073: An add or remove accessor must have a body
// event System.Action E1 { add; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(4, 33),
// (5,30): error CS8107: Feature 'default interface implementation' is not available in C# 7. Please use language version 7.1 or greater.
// event System.Action E2 { remove; }
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "remove").WithArguments("default interface implementation", "7.1").WithLocation(5, 30),
// (5,36): error CS0073: An add or remove accessor must have a body
// event System.Action E2 { remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(5, 36),
// (6,30): error CS8107: Feature 'default interface implementation' is not available in C# 7. Please use language version 7.1 or greater.
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "add").WithArguments("default interface implementation", "7.1").WithLocation(6, 30),
// (6,33): error CS0073: An add or remove accessor must have a body
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(6, 33),
// (6,35): error CS8107: Feature 'default interface implementation' is not available in C# 7. Please use language version 7.1 or greater.
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "remove").WithArguments("default interface implementation", "7.1").WithLocation(6, 35),
// (6,41): error CS0073: An add or remove accessor must have a body
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(6, 41),
Expand Down
Loading