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

Provide a normative spec reference for all types not in the CLI spec #1135

Draft
wants to merge 4 commits into
base: draft-v8
Choose a base branch
from

Conversation

RexJaeschke
Copy link
Contributor

There are no normative semantics for the types we've added in Annex C that are not in the CLI spec (or, for that matter for any members added to CLI-specified types since 2006).

Recently, I thought of an extremely light-weight way to achieve this: Make the .NET API spec site a normative reference! I discussed this with Bill, and after he floated the idea internally, there was no pushback. So here is a PR to make it happen.

Now, you may well ask, "Why didn't we do this before?" Well, I just thought of it, and, in any event, when we were Fast Tracking to ISO, they would not have allowed such a normative reference.

I couldn't find an open issue pertaining to this, but if anyone does, please let me know or link this PR to that issue.

@RexJaeschke RexJaeschke added type: bug The Standard does not describe the language as intended or implemented meeting: proposal There is an informal proposal in the issue, worth discussing in a meeting labels Jun 24, 2024
@RexJaeschke RexJaeschke requested a review from jskeet June 24, 2024 16:10
@BillWagner
Copy link
Member

@gewarren @terrajobst Do either of you want to weigh in on this change? This would have the C# standard refer to the API documentation on Learn as the normative API reference.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

The Learn site supports versioning, so we should link to the matching API version.

@@ -10,4 +10,6 @@ ISO/IEC 2382, *Information technology — Vocabulary*.

ISO/IEC 60559:2020, *Information technology — Microprocessor Systems — Floating-Point arithmetic*

*.NET API Documentation*, https://learn.microsoft.com/en-us/dotnet/api
Copy link
Member

Choose a reason for hiding this comment

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

For C# 8, we should link to the .NET Core 3.1 version of the APIs. We should also remove the locale:

Suggested change
*.NET API Documentation*, https://learn.microsoft.com/en-us/dotnet/api
*.NET API Documentation*, https://learn.microsoft.com/dotnet/api/?view=netcore-3.1

@Nigel-Ecma
Copy link
Contributor

Nigel-Ecma commented Jul 8, 2024

I can see why ISO would be wary, a Standard should not link to a source which might change.

Furthermore the Standard may gain a far larger API surface than it needs, IIRC the current types that are included only include the API of each that is needed.

Therefore I think it would be much better if the Standard included the types directly, using the suggested reference to do that.

Failing that, if only for now, I agree with @BillWagner – we must link to a specific version of each type (not the whole .NET API). Also remove the locale as Bill suggested. We should probably make sure the pages are captured by archive.org and use links to them there.

Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

My comment should probably have been a review.

I am suggesting:

  • Each type not documented in the CLI Standard should have a link to the type in the .NET docs
  • That link should be an Internet Archive link so it is fixed to a particular version and will stay accessible

@terrajobst
Copy link
Member

I agree with @Nigel-Ecma that linking to a doc site feels odd, given that those are operated by a specific implementation of the spec, will include more what is needed by C#, and can change at any point in time.

Practically, the bar we apply to our core APIs is that we don't do breaking changes; that is we're extremely unlikely to make changes that would violate the spec, especially so because this would likely result in functional defects which I'd think we'd catch as part of product testing and customer feedback.

As someone who owned a formal API specification (.NET Standard) it seems you'll want to a snapshot of the APIs in textual form that is sufficiently precise. I think a C# API declaration would achieve that; it would describe the ABI requirements that a functional compiler would use to bind to these APIs. In some cases this will require those types to be co-located with System.Object, in many cases it won't and simple be based on namespace-qualified naming for types and member lookups, some of which allow extension member, some will not.

The module and name-lookup requirements aren't described in our docs and would be specific to C# and thus should be documented there.

As far as behavioral documentation goes, I'm not sure how much detail the C# spec desires here. Most of our APIs don't have very specific documentation around their behaviors as the target audience is users of the API, not another party trying to implement the API compatibly, which is what the C# spec is effectively trying to achieve.

