Implicit conversions to interfaces are not present in expression trees.

[VSadov]

The following expressions should result in a same shape of expression tree:

interface IDeletedID
{
    int DeletedID { get; }
}

class C1 : IDeletedID
{
    int IDeletedID.DeletedID
    {
        get
        {
            return 1;
        }
    }
}

    static void M<T>(T x) where T : IDeletedID
    {
        Expression<Func<bool>> expr1 = () => ((IDeletedID)x).DeletedID == 1;
        Expression<Func<bool>> expr2 = () => (x).DeletedID == 1;
    }

Cast to (IDeletedID) subtly changes the meaning of the expression. If, for example, x happens to be a struct, the cast will result in boxing and "DeletedID" will be called on a copy.

Old compiler inserts casts like this in the expression tree. Roslyn does not.
From execution semantics, the cast should not be there, however the change is observable when traversing the tree and there are known breaks because of this.

Should we try inserting these casts for compat reasons?

[VSadov]

Turns out this is not as straightforward as it looks.

There seem to be not a lot of consistency in when the conversion is introduced in the native compiler. For example it happens with property accesses and not with method calls.
In addition to that introducing the conversion changes the expression semantics and can lead to expression producing different results when executed.

Example:
The following code runs the same expression directly and by turning it into an expression tree, compiling and executed. Both times the result should be the same and they are indeed the same in Rolsyn. The following sample prints "pass".

However, since the old native compiler puts extra conversions, if the code is compiled by the old compiler, this code prints "fail".

using System;
using System.Linq;
using System.Linq.Expressions;
using System.Collections.Generic;
interface I
{
    int P { get; set; }
    int M();
}
struct S : I
{
    int _p;
    public int P { get { return _p++; } set { _p = value; } }
    public int M() { P = 7; return 1; }
}
class Test
{
    public static void Test1<T>() where T : I
    {
        Func<T, int> f = 
            x => x.M() + x.P + x.P;


        Expression<Func<T, int>> e = 
            x => x.M() + x.P + x.P;

        var r1 = f(default(T));
        var r2 = e.Compile()(default(T));

        Console.WriteLine(r1==r2 ? "pass" : "fail");
    }
    static void Main()
    {
        Test1<S>();
    }
}

So it started looking more like a bug in the native compiler.

[joeyadams]

This issue causes problems with LINQ-to-SQL when you try to use interfaces to abstract common database access patterns. For example:

public interface IDataObject
{
    int ID { get; set; }
}
...
public static T GetObjectByID<T>(NorthwindDataContext dc, int id)
    where T : class, IDataObject
{
    return dc.GetTable<T>().SingleOrDefault(row => row.ID == id);
}

This method throws the exception "The mapping of interface member IDataObject.ID is not supported." The exception is thrown from AttributedMetaModel.GetDataMember (source code).

This bug (be it in LINQ-to-SQL or in Roslyn) keeps my company from being able to upgrade to Visual Studio 2015 without changing our code in several places.

[divega]

@joeyadams Not sure if this would be feasible for your company, but in case it helps, it seems that adding the explicit cast makes LINQ to SQL query execution work, e.g.:

public static T GetObjectByID<T>(NorthwindDataContext dc, int id)
    where T : class, IDataObject
{
    return dc.GetTable<T>().SingleOrDefault(row => ((T)row).ID == id);
}

[divega]

Another workaround for LINQ to SQL is to replace the equality operator with the Equals() method:

public static T GetObjectByID<T>(NorthwindDataContext dc, int id)
    where T : class, IDataObject
{
    return dc.GetTable<T>().SingleOrDefault(row => row.ID.Equals(id));
}

[joeyadams]

Have the LINQ-to-SQL developers been notified of this issue yet? It looks like they should fix it on their end, given that @VSadov proved that this issue is a bug in the old compiler rather than the new.

[VSadov]

It seems that the old compiler inserts these conversions only in a case of properties.

Conversion is only observable when

• the member accessed must be a property
• receiver is a generic type parameter
• the property must be defined on an interface
• the implementing type must be a struct
• the implementation of the property getter must mutate the instance.

It seems to be highly improbable scenario in actual code, especially the last condition.
Evidently no one complained about this even though the old compiler had this kind of codegen for a very long time.

I am fairly convinced at this point that from the practical standpoint it makes more sense to just insert the cast there.

[VSadov]

Casts should not be introduced when receiver is known to be a class from its constraints.

[cherrydev]

