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

Conversions to restricted types are not disallowed in expression trees #74143

Open
jjonescz opened this issue Jun 25, 2024 · 4 comments
Open
Labels
Area-Compilers Bug untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@jjonescz
Copy link
Contributor

We disallow restricted types in expression trees (since #30871). But we exclude conversions to restricted types - #30871 (comment) mentions avoiding duplicate diagnostics, but there are other scenarios which result in no diagnostics at all, like:

using System;
using System.Linq.Expressions;

C.R(() => C.M(new string[] { "a" }));

static class C
{
    public static void R(Expression<Action> e) => e.Compile()();
    public static void M(ReadOnlySpan<string> x) => Console.Write(x[0]);
}

SharpLab

Note that this currently compiles and runs, so fixing this bug would be a breaking change.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Jun 25, 2024
@jaredpar
Copy link
Member

I'm still a bit surprised that this runs at all. Part of the reason that we banned ref struct in expression trees is because, at the time at least, they would not actually be able to compile and execute. It seems like at least in some cases there is no issue executing the code.

@VSadov, @agocke can you all help clarify when an expression tree can and cannot call a method that has a ref struct parameter?

@jjonescz
Copy link
Contributor Author

jjonescz commented Jun 25, 2024

I think most expression trees would compile and run fine. A problematic one was for example with default(RefStruct) constant because that would be passed into Expression.Constant as an object parameter and hence boxed - as explained in #30776 (comment).

(A related issue: #31957)

@jaredpar
Copy link
Member

Bringing up part of the conversation from that issue

However the Expression.Constant(object, Type) requires the default(Struct1) to be boxed into object, which ref structs don't support.

In this case though default isn't a constant, it's a value. Is it just a terminology issue and we typically represent using the Expression.Constant API?

@RikkiGibson as he wrote the statement I'm questioniing 😄

@agocke
Copy link
Member

agocke commented Jun 26, 2024

I think most expression trees would compile and run fine.

I'm not sure about most. I think the problematic cases are where ref structs are used as literals (the constant case) or captured. My recollection here was rather than try to enumerate all the problematic cases, we just banned them entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

3 participants