Skip to content

Conversation

gewarren
Copy link
Contributor

@gewarren gewarren commented Apr 25, 2022

mdoc 5.8.9 adds support for function pointers in C#.

@ghost ghost added the area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable. label Apr 25, 2022
@opbld34
Copy link

opbld34 commented Apr 25, 2022

Docs Build status updates of commit add050e:

✅ Validation status: passed

File Status Preview URL Details
xml/System.CommandLine.Builder/CommandLineBuilderExtensions.xml 💡Suggestion View Details
xml/System.Collections.Immutable/ImmutableArray.xml ✅Succeeded View
xml/System.Data.Common/DbBatch.xml ✅Succeeded View
xml/System.Data.Common/DbBatchCommandCollection.xml ✅Succeeded View
xml/System.IO.Compression/BrotliDecoder.xml ✅Succeeded View
xml/System.IO.Compression/BrotliEncoder.xml ✅Succeeded View
xml/System.IO.Compression/BrotliStream.xml ✅Succeeded View
xml/System.Runtime.InteropServices.ObjectiveC/ObjectiveCMarshal+UnhandledExceptionPropagationHandler.xml ✅Succeeded View
xml/System.Runtime.InteropServices.ObjectiveC/ObjectiveCMarshal.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/CertificateRequest.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X500DistinguishedName.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X509Certificate2.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X509Certificate2Collection.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X509ChainPolicy.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X509Extension.xml ✅Succeeded View
xml/System.Security.Cryptography.X509Certificates/X509SubjectKeyIdentifierExtension.xml ✅Succeeded View
xml/System.Security.Cryptography/HMACMD5.xml ✅Succeeded View
xml/System.Security.Cryptography/SHA256.xml ✅Succeeded View
xml/System.Web.DynamicData/DynamicFilter.xml ✅Succeeded View
xml/System.Web.DynamicData/QueryableFilterRepeater.xml ✅Succeeded View
xml/System.Windows.Forms/CloseReason.xml ✅Succeeded View
xml/System/Math.xml ✅Succeeded View

xml/System.CommandLine.Builder/CommandLineBuilderExtensions.xml

  • Line 0, Column 0: [Suggestion: ECMA2Yaml_Inheritdoc_InvalidTagsForStatic] Inheridoc should not use on static object for uid:System.CommandLine.Builder.CommandLineBuilderExtensions.ParseResponseFileAs(System.CommandLine.Builder.CommandLineBuilder,System.CommandLine.Parsing.ResponseFileHandling).
  • Line 0, Column 0: [Suggestion: ECMA2Yaml_Inheritdoc_InvalidTagsForStatic] Inheridoc should not use on static object for uid:System.CommandLine.Builder.CommandLineBuilderExtensions.UseVersionOption(System.CommandLine.Builder.CommandLineBuilder,System.String[]).

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

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.

@gewarren

The changes for the function pointer prototypes look good to me.

In other APIs, information looks like it was lost in this PR. Any ideas why that happened?

@@ -1,10 +1,10 @@
<Type Name="ObjectiveCMarshal+UnhandledExceptionPropagationHandler" FullName="System.Runtime.InteropServices.ObjectiveC.ObjectiveCMarshal+UnhandledExceptionPropagationHandler">
<TypeSignature Language="C#" Value="public delegate method ObjectiveCMarshal.UnhandledExceptionPropagationHandler(Exception exception, RuntimeMethodHandle lastMethod, out IntPtr context);" />
<TypeSignature Language="C#" Value="public delegate delegate* unmanaged&lt;IntPtr, void&gt; ObjectiveCMarshal.UnhandledExceptionPropagationHandler(Exception exception, RuntimeMethodHandle lastMethod, out IntPtr context);" />
Copy link
Member

Choose a reason for hiding this comment

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

Note for other reviewers: Here's one example of a function pointer in the API.

@@ -19,7 +19,7 @@
<Parameter Name="context" Type="System.IntPtr" RefType="out" />
</Parameters>
<ReturnValue>
<ReturnType>method</ReturnType>
<ReturnType>delegate* unmanaged&lt;System.IntPtr, System.Void&gt;</ReturnType>
Copy link
Member

Choose a reason for hiding this comment

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

function pointer as a return type.

Comment on lines +420 to +422
<param name="source">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
Copy link
Member

Choose a reason for hiding this comment

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

@gewarren Changes like this do concern me. Any ideas why this happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, this is a bug. I've logged https://dev.azure.com/ceapex/Engineering/_workitems/edit/598286.

Copy link
Contributor Author

@gewarren gewarren Apr 26, 2022

Choose a reason for hiding this comment

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

I've also logged an issue in this repo to reinstate the comments once the bug is fixed: #7999

@@ -493,7 +493,7 @@ The precise semantics of batch execution vary across ADO.NET providers, especial
<Parameter Name="cancellationToken" Type="System.Threading.CancellationToken" />
</Parameters>
<Docs>
<param name="behavior">One of the enumeration values that specifies options for batch execution and data retrieval.</param>
<param name="cancellationToken">To be added.</param>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The System.Data data losses are because the parameter names were accidentally changed in a previous PR. That's being tracked by #7910.

Comment on lines +420 to +422
<param name="source">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, this is a bug. I've logged https://dev.azure.com/ceapex/Engineering/_workitems/edit/598286.

Comment on lines +420 to +422
<param name="source">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
Copy link
Contributor Author

@gewarren gewarren Apr 26, 2022

Choose a reason for hiding this comment

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

I've also logged an issue in this repo to reinstate the comments once the bug is fixed: #7999

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.

Thanks again @gewarren

This LGTM. Let's :shipit:

@gewarren gewarren merged commit 102e696 into main Apr 26, 2022
@gewarren gewarren deleted the mdoc-5.8.9 branch April 26, 2022 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants