Suspiscous comment about constrained calls in MemoryExtensions.cs, StringBuilder.cs, and DefaultInterpolatedStringHandler.cs

[hamarb123]

Description

If you look at this line here:

runtime/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs

Line 4317 in f9c86dc

if (((ISpanFormattable)value).TryFormat(_destination.Slice(_pos), out int charsWritten, default, _provider)) // constrained call avoiding boxing for value types

It says it's emitting a constrained call to avoid boxing, but this pattern does not seem to avoid boxing at all (even in the case where the type is known & known to implement the interface, it still seems to box):
https://sharplab.io/#v2:EYLgtghglgdgNAFxFANgHwAIGYAEGBMOAwjgN4CwAUDjXgCw4CyAPACoB8AFKzgB4CUZKrRE4oAMxydeYgM44AkgEZBnTsv4CAdAA1O/ANzDaAXypnKsBAFMATuIgBja4qVUK1WhgZ7D5qrIItgCujgg4Ojggru7GNLbWEAAmAPYwKACe9K66+mQWJkA

/cc @stephentoub who seems to have written the code (in MemoryExtensions.cs anyway) to confirm if I'm missing something obvious or not :)

Configuration

N/A

Regression?

I don't know.

Analysis

The idea here is that the JIT will elide the box & instead make a copy (not on heap) and call on that - I think we should update the comment to reflect this, as opposed to claiming we're doing some kind of contrained call. I'm happy to make a PR to do this if there's interest.

[stephentoub]

It's not about the IL. It's about how the JIT handles it.

[hamarb123]

Ok, so it's just a commenting issue then? (I say that because there is still definitely no constrained call or anything - it's more that the box is elided I assume)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

[tannergooding]

The JIT being able to do it is in part dependent on the constrained call, from what I remember. So the comment isn't strictly inaccurate, it just doesn't tell the whole story.

I'm not sure its relevant that the entire story is told in practice, there are hundreds to thousands of places throughout the BCL where similar tricks are done and not commented at all or which are similarly only done via a small comment that alludes to the in depth reason an optimization works.

[dotnet-policy-service]

This issue has been marked needs-author-action and may be missing some important information.

[tannergooding]

Ultimately, I'd defer to Stephen here on if he thinks different wording is appropriate; but I don't necessarily think the churn is worthwhile in this case myself.

[dotnet-policy-service]

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.