We had the same issue for .NET Standard and it was never resolved (and since .NET Standard is discontinued there is no pressure to resolve it now).

@Nigel-Ecma
Copy link
Contributor

I tried to get the first type into the Internet Archive – it works fine apart from MS inserting a locale, with two attempts I ended up with en-nz and and en-us versions despite starting from a URL without a locale in it. The link for the latter is:

https://web.archive.org/web/20240709003128/https://learn.microsoft.com/en-us/dotnet/api/system.formattablestring?view=netcore-3.1

So we can fix the version, but not remove the locale – unless there is a “no locale” locale?

Of course coincidentally this type, FormattableString, points to looseness in the Standard around interpolated strings and how listing these types won’t fill it. C’est la vie.

@RexJaeschke RexJaeschke added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Jul 9, 2024
@BillWagner
Copy link
Member

@Nigel-Ecma 's comments gave me some inspiration for automation.

It would be possible to write a GitHub action that parsed the declarations in Annex C and add links to the correct version of that type, member, or namespace.

Thoughts?

/cc @terrajobst

@terrajobst
Copy link
Member

Assuming Annex C is just plain C# it wouldn't be hard to parse with Roslyn. I do that for apisof.net where I parse the API listings in our review notes to link them from a given API:

https://github.com/dotnet/apisof.net/blob/main/src/Terrajobst.ApiCatalog.Generation/DesignNotes/ReviewedApiParser.cs

@Nigel-Ecma
Copy link
Contributor

@BillWagner – feel free to write a script if you wish :-) You can force a page into the internet archive using a URL, see the end of bullet 3 on this IA page. Whether you can do that without a window to read the archived URL back from I've no idea. You wouldn't want to do this from a script that was run often as it would upload a new copy each time!

@jskeet
Copy link
Contributor

jskeet commented Jul 11, 2024

The summary of the discussion we had was:

  • Using external links: Any external link could go stale or have content updated. Web archive is somewhat better for that, but it could easily still fail in the long term (and is an odd thing to see in a spec)
  • Using imported content: To be certain of the content, we'd need to include it directly in the spec. That would solve the problem (and we could probably get appropriate permissions) - but it's a lot of work.
  • We could just close the issue as "this is a genuine issue, but doesn't seem to actually be causing problems for customers, and isn't worth the pain of fixing"

The current proposal is the last of these. Those present in the meeting were okay with that. @Nigel-Ecma would you be okay with that? Happy to discuss it next time if you want to make the case for an alternative.

@Nigel-Ecma
Copy link
Contributor

Nigel-Ecma commented Jul 11, 2024 via email

@jskeet
Copy link
Contributor

jskeet commented Jul 11, 2024

Do we have a way of parking the issue rather than simply closing?

Not really, but we could create a label of "resolved: maybe revisit" or similar, if that would help?

@Nigel-Ecma
Copy link
Contributor

Nigel-Ecma commented Jul 11, 2024 via email

@BillWagner
Copy link
Member

Proposal: Make a "parked" milestone, and add any issues there.

@RexJaeschke RexJaeschke removed meeting: discuss This issue should be discussed at the next TC49-TG2 meeting meeting: proposal There is an informal proposal in the issue, worth discussing in a meeting labels Jul 16, 2024
@RexJaeschke RexJaeschke added this to the Parked milestone Aug 15, 2024
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

I agree with moving it to Parked.
I'm guessing Parked items should be Open so I won't close.

@Nigel-Ecma Nigel-Ecma self-requested a review September 5, 2024 20:24
@Playgirlkaybraz11 Playgirlkaybraz11 mentioned this pull request Sep 6, 2024
@BillWagner BillWagner marked this pull request as draft September 9, 2024 18:33
@BillWagner BillWagner dismissed their stale review September 9, 2024 18:33

Parking for later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug The Standard does not describe the language as intended or implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants