dotnet / csharpstandard

Working space for ECMA-TC49-TG2, the C# standard committee.
Creative Commons Attribution 4.0 International
707 stars 84 forks source link

[return: attribute] on accessor that returns void #340

Open KalleOlaviNiemitalo opened 3 years ago

KalleOlaviNiemitalo commented 3 years ago

Microsoft's documentation on Attribute specification and Microsoft C# compilers allow a return _attribute_targetspecifier in the following contexts, where ECMA-334 5th edition § 22.3 (Attribute specification) does not allow it:

using System;

public class Class1
{
    public event EventHandler Event
    {
        [return: Demo] add { }
        [return: Demo] remove { }
    }

    public int this[int index]
    {
        get { return 0; }
        [return: Demo] set { }
    }

    public int Property
    {
        get { return 0; }
        [return: Demo] set { }
    }
}

[AttributeUsage(AttributeTargets.ReturnValue)]
public sealed class DemoAttribute : Attribute
{
}

Because return: attributes are generally allowed on methods that return void, this feature does not seem to cause any semantic problems.

The following C# compilers allow these attribute specifications even with /langversion:ISO-1:

I don't know whether this feature has been formally proposed in https://github.com/dotnet/csharplang/. Anyway, it has already been implemented and documented, so perhaps it should be standardized.

KalleOlaviNiemitalo commented 3 years ago

This seems to have been allowed already in the first public commit of Roslyn: https://github.com/dotnet/roslyn/blob/3611ed35610793e814c8aa25715aa582ec08a8b6/Src/Compilers/CSharp/Source/Symbols/Source/SourceMethodSymbol.cs#L776-L779

That code does not include any comment pointing this out as a deviation from the specification. AttributeTests.cs includes several instances of the return _attribute_targetspecifier on attributes on add and set accessors, although not on remove. I did not find any related issue or discussion in dotnet/roslyn.

KalleOlaviNiemitalo commented 3 years ago

Alternatively, this could be considered a Microsoft extension, and then from ECMA-334 5th edition chapter 2:

A conforming implementation of C# shall be accompanied by a document that defines all implementation-defined characteristics, and all extensions.

Which would make this a docs issue as soon as docs.microsoft.com replaces their current C# 6.0 draft specification with the ECMA committee draft spec.

KalleOlaviNiemitalo commented 2 years ago

In https://github.com/dotnet/docs/pull/27795, Microsoft started publishing the ECMA C# 6 draft from this repository to docs.microsoft.com, replacing https://github.com/dotnet/csharplang/tree/ms-spec-text/spec. So the usage of the return attribute target with such accessors is no longer documented there, although it is AFAIK still implemented in the compiler.

I suppose I should file a separate issue asking Microsoft to document this extension at https://github.com/dotnet/roslyn/tree/main/docs/compilers/CSharp, where they document other extensions. But I'll leave this one open in case the C# committee wants to standardise this feature in some future version of C#.