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

Support declaring operators + - ! ~ ++ -- true false * / % & | ^ << >> > < >= <= in interfaces. #20401

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
2 changes: 2 additions & 0 deletions docs/features/DefaultInterfaceImplementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class Test1 : I1

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

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

Choose a reason for hiding this comment

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

Is the intention to support equality later on? If not, maybe add a note as to why it is not allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the intention to support equality later on? If not, maybe add a note as to why it is not allowed.

This is a topic for LDM discussion. The list is meant to simply enumerate what is supported by the prototype without any reasoning, and it doesn't have to match the proposed spec.



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

Expand Down
26 changes: 18 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1083,17 +1083,29 @@ private BinaryOperatorAnalysisResult BinaryOperatorOverloadResolution(BinaryOper
resultKind = possiblyBest.HasValue ? LookupResultKind.Viable : LookupResultKind.Empty;
}

if (possiblyBest.HasValue &&
(object)possiblyBest.Signature.Method != null)
if (possiblyBest.HasValue)
{
Symbol symbol = possiblyBest.Signature.Method;
ReportDiagnosticsIfObsolete(diagnostics, symbol, node, hasBaseReceiver: false);
ReportObsoleteAndFeatureAvailabilityDiagnostics(possiblyBest.Signature.Method, node, diagnostics);
}

result.Free();
return possiblyBest;
}

private void ReportObsoleteAndFeatureAvailabilityDiagnostics(MethodSymbol operatorMethod, CSharpSyntaxNode node, DiagnosticBag diagnostics)
{
if ((object)operatorMethod != null)
{
ReportDiagnosticsIfObsolete(diagnostics, operatorMethod, node, hasBaseReceiver: false);

if (operatorMethod.ContainingType.IsInterface &&
operatorMethod.ContainingModule != Compilation.SourceModule)
{
Binder.CheckFeatureAvailability(node, MessageID.IDS_DefaultInterfaceImplementation, diagnostics);
}
}
}

private bool IsDefaultLiteralAllowedInBinaryOperator(BinaryOperatorKind kind, BoundExpression left, BoundExpression right)
{
bool isEquality = kind == BinaryOperatorKind.Equal || kind == BinaryOperatorKind.NotEqual;
Expand Down Expand Up @@ -1169,11 +1181,9 @@ private UnaryOperatorAnalysisResult UnaryOperatorOverloadResolution(
resultKind = possiblyBest.HasValue ? LookupResultKind.Viable : LookupResultKind.Empty;
}

if (possiblyBest.HasValue &&
(object)possiblyBest.Signature.Method != null)
if (possiblyBest.HasValue)
{
Symbol symbol = possiblyBest.Signature.Method;
ReportDiagnosticsIfObsolete(diagnostics, symbol, node, hasBaseReceiver: false);
ReportObsoleteAndFeatureAvailabilityDiagnostics(possiblyBest.Signature.Method, node, diagnostics);
}

result.Free();
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,15 @@ public static BinaryOperatorOverloadResolutionResult GetInstance()

public void Free()
{
this.Results.Clear();
Clear();
Pool.Free(this);
}

public void Clear()
{
this.Results.Clear();
}

public static readonly ObjectPool<BinaryOperatorOverloadResolutionResult> Pool = CreatePool();

private static ObjectPool<BinaryOperatorOverloadResolutionResult> CreatePool()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public static bool DefinitelyHasNoUserDefinedOperators(TypeSymbol type)
case TypeKind.Struct:
case TypeKind.Class:
case TypeKind.TypeParameter:
case TypeKind.Interface:
break;
default:
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Diagnostics;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
using System.Collections.Generic;
using System.Collections.Immutable;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand Down Expand Up @@ -284,6 +287,8 @@ private bool GetUserDefinedOperators(UnaryOperatorKind kind, BoundExpression ope
// SPEC: operators is the set provided by the direct base class of T0, or the effective
// SPEC: base class of T0 if T0 is a type parameter.

// PROTOTYPE(DefaultInterfaceImplementation): The spec quote should be adjusted to cover operators from interfaces as well.

TypeSymbol type0 = operand.Type.StrippedType();

// Searching for user-defined operators is expensive; let's take an early out if we can.
Expand Down Expand Up @@ -319,6 +324,51 @@ private bool GetUserDefinedOperators(UnaryOperatorKind kind, BoundExpression ope
}
}

// Look in base interfaces, or effective interfaces for type parameters
Copy link
Member

@agocke agocke Jun 22, 2017

Choose a reason for hiding this comment

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

A note above for the spec quote could be useful to indicate that it's no longer valid. #Resolved

if (!hadApplicableCandidates)
{
ImmutableArray<NamedTypeSymbol> interfaces = default;
if (type0.IsInterfaceType())
{
interfaces = type0.AllInterfacesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics);
}
else if (type0.IsTypeParameter())
{
interfaces = ((TypeParameterSymbol)type0).AllEffectiveInterfacesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics);
}

