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

Compiler Crash on Equality Comparison of a Tuple Casted from dynamic #76028

Open
Jason5Lee opened this issue Nov 22, 2024 · 3 comments
Open

Compiler Crash on Equality Comparison of a Tuple Casted from dynamic #76028

Jason5Lee opened this issue Nov 22, 2024 · 3 comments
Assignees
Milestone

Comments

@Jason5Lee
Copy link

Version Used: .NET 9.0.0

Steps to Reproduce:

  1. Compile the following C# code (SharpLab Link):
    • The code involves casting a dynamic value to a tuple and performing an equality comparison on another tuple.
public class C {
    public static bool Test(dynamic o) =>
        (((int, int))o) == (2, 3);
}

Expected Behavior: The compiler compiles the code without crashes.

Actual Behavior:

  • The compilation process fails in Visual Studio with the error:

    "csc.exe" exited with code -2146232797

  • Additional details, including a stack trace, are available in the linked SharpLab page.
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 22, 2024
@jjonescz
Copy link
Member

System.InvalidOperationException: Unexpected value 'ExplicitDynamic' of type 'Microsoft.CodeAnalysis.CSharp.ConversionKind'
   at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitConversion(BoundConversion conversion)

@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 26, 2024
@jaredpar jaredpar added this to the 17.13 milestone Nov 26, 2024
@RikkiGibson
Copy link
Contributor

RikkiGibson commented Dec 17, 2024

Lowering of tuple equality operators has its own code path which visits the operands element-wise, digging through conversions to locate more tuple conversions recursively, in order to compare both tuples element-wise, as the language specifies.

The problem is that the path in LocalRewriter_TupleBinaryOperator.cs which digs through conversions, will lower the conversion operand, but not the conversion itself in some cases. e.g. in the following code, conv's operand is lowered but not conv itself.

case BoundConversion conv:
{
// other conversions are deferred
var deferredOperand = DeferSideEffectingArgumentToTempForTupleEquality(conv.Operand, effects, temps, conv.ExplicitCastInCode || enclosingConversionWasExplicit);
return conv.UpdateOperand(deferredOperand);
}

I can't think of a ton of ways to observe this. But casting a dynamic to tuple is one of them. Another would be a fairly contrived case like the following: SharpLab

public void M((Action, Action) dels) {
    _ = ((Action, Action))(object)((Action, Action))(() => {}, () => {}) == dels;
}

So the fix is figuring out how to get LocalRewriter to "visit" the conversion itself, while also preserving its behavior to dig through things element-wise and evaluate to temps. A few possibilities:

  1. Do the lowering of the whole conversion node normally, then dig thru it afterwards to find things that need to be stored to temps. This may require changing the DeferSideEffectingArgumentToTempForTupleEquality quite a bit in order to recognize the lowered shapes of things instead of the initial-bound shapes.
  2. Extract/duplicate/repurpose code in MakeConversionNodeCore which is handling the specific conversion kinds themselves and reuse it when lowering tuple binary operators.

There may be more options I didn't think of. (2) feels most viable to me as a start.

Tagging @jcouv for thoughts here.

@RikkiGibson
Copy link
Contributor

This bug is likely not a regression and I probably need more assistance in order to be able to solve it.

@RikkiGibson RikkiGibson modified the milestones: 17.13, 17.14 Jan 8, 2025
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

4 participants