Check interpolated string arguments in Binder.GetValEscape()#63263
Check interpolated string arguments in Binder.GetValEscape()#63263cston merged 4 commits intodotnet:mainfrom
Conversation
333fred
left a comment
There was a problem hiding this comment.
Done review pass (commit 1). /cc @stephentoub, presumably DefaultInterpolatedStringHandler will need to react to this breaking change.
RikkiGibson
left a comment
There was a problem hiding this comment.
LGTM modulo the review comments.
| { | ||
| Span<char> s = stackalloc char[10]; | ||
| CustomHandler h2 = new CustomHandler(0, 1); | ||
| h2.AppendFormatted(s); // 1 |
There was a problem hiding this comment.
I didn't understand why the interpolated string version had no errors but the "explicit" version has this error. Could you please clarify?
There was a problem hiding this comment.
My understanding is the interpolated string version is considered as creating a CustomHandler instance as if the new CustomHandler() and the interpolated string arguments were combined as an object initializer. In F1(), where the interpolated string argument s has safe-to-escape of the current method, the resulting CustomHandler instance will have safe-to-escape of the current method as well. In F2(), the argument s is not included in the CustomHandler object initializer, so the resulting instance is safe-to-escape of the calling method.
There was a problem hiding this comment.
My understanding is the interpolated string version is considered as creating a CustomHandler instance as if the new CustomHandler() and the interpolated string arguments were combined as an object initializer.
I would expect these scenarios to have the same behavior.
There was a problem hiding this comment.
I would expect these scenarios to have the same behavior.
If we make the CustomHandler in the first example explicit, the two examples are essentially:
CustomHandler h1 = $"{s}";
M(h1);
CustomHandler h2 = new CustomHandler(0, 1);
h2.AppendFormatted(s);
M(h2);The difference between the two is the initializers: the initializer for h1 limits the scope to the current method; the initializer for h2 does not limit the scope.
src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs
Outdated
Show resolved
Hide resolved
333fred
left a comment
There was a problem hiding this comment.
Done review pass (commit 2). The current behavior is not what I'd expect from the feature.
Fixes #63262