if (!interfaces.IsDefaultOrEmpty)
{
var shadowedInterfaces = PooledHashSet<NamedTypeSymbol>.GetInstance();
var resultsFromInterface = ArrayBuilder<UnaryOperatorAnalysisResult>.GetInstance();
results.Clear();

foreach (NamedTypeSymbol @interface in interfaces)
{
if (shadowedInterfaces.Contains(@interface))
{
// this interface is "shadowed" by a derived interface
continue;
}

operators.Clear();
resultsFromInterface.Clear();
GetUserDefinedUnaryOperatorsFromType(@interface, kind, name, operators);
if (CandidateOperators(operators, operand, resultsFromInterface, ref useSiteDiagnostics))
{
hadApplicableCandidates = true;
results.AddRange(resultsFromInterface);

// this interface "shadows" all its base interfaces
shadowedInterfaces.AddAll(@interface.AllInterfacesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics));
}
}

shadowedInterfaces.Free();
resultsFromInterface.Free();
}
}

operators.Free();

return hadApplicableCandidates;
Expand Down
14 changes: 7 additions & 7 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1574,8 +1574,8 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_BadShiftOperatorSignature" xml:space="preserve">
<value>The first operand of an overloaded shift operator must have the same type as the containing type, and the type of the second operand must be int</value>
</data>
<data name="ERR_InterfacesCantContainOperators" xml:space="preserve">
<value>Interfaces cannot contain operators</value>
<data name="ERR_InterfacesCantContainConversionOrEqualityOperators" xml:space="preserve">
<value>Interfaces cannot contain conversion, equality, or inequality operators</value>
Copy link
Member

@jcouv jcouv Jun 23, 2017

Choose a reason for hiding this comment

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

Looking at test Operators_01, it seems that inequality operators are supported. Maybe the error message is out-of-date? #Closed

Copy link
Contributor Author

@AlekseyTs AlekseyTs Jun 23, 2017

Choose a reason for hiding this comment

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

Looking at test Operators_01, it seems that inequality operators are supported. Maybe the error message is out-of-date?

This test doesn't test unsupported operators. Neither conversion, nor equality/inequality. #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 got confused thinking that inequality was <= and related...


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

