dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.94k stars 4.02k forks source link

Document C# extension: [return: attribute] on accessor that returns void #62145

Open KalleOlaviNiemitalo opened 2 years ago

KalleOlaviNiemitalo commented 2 years ago

The Roslyn C# compiler allows a return attribute_target_specifier in the following contexts, where ECMA-334 5th edition § 22.3 (Attribute specification) does not allow it:

Please document this extension, to comply with 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.

The feature used to be documented at https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/attributes#attribute-specification, but it was not marked as an extension, and anyway has been removed from that document, which now comes from an ECMA C# draft.

I imagine the extension could be documented in https://github.com/dotnet/roslyn/tree/main/docs/compilers/CSharp, which already describes some other extensions (if a directory can be considered "a document"), or perhaps in https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/.

Split from https://github.com/dotnet/csharpstandard/issues/340.

jcouv commented 2 years ago

Tagging @BillWagner to advise.

BillWagner commented 2 years ago

We should fix this. Our implementation docs should do two things:

  1. Document any extensions (like the one above).
  2. Document any behavior that the standard describes as "implementation defined".

I'd like to surface these facts in a docs folder that's a sibling of the spec. The source of truth could be either in dotnet/docs or dotnet/csharplang.

We can assign this to me. I'd like @jcouv's opinion on where the source of truth should be stored.

jcouv commented 2 years ago

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

@BillWagner Based on this, I think the document should live in the roslyn repo (ie. the implementation). What do you think?

BillWagner commented 2 years ago

I opened dotnet/csharpstandard#600 to address the specific issue first raised here. (My opinion is that this should be added to the standard, not considered as an extension). The committee will take that up.

As for the overall discussion, the Appendix B lists the behavior that is undefined, implementation-defined, and unspecified.

The new article must address all the "implementation defined" behavior. We can choose to clarify any of the "unspecified behavior" we choose to. We have no obligation to clarify the "undefined" behavior.

Adding @MadsTorgersen to help determine which, if any, "unspecified" behavior we should clarify.

CyrusNajmabadi commented 1 year ago

@jcouv do you think we can close this out since this is tracked in csharpstandard?

jcouv commented 1 year ago

Indeed. Closing as duplicate/moved to https://github.com/dotnet/csharpstandard/issues/600

KalleOlaviNiemitalo commented 1 year ago

Does https://github.com/dotnet/csharpstandard/issues/600 track publication of the document for Roslyn implementation-defined behaviour?

jcouv commented 1 year ago

Good catch. We do need to keep this open for a Roslyn change.