Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Are code contracts guaranteed to be evaluated before chained constructors are called?

Before I started using Code Contracts I sometimes ran into fiddlyness relating to parameter validation when using constructor chaining.

This is easiest to explain with a (contrived) example:

class Test
{
    public Test(int i)
    {
        if (i == 0)
            throw new ArgumentOutOfRangeException("i", i, "i can't be 0");
    }

    public Test(string s): this(int.Parse(s))
    {
        if (s == null)
            throw new ArgumentNullException("s");
    }
}

I want the Test(string) constructor to chain the Test(int) constructor, and to do so I use int.Parse().

Of course, int.Parse() doesn't like having a null argument, so if s is null it will throw before I reach the validation lines:

if (s == null)
    throw new ArgumentNullException("s");

which renders that check useless.

How to fix that? Well, I sometimes used to do this:

class Test
{
    public Test(int i)
    {
        if (i == 0)
            throw new ArgumentOutOfRangeException("i", i, "i can't be 0");
    }

    public Test(string s): this(convertArg(s))
    {
    }

    static int convertArg(string s)
    {
        if (s == null)
            throw new ArgumentNullException("s");

        return int.Parse(s);
    }
}

That's a bit fiddly, and the stack trace isn't ideal when it fails, but it works.

Now, along come Code Contracts, so I start using them:

class Test
{
    public Test(int i)
    {
        Contract.Requires(i != 0);
    }

    public Test(string s): this(convertArg(s))
    {
    }

    static int convertArg(string s)
    {
        Contract.Requires(s != null);
        return int.Parse(s);
    }
}

All well and good. It works fine. But then I discover that I can do this:

class Test
{
    public Test(int i)
    {
        Contract.Requires(i != 0);
    }

    public Test(string s): this(int.Parse(s))
    {
        // This line is executed before this(int.Parse(s))
        Contract.Requires(s != null);
    }
}

And then if I do var test = new Test(null), the Contract.Requires(s != null) is executed before this(int.Parse(s)). This means that I can do away with the convertArg() test altogether!

So, on to my actual questions:

  • Is this behaviour documented anywhere?
  • Can I rely on this behaviour when writing code contracts for chained constructors like this?
  • Is there some other way I should be approaching this?
like image 879
Matthew Watson Avatar asked Jun 12 '13 13:06

Matthew Watson


1 Answers

The short answer

Yes, the behavior is documented in the definition of "precondition", and in how legacy verification (if/then/throw) without a call to Contract.EndContractBlock is handled.

If you don't want to use Contract.Requires, you can change your constructor to

public Test(string s): this(int.Parse(s))
{
    if (s == null)
        throw new ArgumentNullException("s");
    Contract.EndContractBlock();
}

The long answer

When you place a Contract.* call in your code, you are not actually calling a member in the System.Diagnostics.Contracts namespace. For example, Contract.Requires(bool) is defined as:

[Conditional("CONTRACTS_FULL")]
public static void Requires(bool condition) 
{
    AssertMustUseRewriter(ContractFailureKind.Precondition, "Requires"); 
}

AssertMustUseRewriter unconditionally throws a ContractException, so in absence of rewriting the compiled binary, the code will just crash if CONTRACTS_FULL is defined. If it is not defined, the pre-condition is never even checked, as the call to Requires is omitted by the C# compiler due to the presence of the [Conditional] attribute.

The Rewriter

Based off of the settings selected in the project properties, Visual Studio will define CONTRACTS_FULL and call ccrewrite to generate the appropriate IL to check the contracts at runtime.

Example contract:

private string NullCoalesce(string input)
{
    Contract.Requires(input != "");
    Contract.Ensures(Contract.Result<string>() != null);

    if (input == null)
        return "";
    return input;
}

Compiled with csc program.cs /out:nocontract.dll, you get:

private string NullCoalesce(string input)
{
    if (input == null)
        return "";
    return input;
}

Compiled with csc program.cs /define:CONTRACTS_FULL /out:prerewrite.dll and run through ccrewrite -assembly prerewrite.dll -out postrewrite.dll you will get the code which will actually perform runtime checking:

private string NullCoalesce(string input)
{
    __ContractRuntime.Requires(input != "", null, null);
    string result;
    if (input == null)
    {
        result = "";
    }
    else
    {
        result = input;
    }
    __ContractRuntime.Ensures(result != null, null, null);
    return input;
}

Of prime interest is that our Ensures (a postcondition) got moved to the bottom of the method, and our Requires (a precondition) didn't really move since it was already at the top of the method.

This fits with the documentation's definition:

[Preconditions] are contracts on the state of the world when a method is invoked.
...
Postconditions are contracts on the state of a method when it terminates. In other words, the condition is checked just prior to exiting a method.

Now, the complexity in your scenario exists in the very definition of a precondition. Based off of the definition listed above, the precondition runs before the method runs. The problem is that the C# specification says that the constructor initializer (chained constructor) must be invoked immediately before the constructor-body [CSHARP 10.11.1], which is at odds with the definition of a precondition.

Magic lives here

The code that ccrewrite generates cannot therefore be represented as C#, as the language provides no mechanism to run code before the chained constructor (except by calling static methods in the chained constructor parameter list as you mention). ccrewrite, as required by the definition takes your constructor

public Test(string s)
    : this(int.Parse(s))
{
    Contract.Requires(s != null);
}

which gets compiled as

the MSIL of the compiled code above

and moves the call to requires to before the call to the chained constructor:

the msil of the code above passed through the contract rewriter

Which means...

The way to avoid having to resort to static methods doing argument validation is to use the contract rewriter. You can invoke the rewriter by using Contract.Requires, or by signifying that a block of code is a precondition by ending it with Contract.EndContractBlock();. Doing so will cause the rewriter to place it at the start of the method, before the call to the constructor initializer.

like image 130
Mitch Avatar answered Sep 27 '22 21:09

Mitch