-
Notifications
You must be signed in to change notification settings - Fork 4k
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
NullReferenceException when using switch expression with CSharpScript #49529
Comments
I also tried this with |
cc @tmat |
Hello! I opened a PR to start investigating this. I added a unit test that fails. It fails with a different stack trace when I run it in the dotnet/roslyn test suite against the master branch. Here's the error and stack trace copied over from that PR. If this rings any bells for anyone, I'd appreciate a pointer in the right direction. Otherwise I will continue to flail around and do my best 😄.
System.AggregateException : One or more errors occurred. (Assertion failed)
---- System.InvalidOperationException : Assertion failed
Stack trace: |
Looks like the assertion on line 143 is failing: https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs#L142-L143 142: Binder? switchBinder = this.GetBinder(node);
143: RoslynDebug.Assert(switchBinder is { }); |
To add context why this issue is important to me: https://twitter.com/haacked/status/1359895904719753217?s=21 Switch expressions are perfect for bot skills but they’re broken in our product. If we can fix this, it makes for a nice showcase of C# as a great language for automation imho. 😊 |
We do not have any current plans to enhance C# Interactive, including fixing issues like this with newer language features. It is useful for us to understand how people are using it/want to use it to consider future plans. Are you using C# Interactive within your own application? |
Hi @KathleenDollard! 👋
Aw bummer. Was there an announcement I missed? I wouldn't be surprised as there's so much good stuff happening with C#.
To be clear, it's not the interactive part we care about, but the use of C# as a scripting language. I'm guessing the two are the same thing as far as Roslyn is concerned, correct? But I'm happy to describe how we're using I just announced a beta of a new product that is a hosted chat bot. The bot can be extended with "skills" written by customers in Python, JavaScript, and of course, C#! The code is written in the browser and we save it off and compile it. We want the skill authoring experience to have as little boilerplate as possible. Here's an incomplete example of a skill you might right. var (cmd, value) = Bot.Arguments;
if (cmd.Value is "foo") {
await Bot.ReplyAsync("Be more original.");
return;
}
await Bot.Brain.WriteAsync(new Bar { Baz = value.Value });
await Bot.ReplyAsync("This is really contrived.");
public class Bar {
public string Baz { get; set; }
} The important thing to note is the author doesn't have to declare a class or method. They just write code like a top level program. However, they can declare classes and we pass in a This is why we chose var script = CSharpScript.Create<string>(code, options, typeof(IBot));
var bot = ServiceProvider.Create<IBot>(); // Create the bot using DI.
var result = await script.RunAsync(bot); Other options we considered were:
Unfortunately we've invested a lot of time in our approach. I'm happy to switch if this is a dead-end, but not clear on the best path forward. Any suggestions or ideas are appreciated. |
Phil, Thanks for the additional information, particularly more information on what you doing! For clarity, it looks like people can be successful today with the current state of scripting, they just have to work harder. Is that true? You did clarify why pattern matching would be nice for these bots, and it might be confusing for C# programmers to reason about what features are available. Kathleen |
Absolutely! It's been working great. The issue is we support most of the features of C# 8, but some features fail and it's confusing to our end users. And in our case, the feature that would make writing bot skills really elegant causes a
Exactly! Switch expressions make writing bot skills in C# a joy. And I don't use the word "joy" lightly here. The lack of that feature is painful for us. There's also the long term support question we need to understand. If |
So C#'s Server Discord bot is facing exactly the same issue on it's REPL <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="3.9.0-5.final" /> var income = 900m;
var res = income switch
{
< 9876 => 0 + (0.10m * (income - 0m)),
< 40126 => 987.5m + (0.12m * (income - 9875m)),
< 85126 => 4617.5m + (0.22m * (income - 40125m)),
< 163301 => 14605.5m + (0.24m * (income - 85252m)),
< 207351 => 33271.5m + (0.32m * (income - 163300m)),
< 518401 => 47367.5m + (0.35m * (income - 207350m)),
_ => 156235m + (0.37m * (income - 518400m))
};
Console.WriteLine($"Owned: {res}"); I get
While executing this line: var compileResult = await compilation.GetAllDiagnosticsAsync(); This REPL is being used to demonstrate code examples with results to users need help in the Server. |
So after some investigation i've noticed that on
is getting called over and over again until we get into BuckStopsHereBinder.cs roslyn/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs Lines 166 to 169 in 01126cb
After some more investigation if i replace on the original code with var income = 900m;
decimal res = income switch
{
< 9876 => 0 + (0.10m * (income - 0m)),
< 40126 => 987.5m + (0.12m * (income - 9875m)),
< 85126 => 4617.5m + (0.22m * (income - 40125m)),
< 163301 => 14605.5m + (0.24m * (income - 85252m)),
< 207351 => 33271.5m + (0.32m * (income - 163300m)),
< 518401 => 47367.5m + (0.35m * (income - 207350m)),
_ => 156235m + (0.37m * (income - 518400m))
};
Console.WriteLine($"Owned: {res}"); so effectively replacing Also adding invalid code inside the switch doesn't throw any syntax error, it just crashes with NullReferenceException because of the "var" var reply = args switch {
null => argumasdgadgadhafjgfhk,mfgdhgsdfhdfjd
423W^RYweafrr23nb w4 yh3ewrtgfbewsdfghbdfcvdsaghrt uy354erwt fq3wesge sdff as
fhl.
gjk
jterd
_ => args
}; If i replace |
@ProIcons oh nice! I confirmed this is the case. Still would like |
- Added ExecutableBinder on chain
Version Used:
Microsoft.CodeAnalysis.CSharp.Scripting
3.8.0.0
Steps to Reproduce:
CSharpScript
to create a script that contains a switch expression (see Unit test below for a full demo).Compile
orRunAsync
on theScript
.Expected Behavior:
It works
Actual Behavior:
It fails
Here's a failing unit test that should pass. It demonstrates the problem.
The
script.Compile();
call throws anAggregateException
. The inner exception is aNullReferenceException
.The stack trace for the
NullReferenceException
isThe text was updated successfully, but these errors were encountered: