C# - closures over class fields inside an initializer?

Question

Consider the following code:

using System;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            var square = new Square(4);
            Console.WriteLine(square.Calculate());
        }
    }

    class MathOp
    {        
        protected MathOp(Func<int> calc) { _calc = calc; }
        public int Calculate() { return _calc(); }
        private Func<int> _calc;
    }

    class Square : MathOp
    {
        public Square(int operand)
            : base(() => _operand * _operand)  // runtime exception
        {
            _operand = operand;
        }

        private int _operand;
    }
}

(ignore the class design; I'm not actually writing a calculator! this code merely represents a minimal repro for a much bigger problem that took awhile to narrow down)

I would expect it to either:

• print "16", OR
• throw a compile time error if closing over a member field is not allowed in this scenario

Instead I get a nonsensical exception thrown at the indicated line. On the 3.0 CLR it's a NullReferenceException; on the Silverlight CLR it's the infamous Operation could destabilize the runtime.

Answer 1

It was a compiler bug that has been fixed. The code should never have been legal in the first place, and if we were going to allow it, we should have at least generated valid code. My bad. Sorry about the inconvenience.

Answer 2

It's not going to result in a compile-time error because it is a valid closure.

The problem is that this is not initialized yet at the time the closure is created. Your constructor hasn't actually run yet when that argument is supplied. So the resulting NullReferenceException is actually quite logical. It's this that's null!

I'll prove it to you. Let's rewrite the code this way:

class Program
{
    static void Main(string[] args)
    {
        var test = new DerivedTest();
        object o = test.Func();
        Console.WriteLine(o == null);
        Console.ReadLine();
    }
}

class BaseTest
{
    public BaseTest(Func<object> func)
    {
        this.Func = func;
    }

    public Func<object> Func { get; private set; }
}

class DerivedTest : BaseTest
{
    public DerivedTest() : base(() => this)
    {
    }
}

Guess what this prints? Yep, it's true, the closure returns null because this is not initialized when it executes.

Edit

I was curious about Thomas's statement, thinking that maybe they'd changed the behaviour in a subsequent VS release. I actually found a Microsoft Connect issue about this very thing. It was closed as "won't fix." Odd.

As Microsoft says in their response, it is normally invalid to use the this reference from within the argument list of a base constructor call; the reference simply does not exist at that point in time and you will actually get a compile-time error if you try to use it "naked." So, arguably it should produce a compile error for the closure case, but the this reference is hidden from the compiler, which (at least in VS 2008) would have to know to look for it inside the closure in order to prevent people from doing this. It doesn't, which is why you end up with this behaviour.

Answer 3

How about this:

using System;
using System.Linq.Expressions;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            var square = new Square(4);
            Console.WriteLine(square.Calculate());
        }
    }

    class MathOp
    {
        protected MathOp(Expression<Func<int>> calc) { _calc = calc.Compile(); }
        public int Calculate() { return _calc(); }
        private Func<int> _calc;
    }

    class Square : MathOp
    {
        public Square(int operand)
            : base(() => _operand * _operand)
        {
            _operand = operand;
        }

        private int _operand;
    }
}

Answer 4

Have you tried using () => operand * operand instead? The issue is that there's no certainty that _operand will be set by the time you call the base. Yes, it's trying to create a closure on your method, and there's no guarantee of the order of things here.

Since you're not setting _operand at all, I'd recommend just using () => operand * operand instead.