</data>
<data name="ERR_StructsCantContainDefaultConstructor" xml:space="preserve">
<value>Structs cannot contain explicit parameterless constructors</value>
Expand Down Expand Up @@ -2510,7 +2510,7 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<value>Could not find '{0}' specified for Main method</value>
</data>
<data name="ERR_MainClassNotClass" xml:space="preserve">
<value>'{0}' specified for Main method must be a valid non-generic class or struct or interface</value>
<value>'{0}' specified for Main method must be a non-generic class, struct, or interface</value>
</data>
<data name="ERR_NoMainInClass" xml:space="preserve">
<value>'{0}' does not have a suitable static Main method</value>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ internal enum ErrorCode
ERR_BadUnaryOperatorSignature = 562,
ERR_BadBinaryOperatorSignature = 563,
ERR_BadShiftOperatorSignature = 564,
ERR_InterfacesCantContainOperators = 567,
ERR_InterfacesCantContainConversionOrEqualityOperators = 567,
ERR_StructsCantContainDefaultConstructor = 568,
ERR_CantOverrideBogusMethod = 569,
ERR_BindToBogus = 570,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,11 @@ private static bool ConvertedHasEqual(BinaryOperatorKind oldOperatorKind, BoundN
var conv = (BoundConversion)node;
if (conv.ExplicitCastInCode) return false;
NamedTypeSymbol nt = conv.Operand.Type as NamedTypeSymbol;
if ((object)nt == null || !nt.IsReferenceType) return false;
if ((object)nt == null || !nt.IsReferenceType || nt.IsInterface)
{
return false;
}

string opName = (oldOperatorKind == BinaryOperatorKind.ObjectEqual) ? WellKnownMemberNames.EqualityOperatorName : WellKnownMemberNames.InequalityOperatorName;
for (var t = nt; (object)t != null; t = t.BaseTypeNoUseSiteDiagnostics)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2711,8 +2711,13 @@ private static void CheckInterfaceMember(Symbol member, DiagnosticBag diagnostic
}
break;
case MethodKind.Conversion:
diagnostics.Add(ErrorCode.ERR_InterfacesCantContainConversionOrEqualityOperators, member.Locations[0]);
break;
case MethodKind.UserDefinedOperator:
diagnostics.Add(ErrorCode.ERR_InterfacesCantContainOperators, member.Locations[0]);
if (meth.Name == WellKnownMemberNames.EqualityOperatorName || meth.Name == WellKnownMemberNames.InequalityOperatorName)
Copy link
Member

Choose a reason for hiding this comment

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

Consider extracting name checks to helper method since the same checks occur in multiple places.

{
diagnostics.Add(ErrorCode.ERR_InterfacesCantContainConversionOrEqualityOperators, member.Locations[0]);
}
break;
case MethodKind.Destructor:
diagnostics.Add(ErrorCode.ERR_OnlyClassesCanContainDestructors, member.Locations[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ private SourceUserDefinedOperatorSymbol(
{
CheckForBlockAndExpressionBody(
syntax.Body, syntax.ExpressionBody, syntax, diagnostics);

if (name != WellKnownMemberNames.EqualityOperatorName && name != WellKnownMemberNames.InequalityOperatorName)
{
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody: syntax.Body != null || syntax.ExpressionBody != null, diagnostics: diagnostics);
}
}

internal new OperatorDeclarationSyntax GetSyntax()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ protected SourceUserDefinedOperatorSymbolBase(

this.MakeFlags(methodKind, declarationModifiers, returnsVoid: false, isExtensionMethod: false);

if (this.ContainingType.IsInterface)
if (this.ContainingType.IsInterface &&
(methodKind == MethodKind.Conversion || name == WellKnownMemberNames.EqualityOperatorName || name == WellKnownMemberNames.InequalityOperatorName))
{
// If we have an operator in an interface, we already have reported that fact as
// If we have a conversion or equality/inequality operator in an interface, we already have reported that fact as
// an error. No need to cascade the error further.
return;
}
Expand Down Expand Up @@ -160,9 +161,11 @@ protected override void MethodChecks(DiagnosticBag diagnostics)

this.SetReturnsVoid(_lazyReturnType.SpecialType == SpecialType.System_Void);

// If we have an operator in an interface or static class then we already
// have reported that fact as an error. No need to cascade the error further.
if (this.ContainingType.IsInterfaceType() || this.ContainingType.IsStatic)
// If we have a conversion/equality/inequality operator in an interface or static class then we already
// have reported that fact as an error. No need to cascade the error further.
if ((this.ContainingType.IsInterfaceType() &&
(MethodKind == MethodKind.Conversion || Name == WellKnownMemberNames.EqualityOperatorName || Name == WellKnownMemberNames.InequalityOperatorName)) ||
this.ContainingType.IsStatic)
{
return;
}
Expand Down
11 changes: 10 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Roslyn.Utilities;
Expand Down Expand Up @@ -994,6 +995,7 @@ private static Symbol FindMostSpecificImplementation(Symbol interfaceMember,
conflictingImplementation1 = null;
conflictingImplementation2 = null;
Symbol implementation = null;
PooledHashSet<NamedTypeSymbol> shadowedInterfaces = null;

foreach (var interfaceType in implementingType.AllInterfacesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics))
{
Expand All @@ -1004,8 +1006,10 @@ private static Symbol FindMostSpecificImplementation(Symbol interfaceMember,
if ((object)implementation == null)
{
implementation = candidate;
shadowedInterfaces = PooledHashSet<NamedTypeSymbol>.GetInstance();
shadowedInterfaces.AddAll(implementation.ContainingType.AllInterfacesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics));
}
else if(!implementation.ContainingType.ImplementsInterface(interfaceType, ref useSiteDiagnostics))
else if(!shadowedInterfaces.Contains(interfaceType))
{
// we have a conflict
conflictingImplementation1 = implementation;
Expand All @@ -1016,6 +1020,11 @@ private static Symbol FindMostSpecificImplementation(Symbol interfaceMember,
}
}

if (shadowedInterfaces != null)
{
shadowedInterfaces.Free();
}

return implementation;
}

Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Test/Emit/Emit/EntryPointTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ namespace N { namespace M { } }
";
var compilation = CreateStandardCompilation(source, options: TestOptions.ReleaseExe.WithMainTypeName("N.M"));
compilation.VerifyDiagnostics(
// (2,25): error CS1556: 'N.M' specified for Main method must be a valid non-generic class or struct or interface
// (2,25): error CS1556: 'N.M' specified for Main method must be a non-generic class, struct, or interface
Diagnostic(ErrorCode.ERR_MainClassNotClass, "M").WithArguments("N.M"));
}

Expand All @@ -269,7 +269,7 @@ public static void Main() { }
";
var compilation = CreateStandardCompilation(source, options: TestOptions.ReleaseExe.WithMainTypeName("C.D"));
compilation.VerifyDiagnostics(
// (4,12): error CS1556: 'C<T>.D' specified for Main method must be a valid non-generic class or struct or interface
// (4,12): error CS1556: 'C<T>.D' specified for Main method must be a non-generic class, struct, or interface
Diagnostic(ErrorCode.ERR_MainClassNotClass, "D").WithArguments("C<T>.D"));
}

Expand Down Expand Up @@ -343,7 +343,7 @@ public static class E
// Dev10 reports: CS1555: Could not find 'D.DD' specified for Main method
compilation = CreateStandardCompilation(cs, options: TestOptions.ReleaseExe.WithMainTypeName("D.DD"));
compilation.VerifyDiagnostics(
// (18,25): error CS1556: 'D<T>.DD' specified for Main method must be a valid non-generic class or struct or interface
// (18,25): error CS1556: 'D<T>.DD' specified for Main method must be a non-generic class, struct, or interface
Diagnostic(ErrorCode.ERR_MainClassNotClass, "DD").WithArguments("D<T>.DD"));
}

Expand Down Expand Up @@ -420,7 +420,7 @@ class C
}";
// Dev10 reports CS1555: Could not find 'A.B.C' specified for Main method
CreateStandardCompilation(source, options: TestOptions.ReleaseExe.WithMainTypeName("A.B.C")).VerifyDiagnostics(
// (14,11): error CS1556: 'A.B<T>.C' specified for Main method must be a valid non-generic class or struct or interface
// (14,11): error CS1556: 'A.B<T>.C' specified for Main method must be a non-generic class, struct, or interface
Diagnostic(ErrorCode.ERR_MainClassNotClass, "C").WithArguments("A.B<T>.C"));
}

Expand Down Expand Up @@ -465,7 +465,7 @@ static void Main() { }
";
var compilation = CreateStandardCompilation(source, options: TestOptions.ReleaseExe.WithMainTypeName("C"));
compilation.VerifyDiagnostics(
// (7,7): error CS1556: 'C<T>' specified for Main method must be a valid non-generic class or struct or interface
// (7,7): error CS1556: 'C<T>' specified for Main method must be a non-generic class, struct, or interface
Diagnostic(ErrorCode.ERR_MainClassNotClass, "C").WithArguments("C<T>"));
}

Expand Down
Loading