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

Language changes are breaking real-world use of dynamic #72750

Closed
stephentoub opened this issue Mar 27, 2024 · 4 comments · Fixed by #73314
Closed

Language changes are breaking real-world use of dynamic #72750

stephentoub opened this issue Mar 27, 2024 · 4 comments · Fixed by #73314

Comments

@stephentoub
Copy link
Member

stephentoub commented Mar 27, 2024

Version Used:
Version 17.10.0 Preview 3.0 [34721.271.main]

Steps to Reproduce:

using System.Text.Json;

public class C
{
    public static C M(IFoo foo, dynamic value)
    {
        var result = foo.Bar("name", value);
        return JsonSerializer.Deserialize<C>(result);
    }
}

public interface IFoo
{
    object Bar(string name, object value);
}

Expected Behavior:
Previously this compiled fine.
SharpLab

Actual Behavior:
Now it fails to compile:

error CS1503: Argument 1: cannot convert from 'object' to 'System.IO.Stream'

After the recent changes around dynamic, the type of result is now object whereas previously it was dynamic.

[jcouv:] Relates to #71421

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 27, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 28, 2024
@jaredpar jaredpar added this to the 17.10 milestone Mar 28, 2024
AlekseyTs added a commit to dotnet/csharplang that referenced this issue Apr 1, 2024
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Apr 5, 2024
…have `dynamic` result if their dynamic binding succeeded in C# 12

Fixes dotnet#72750.

Corresponding spec update - dotnet/csharplang#8027
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Apr 9, 2024
… local functions, extension methods or expanded non-array params cases.

Related to dotnet#72750.
This is an alternative limited approach for QB mode.
@jaredpar jaredpar added the servicing-consider .NET Core Tactics bug label Apr 16, 2024
@rbhanda rbhanda added servicing-approved and removed servicing-consider .NET Core Tactics bug labels Apr 16, 2024
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Apr 16, 2024
…c` arguments (dotnet#72964)

Fixes dotnet#72750.

For C# 12 language version: behavior of the compiler in regards to deciding between whether binding should be static or dynamic is the same as behavior of C# 12 compiler. As a result, for affected scenarios, what used to have `dynamic` type in C# 12 compiler will have `dynamic` type when C# 12 language version is targeted.

For newer language versions: invocations statically bound in presence of dynamic arguments should have dynamic result if their dynamic binding succeeded in C# 12. Corresponding spec update - dotnet/csharplang#8027 at commit 8.

Related to dotnet#33011, dotnet#72906, dotnet#72912, dotnet#72913, dotnet#72914, dotnet#72916, dotnet#72931.
AlekseyTs added a commit that referenced this issue Apr 17, 2024
Restore dynamic as result type of some operations involving dynamic arguments (#72964)

Fixes #72750.

For C# 12 language version: behavior of the compiler in regards to deciding between whether binding should be static or dynamic is the same as behavior of C# 12 compiler. As a result, for affected scenarios, what used to have dynamic type in C# 12 compiler will have dynamic type when C# 12 language version is targeted.

For newer language versions: invocations statically bound in presence of dynamic arguments should have dynamic result if their dynamic binding succeeded in C# 12. Corresponding spec update - dotnet/csharplang#8027 at commit 8.

Related to #33011, #72906, #72912, #72913, #72914, #72916, #72931.
@mattheww-freshview
Copy link

mattheww-freshview commented Apr 18, 2024

Will this also fix the fact that old versions of VS will compile https://dotnetfiddle.net/OdQkAQ as a legacy .NET Framework 4.8 console app but Version 17.10.0 Preview 4.0 will not?

Specifically

var f = parameter.LastModifiedDate != null ? DateTime.Parse(parameter.LastModifiedDate) : null;
DateTime? f1 = parameter.LastModifiedDate != null ? DateTime.Parse(parameter.LastModifiedDate) : null;

used to compile, now it doesn't (CS8957) if parameter is dynamic.

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Apr 18, 2024

Will this also fix the fact that old versions of VS will compile https://dotnetfiddle.net/OdQkAQ as a legacy .NET Framework 4.8 console app but Version 17.10.0 Preview 4.0 will not?

It is not clear to me how this is related to dynamic invocations. If it is, more details might help to give the answer.

@mattheww-freshview
Copy link

mattheww-freshview commented Apr 18, 2024

Specifically

var f = parameter.LastModifiedDate != null ? DateTime.Parse(parameter.LastModifiedDate) : null;
DateTime? f1 = parameter.LastModifiedDate != null ? DateTime.Parse(parameter.LastModifiedDate) : null;

used to compile, now it doesn't (CS8957) if parameter is dynamic and using Visual Studio Version 17.10.0 Preview 4.0.

To repro - copy and paste the code from the dotnetfiddle into Visual Studio Version 17.10.0 Preview 4.0 in a .NET 4.8 Console app and try and run it. It will fail. It works fine on older compilers (e.g. dotnetfiddle, VS 17.9.0) and earlier VS previews (not sure how far back though).

I suppose I am trying to clarify if this is a known bug or not. Apologies if this is unrelated.

@AlekseyTs
Copy link
Contributor

I suppose I am trying to clarify if this is a known bug or not. Apologies if this is unrelated.

This looks like the same issue and I think this scenario will be fixed as well. As a temporary workaround you can convert result of DateTime.Parse calls to dynamic.

AlekseyTs added a commit that referenced this issue May 7, 2024
… local functions (#73314)

Fixes #72750.

This implements the latest LDM decision.
In order to make sure all artifacts of the previous fix (#72964) were removed, I reverted all implementation (but not test changes) from that PR by using 'git revert`. All cleanups/refactorings that are still relevant were manually ported back.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment