2
Vote

LiftedToNull OrElse bug for user types

description

The attached test case, (extracted from the tests cases I wrote when implementing the 3.5 System.Linq.Expressions in Mono), fails on .net 4.0 RC and with the latest DLR checkout, while it works on 3.5.

file attachments

comments

jmesserly wrote Apr 20, 2010 at 11:18 PM

There were numerous bug fixes in 4.0 to lifted and userdefined AndAlso/OrElse codegen. The example illustrated is using nullable-lifted and userdefined OrElse. Trees like this are only produced by VB, and we now match VB behavior. It's documented in the expression tree spec: http://dlr.codeplex.com/Project/Download/FileDownload.aspx?DownloadId=85661

jmesserly wrote Apr 20, 2010 at 11:56 PM

Did some more digging, I think this one might be a regression that was an unintended consequence of sharing code between AndAlso and OrElse. We'll have to look at this one and see if it is indeed a bug, and if so see if it can be fixed next for the next framework release.

jbevain wrote Apr 22, 2010 at 11:02 AM

Yes, this is a genuine bug. OrElse is not correct, while AndAlso is.

Here's the patch I'm using that passes my test suite:

http://gist.github.com/375076

It's not pretty as testBlock is repeated twice for OrElse, but it works.

jbevain wrote Apr 22, 2010 at 11:06 AM

Note that the patch is just a proof of concept to show the issue, in itself, it's wrong as it will evaluate the rhs twice for OrElse. Not that I think that you're allowed to take patches, but still.

jbevain wrote Apr 22, 2010 at 11:33 AM

Scratch my last one, the rhs won't be evaluated twice with the patch. It's just emitted twice in the IL though.

dinov wrote Apr 30, 2010 at 8:24 PM

Tracked internally as bug #892019

rdawson wrote Aug 6, 2010 at 8:05 PM

Thanks for reporting this. We definitely are behaving differently than we did in .NET 3.5, and it was good to revisit the design discussions that led to the new spec which JMesserly referred to in his original reply.

Having had the discussions again, we favor the 4.0 behavior over the 3.5 behavior. Matching VB was one of our goals in refactoring this code, and we do now. (3.5 did not.)