Add conceptual documentation for "disabled runtime marshalling" #27820
Conversation
|
|
||
| ## Scenarios where marshalling is disabled | ||
|
|
||
| When the `DisableRuntimeMarshallingAttribute` is applied to an assembly, it affects P/Invokes and Delegate types in the assembly, as well as any calls to unmanaged function pointers in the assembly. It does not affect any P/Invoke or interop delegate types defined in other assemblies. It also does not disable marshalling for the runtime's built-in COM interop support. The built-in COM interop support can be enabled or disabled through a [feature switch](https://github.com/dotnet/runtime/blob/main/docs/workflow/trimming/feature-switches.md). |
There was a problem hiding this comment.
Could you give some clarifying examples here?
It's not exactly clear which of the below work vs not and what the behavior is given
[UnmanagedFunctionPointer(...)]
public delegate void Callback();
[DllImport(...)]
public static extern void Import(Callback callback);Where you have the varying combinations of Import defined in A and Callback defined in B where A and/or B have DisableRuntimeMarshalling defined.
There was a problem hiding this comment.
-- It's also "somewhat" unfortunate that COM interop is controlled by a separate switch I think. It would be nice to have a "one stop shop" to disable all built-in marshalling and enforce blittable conventions.
It's not a big deal though, it just means one extra thing to tweak in my own libraries.
There was a problem hiding this comment.
Since this feature doesn't totally disable P/Invoke and delegate-based interop, it didn't make sense to fully disable COM interop as part of this attribute. Since COM would still have to generate IL stubs and use marshalling for the this parameter, it doesn't make sense (at least to me) to disable the marshalling but not the built-in COM system as a whole.
There was a problem hiding this comment.
It would be nice to have a "one stop shop" to disable all built-in marshalling and enforce blittable conventions.
I'm not against that, but it seems hard in my mind. Does it mean the PIA itself – the containing assembly of the COM type – applies to that definition of the interface? Does it apply the GUID for the type? If the assembly using COM interop uses an interface does the attribute apply to the call? (I assume not). I guess we could also apply the attribute to the assembly that activates the COM object, but that also gets weird.
If you can come up with a crisp set of rules that makes intuitive sense I'd be very interested.
| - .NET variadic argument method signatures (varargs) | ||
| - `in`, `ref`, `out` parameters | ||
|
|
||
| ## Default rules for marshaling common types |
There was a problem hiding this comment.
| ## Default rules for marshaling common types | |
| ## Default rules for marshalling common types |
There was a problem hiding this comment.
Bah. So, what are we doing here? I thought we agreed on two ll, but could be misremembering.
There was a problem hiding this comment.
I think we as the interop team agreed on two Ls, but we never updated the style guide that the docs team uses.
@gewarren, can we update the style guide and rename things back to "marshalling"?
There was a problem hiding this comment.
Here's the style guide that Maira was likely referencing: https://styleguides.azurewebsites.net/StyleGuide/Read?id=2719&topicid=31240. If you think we should change this, there's a "Contact us" menu option under "Help" at the top.
There was a problem hiding this comment.
It would be good to also update and ensure its covered in https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions#capitalizing-compound-words-and-common-terms
CC. @bartonjs as he may know if we have concrete guidance on marshaling vs marshalling
I'd guess the latter based on usage throughout the BCL. The former only has two instances in WinForms
There was a problem hiding this comment.
Based on email from Celia Parker today (she spent 7 hours researching this), it sounds like we are sticking to one l for now.
"[...] That should be reason enough to support the argument of not changing "marshal" to "marshall" in the IESG.
Unless you have much more compelling reasoning with more powerful evidence to support it, I believe we should leave the IESG as it is."
That said, is this good to merge?
There was a problem hiding this comment.
I think so, unless @bartonjs has any pushback based on the FDG
There was a problem hiding this comment.
-- (We can also always fix it up later if there is pushback there)
There was a problem hiding this comment.
It's not in the standard spellings table.
Based on existing public types (which @tannergooding already pointed out), I believe our de facto in .NET is -ller and -lling. Here are all the hits of Marshal in our refs.cs files where it's not the last word in the type/method name or followed by a capital letter:
System.Reflection.Metadata.cs: public System.Reflection.Metadata.BlobHandle GetMarshallingDescriptor() { throw null; }
System.Reflection.Metadata.cs: public System.Reflection.Metadata.BlobHandle GetMarshallingDescriptor() { throw null; }
System.Reflection.Metadata.cs: public void AddMarshallingDescriptor(System.Reflection.Metadata.EntityHandle parent, System.Reflection.Metadata.BlobHandle descriptor) { }
System.Runtime.InteropServices.cs: public partial interface ICustomMarshaler
System.Runtime.InteropServices.cs: [System.ObsoleteAttribute("Marshalling as Currency may be unavailable in future releases.")]
System.Runtime.InteropServices.cs: [System.ObsoleteAttribute("Marshalling as VBByRefString may be unavailable in future releases.")]
System.Runtime.InteropServices.cs: [System.ObsoleteAttribute("Marshalling as AnsiBStr may be unavailable in future releases.")]
System.Runtime.InteropServices.cs: [System.ObsoleteAttribute("Marshalling as TBstr may be unavailable in future releases.")]
System.Runtime.InteropServices.cs: [System.ObsoleteAttribute("Marshalling arbitrary types may be unavailable in future releases. Please specify the type you wish to marshal as.")]
System.Runtime.InteropServices.cs: public static void RegisterForMarshalling(ComWrappers instance) { }
Counting the obsoletion messages, it's 9:1, ignoring them, it's 4:1. (And that's not counting things like the MarshallerHelpers from the new interop generator)
Wikipedia's current style for the word marshalling is the two-l form (https://en.wikipedia.org/wiki/Marshalling_(computer_science)). While I generally agree with "US spelling" when there's a country-based difference, in this case, I believe one-l is wrong.
We've sent a request to the Cloud & AI Style Guide to update the guidance to use two l's. I'm assuming they'll approve this request, so I've updated the wording in this PR to use two l's. |
Summary
Add some conceptual documentation for the upcoming
System.Runtime.InteropServices.DisableRuntimeMarshallingAttributein our new interop documentation.This PR is expected to fail until after dotnet/runtime#63320 is merged and dotnet/dotnet-api-docs is updated with API docs including the new attribute.
cc: @dotnet/interop-contrib