When moving to DNX beta8 (from beta6) I encountered a problem that seems like it might be related to this. It's effectively the same as dotnet/efcore#3267 except it's on EF6, which I can't reasonably expect an update to (in any timeline that would be useful for me). Originally, I was building a DbContext mapping referencing generic classes as follows:

var userMap = builder.Entity<TUser>();
user.HasKey(u => u.Id);

This resulted in the error: The properties expression 'e => Convert(u).Id' is not valid. The expression should represent a property access: 't => t.MyProperty'. When specifying multiple properties use an anonymous type: 't => new { t.MyProperty1, t.MyProperty2 }'.

I then completely overrode the mapping method in a concrete class with no open type parameters:

var user = builder.Entity<ApplicationUser>();
user.HasKey(u => u.Id);

That change fixed the above error when configuring the DbContext. But now, at runtime, while attempting to query in EF6, I instead get the stack trace at the bottom of this post. ApplicationUser extends IdentityUser<Guid, GuidUserLogin, GuidUserRole, GuidUserClaim> and obviously should not need a cast.

I've suddenly gotten extraordinarily nervous because this effectively means that the code that roslyn is generating is no longer compatible with EF6 whenever any of the entities subclass a generic class (for example when using the Identity Framework, as in my example) and the application we have in development relies on EF6 because EF7 is not (yet) a full replacement. I also don't think it's reasonable to assume there will be an update to EF6 that addresses this issue from its side.

I am going to attempt to switch over to the daily dnx builds to see if these merges have made it in and will report back if it fixes the problem. Does anybody know if these fixes are actually currently in the daily DNX drops for rc1?

Original stack trace:

Microsoft.AspNet.Diagnostics.DeveloperExceptionPageMiddleware] An unhandled exception has occurred while executing the request
System.NotSupportedException: Unable to cast the type 'MyProduct.DbModelAssembly.Contexts.ApplicationUser' to type 'MyProduct.DbModelAssembly.Identity.IdentityUser`4[[System.Guid, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[MyProduct.DbModelAssembly.Models.GuidUserLogin, MyProduct.DbModelAssembly, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[MyProduct.DbModelAssembly.Models.GuidUserRole, MyProduct.DbModelAssembly, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[MyProduct.DbModelAssembly.Models.GuidUserClaim, MyProduct.DbModelAssembly, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]'. LINQ to Entities only supports casting EDM primitive or enumeration types.
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.ValidateAndAdjustCastTypes(TypeUsage toType, TypeUsage fromType, Type toClrType, Type fromClrType)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.GetCastTargetType(TypeUsage fromType, Type toClrType, Type fromClrType, Boolean preserveCastForDateTime)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.CreateCastExpression(DbExpression source, Type toClrType, Type fromClrType)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.ConvertTranslator.TranslateUnary(ExpressionConverter parent, UnaryExpression unary, DbExpression operand)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.UnaryTranslator.TypedTranslate(ExpressionConverter parent, UnaryExpression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TypedTranslator`1.Translate(ExpressionConverter parent, Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateExpression(Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.MemberAccessTranslator.TypedTranslate(ExpressionConverter parent, MemberExpression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TypedTranslator`1.Translate(ExpressionConverter parent, Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateExpression(Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.EqualsTranslator.TypedTranslate(ExpressionConverter parent, BinaryExpression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TypedTranslator`1.Translate(ExpressionConverter parent, Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateExpression(Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateLambda(LambdaExpression lambda, DbExpression input)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateLambda(LambdaExpression lambda, DbExpression input, DbExpressionBinding& binding)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.MethodCallTranslator.OneLambdaTranslator.Translate(ExpressionConverter parent, MethodCallExpression call, DbExpression& source, DbExpressionBinding& sourceBinding, DbExpression& lambda)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.MethodCallTranslator.FirstPredicateTranslatorBase.Translate(ExpressionConverter parent, MethodCallExpression call)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.MethodCallTranslator.SequenceMethodTranslator.Translate(ExpressionConverter parent, MethodCallExpression call, SequenceMethod sequenceMethod)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.MethodCallTranslator.TypedTranslate(ExpressionConverter parent, MethodCallExpression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TypedTranslator`1.Translate(ExpressionConverter parent, Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.TranslateExpression(Expression linq)
   at System.Data.Entity.Core.Objects.ELinq.ExpressionConverter.Convert()
   at System.Data.Entity.Core.Objects.ELinq.ELinqQueryState.GetExecutionPlan(Nullable`1 forMergeOption)
   at System.Data.Entity.Core.Objects.ObjectQuery`1.<>c__DisplayClassc.<GetResultsAsync>b__a()
   at System.Data.Entity.Core.Objects.ObjectContext.<ExecuteInTransactionAsync>d__3d`1.MoveNext()

[cherrydev]

So, ironically, in a completely unrelated matter I already requested of the EF6 team that they support upcasting in expressions and they have decided not to implement it. Issue is here. Since I believe roslyn is now effectively including an upcast where there was not one before, this situation seems to apply as well.

[cherrydev]

Okay, it seems like there are no actual nightly builds published anywhere for this package, so upgrading to the RC1 nightly build of DNX doesn't help.
@VSadov - Is there any word on this? Will this be in the RC1 release? Will there be some build on some feed before then that implements this fix? Has this been confirmed to fix the problems that both @TylerWoods and I have reported within dotnet/efcore#3267? Is there anything I can do to help make sure this explicitly gets tested for?

I'm fine with reverting to beta7 until RC1 gets fixed, but my anxiety is that it won't end up actually fixing the specific problem that we're encountering, since it's revealed only by an intersection of this version of Roslyn, EF6, interfaces and generics.

[VSadov]

The change should be in VS Update1. I am not sure about exact dates.

There are nightly builds on https://www.myget.org/gallery/roslyn-nightly
You can try installing Microsoft.Net.Compilers in a sample project. That should override the default compiler when you build and you should see if that fixes your problem.

[cherrydev]

Okay, after discovering there's another required repo in order to use the nightly roslyn (required for System.Reflection Metadata 1.1.0-beta-23413) at https://www.myget.org/gallery/aspnetcidev/ I tried this out with no apparently change.

InvalidOperationException: The properties expression 'u => Convert(u).Id' is not valid. The expression should represent a property: C#: 't => t.MyProperty' VB.Net: 'Function(t) t.MyProperty'. When specifying multiple properties use an anonymous type: C#: 't => new { t.MyProperty1, t.MyProperty2 }' VB.Net: 'Function(t) New With { t.MyProperty1, t.MyProperty2 }'.

Doing a > dnu build after a > dnu restore provides the following output, which indicates that the nightly build of roslyn is being used as a dependency.

https://gist.github.com/cherrydev/79641884721128a3951b

If you are certain that simply adding daily version of Microsoft.Net.Compilers as a dependency of my project will actually cause the expressions to be compiled with that version of the compiler, than the change made in regards to this issue does NOT fix the problem and that roslyn is still not producing expressions that EF6 can consume.

HOWEVER, I can see by looking in the .dll files from Microsoft.Dnx.Compilation.CSharp.dll that comes with DNX beta-8 that it is built with a reference to Microsoft.CodeAnalysis.CSharp 1.1.0.0, whereas the nightlies are a 1.2.0 beta. I don't actually have a deep enough understanding of the toolchain at the moment to know if the version of the compiler linked to by the runtime by the dnx command is the one that will be used or if it's the version that's specified in the project.json file.

[cherrydev]

I'm tentatively convinced that the nightly build of the compiler is actually being used since when I clear out my .dnx/packages of those packages, only the nightly version is actually in existence when my project is being run.

So, that's a pretty good indication that there's still a major incompatibility between the current roslyn release and EF6. What's the next step? I should make a sample project that reproduces the problem and post the link to it here?

[VSadov]

@cherrydev - yes, please post a simple repro here. It would make it easier to figure what is going on. It is also possible there is another difference, that we did not notice.

[cherrydev]

I'll do my best to make it as simple as possible, but since it's working with EF6 and there are at least two different places in the EF6 (that I've found so far) that choke on the expressions produced by the 1.1/1.2 compiler, at the very least the sample project will require LocalDB installed to run.

[cherrydev]

Never mind! This is trivial to reproduce, as long as you trust me that having the extra Convert() where it is breaks EF6. This seems to happen when you have any class A and any class X<TType> where TType : A and inside X<TType> define an expression that has <TType> as a lambda parameter.

This code passes on beta6 and beta7 and fails on beta8 and rc1-15838, with or without Microsoft.Net.Compilers 1.2.0-beta-20151022-05 as a dependency.

using System;
using System.Linq.Expressions;

namespace ImplicitRoslynConversion
{
    public class Program
    {
        public void Main(string[] args)
        {
            var e = new GenericService<Entity>().MakeExpression();
            Expression<Func<Entity, string>> e1 = (Expression<Func<Entity, string>>) e;
            if (((MemberExpression) e1.Body).Expression.NodeType == ExpressionType.Convert)
            {
                Console.WriteLine("Extraneous cast detected.  Is this running on beta-8 or higher?");
            }
            else
            {
                Console.WriteLine("A-okay");
            }
            Console.WriteLine("Press enter to exit");
            Console.ReadLine();
        }
    }

    public class GenericService<TEntity> where TEntity : Entity
    {
        public Expression MakeExpression()
        {
            Expression<Func<TEntity, string>> e = b => b.Key;
            return e;
        }
    }

    public class Entity
    {
        public string Key { get; set; }
    }
}

[cherrydev]

One more thing: Keeping the runtime at beta6 and adding the Microsoft.Net.Compilers 1.2.0-beta-20151022-05 dependency does NOT result in failure, as I would expect it to if adding the compiler to the dependencies would override the compiler being used by the runtime. Each runtime includes its own version of Microsoft.CodeAnalysis and Microsoft.CodeAnalysis.CSharp which seems incompatible with the 1.2.0-beta version (for example, if I place the Microsoft.CodeAnalysis.CSharp dll in the runtime directory).

So, at this point, I have no idea if the above is a valid test, or of any method available to me to create a valid test without somehow building my own runtime that includes the 1.2.0-beta CodeAnalysis assemblies linked (which is beyond my capabilities; I tried).

@VSadov, if you can think of anything else I can do to help move this issue ahead, please let me know. This seems like a make-or-break issue to my project so I will do anything in my capability to get it resolved.

[divega]

@cherrydev FYI, I tried your repro above using a recent build of DNX RC1 and the expression didn't contain the extraneous convert expression.

Microsoft.CodeAnalysis.CSharp.dll product version is 1.1.0.51014.

Does that help?

[cherrydev]

It looks like it got changed between rc1-15838 and rc1-16048. Thanks a lot for pointing this out, that's a relief.

For those of you that CAN'T use the dailies (I can't because I rely on a library that doesn't include the massive Microsoft.Framework -> Microsoft.Extensions rename) but need to use EF6 with beta8, you're welcome to use my fork of EF6 that includes a work-around:

https://entityframework.codeplex.com/SourceControl/network/forks/acherryresilience/Bug2846

[divega]

Another piece of good news: Visual Studio 2015 Update 1 RC released a few days ago also includes the version of the C# compiler with the fix.

[cherrydev]

Right, but for DNX projects, the compiler that comes with the DNX runtime version that's listed in global.json is the one that's used to compile the actual project when it's run, not the one that comes with Visual Studio, so unless I'm using DNX RC1-16048 as the runtime, that doesn't help, right? I could have that totally wrong, in which case I'm totally confused about how Visual Studio and DNX interact with each other.

[staff0rd]

Ran into this just today also. Gave your fork a shot @cherrydev but ran into further problems because it can't find EntityFramework.resources (which is clearly in the repo), but also beacause I'm using npgsql. Guess I'll just stick to beta7 and wait for rc1 at this stage.

Also, I installed the VS2015 Update 1 RC, but as you suggest this is irrelevant when executing the project via DNX.

[cherrydev]

Oh well. I'm not sure why npgsql should make a difference, but you could try getting it to build without running tests or anything with build.cmd /t:package. You will still need to turn on strong name verification for that assembly to get it to work, of course.

[staff0rd]

Updating my codebase to use rc1-16048 is a fix for this as suggested, however I've got my own problems with rc1.

@cherrydev I managed to properly link to your EF6 update, however I was getting a NotSupportedException: LINQ to Entities only supports casting EDM primitive or enumeration types when calling UserManager.CreateAsync due to it being unable to convert across ApplicationUser : IdentityUser inheritence.

I had to make this change to make it work for me. Otherwise, thanks!

[HellBrick]

This is somewhat ironic, but I've got broken today by this change that was supposed to be introduced for compat reasons =) The new version of Roslyn used by VS 2015 update 1 emits the additional cast in the case when neither VS 2015 RTM nor VS 2013 emitted it before, and DataStax Cassandra driver apparently wasn't prepared to handle it.

I'm not sure what would be the right thing to do here, but introducing the cast to avoid breaking existing code while breaking other existing code that relied on the cast not being there sounds a bit weird. This is probably not a big deal, since the issue can be worked around by adding where T : class to the constraint (which, by the way, seems impossible to figure out without stumbling upon this issue), but I thought it's still worth mentioning.