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

Escape analysis ignores local passed by reference to interpolated string handler AppendFormatted() method #63306

Open
cston opened this issue Aug 10, 2022 · 2 comments

Comments

@cston
Copy link
Member

cston commented Aug 10, 2022

using System;
using System.Runtime.CompilerServices;

[InterpolatedStringHandler]
ref struct CustomHandler
{
    private ref readonly int _i;
    public CustomHandler(int literalLength, int formattedCount) { }
    public void AppendFormatted(in int i) { _i = ref i; }
}

class Program
{
    static CustomHandler F1()
    {
        int i = 1;
        return $"{i}"; // error?
    }

    static CustomHandler F2()
    {
        CustomHandler h = $"{2}"; // error?
        return h;
    }
}

Expected:
Escape analysis should report errors for the conversions from interpolated string to CustomHandler since a reference to the local is potentially returned to the calling method.

Actual:
No errors.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 10, 2022
@cston cston self-assigned this Aug 10, 2022
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 10, 2022
@jaredpar jaredpar added this to the 17.4 milestone Aug 10, 2022
@RikkiGibson
Copy link
Contributor

In this bit, it seems like we weren't precisely correct in our assumptions on how the calls within interpolated string conversions could be formed. An unscoped in-parameter could come in here which we don't handle properly:

if (UseUpdatedEscapeRules &&
call.Method.Parameters[0].EffectiveScope == DeclarationScope.ValueScoped)
{
continue;
}

I think we could avoid entirely making assumptions about the shape of the call here, and instead just pass it through the appropriate general purpose helper for calls (GetInvocationArgumentsForEscape or GetInvocationArgumentsForEscapeWithUpdatedRules in #62886). Another layer may be synthesizing the call within some limited set of constraints, but it doesn't matter to the escape analysis layer.

@jaredpar
Copy link
Member

jaredpar commented Oct 5, 2022

This is no longer a bug because the in here is RTSE of return only so it can't be captured. A very related scenario is if a ref struct is passed by value that is tracked by our current code. There is no test for that scenario though. This bug will now track updating the existign test for in and adding a new test for passing a ref struct by value

https://sharplab.io/#v2:EYLgtghglgdgNAFxFANnAJiA1AHwAIBMAjALABQeADAAR5EB0ASgK4wJRgCm9AwgPZgADqk4AnAMpiAblADGnAM4BucuQDaASTZjBfFBASd04hKNgBzABIQY6FGIC65UZwBm1BaeayE1Hs08Ba1t7UXIAb3JqaOpBMykDTmpxQRsAHlgEAD5qAH1XKE4UdBUyGNoAZj8AhCCbOzEACkzqFChDUQgUABlOGHMEAAs4ahbXPlFIBEN0flYEAEpqcOoAXyiYvCq8ABZqAEFBQT70ADEJqZnGlPTMnIVUmCWV/MLi6gBeD0elNfJ1sjkQi0IgAdgiG2idAAbNVAmBgg1RNRTkRGgtIctMeUbjAMmx7o9Ph4EBBZABrLooPiyUZsNQ7AgOUrlcp4UHUAAkACJwg8bKtub8APTC6hiUQTAD8mIBqyAA===

@jaredpar jaredpar closed this as completed Oct 5, 2022
@jaredpar jaredpar reopened this Oct 5, 2022
@jaredpar jaredpar modified the milestones: 17.4, 17.5 Oct 10, 2022
@jaredpar jaredpar modified the milestones: 17.5, 17.6 Jan 5, 2023
@jaredpar jaredpar assigned jaredpar and unassigned cston Jul 11, 2024
@jaredpar jaredpar modified the milestones: 17.6, 17.12 Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants