Impure method is called for readonly field

Question

I'm using Visual Studio 2010 + ReSharper and it shows a warning on the following code:

if (rect.Contains(point))
{
    ...
}

rect is a readonly Rectangle field, and ReSharper shows me this warning:

"Impure Method is called for readonly field of value type."

What are impure methods and why is this warning being shown to me?

Answer 1

First off, Jon, Michael and Jared's answers are essentially correct but I have a few more things I'd like to add to them.

What is meant by an "impure" method?

It is easier to characterize pure methods. A "pure" method has the following characteristics:

• Its output is entirely determined by its input; its output does not depend on externalities like the time of day or the bits on your hard disk. Its output does not depend on its history; calling the method with a given argument twice should give the same result.
• A pure method produces no observable mutations in the world around it. A pure method may choose to mutate private state for efficiency's sake, but a pure method does not, say, mutate a field of its argument.

For example, Math.Cos is a pure method. Its output depends only on its input, and the input is not changed by the call.

An impure method is a method which is not pure.

What are some of the dangers of passing readonly structs to impure methods?

There are two that come to mind. The first is the one pointed out by Jon, Michael and Jared, and this is the one that ReSharper is warning you about. When you call a method on a struct, we always pass a reference to the variable that is the receiver, in case the method wishes to mutate the variable.

So what if you call such a method on a value, rather than a variable? In that case, we make a temporary variable, copy the value into it, and pass a reference to the variable.

A readonly variable is considered a value, because it cannot be mutated outside the constructor. So we are copying the variable to another variable, and the impure method is possibly mutating the copy, when you intend it to mutate the variable.

That's the danger of passing a readonly struct as a receiver. There is also a danger of passing a struct that contains a readonly field. A struct that contains a readonly field is a common practice, but it is essentially writing a cheque that the type system does not have the funds to cash; the "read-only-ness" of a particular variable is determined by the owner of the storage. An instance of a reference type "owns" its own storage, but an instance of a value type does not!

struct S
{
  private readonly int x;
  public S(int x) { this.x = x; }
  public void Badness(ref S s)
  {
    Console.WriteLine(this.x);   
    s = new S(this.x + 1);
    // This should be the same, right?
    Console.WriteLine(this.x);   
  }
}

One thinks that this.x is not going to change because x is a readonly field and Badness is not a constructor. But...

S s = new S(1);
s.Badness(ref s);

... clearly demonstrates the falsity of that. this and s refer to the same variable, and that variable is not readonly!

Answer 2

An impure method is one which isn't guaranteed to leave the value as it was.

In .NET 4, you can decorate methods and types with [Pure] to declare them to be pure, and R# will take notice of this. Unfortunately, you can't apply it to someone else's members, and you can't convince R# that a type/member is pure in a .NET 3.5 project as far as I'm aware. (This bites me in Noda Time all the time.)

The idea is that if you're calling a method which mutates a variable, but you call it on a read-only field, it's probably not doing what you want, so R# will warn you about this. For example:

public struct Nasty
{
    public int value;

    public void SetValue()
    {
        value = 10;
    }
}

class Test
{
    static readonly Nasty first;
    static Nasty second;

    static void Main()
    {
        first.SetValue();
        second.SetValue();
        Console.WriteLine(first.value);  // 0
        Console.WriteLine(second.value); // 10
    }
}

This would be a really useful warning if every method which was actually pure was declared that way. Unfortunately they're not, so there are a lot of false positives :(

Answer 3

The short answer is that this is a false positive, and you can safely ignore the warning.

The longer answer is that accessing a read-only value type creates a copy of it, so that any changes to the value made by a method would only affect the copy. ReSharper doesn't realize that Contains is a pure method (meaning it has no side effects). Eric Lippert talks about it here: Mutating Readonly Structs

Answer 4

It sounds like ReSharper believes that the method Contains can mutate the rect value. Because rect is a readonly struct, the C# compiler makes defensive copies of the value to prevent the method from mutating a readonly field. Essentially, the final code looks like this:

Rectangle temp = rect;
if (temp.Contains(point)) {
  ...
}

ReSharper is warning you here that Contains may mutate rect in a way that would be immediately lost because it happened on a temporary.

Answer 5

An Impure method is a method that could have side-effects. In this case, ReSharper seems to think it could change rect. It probably doesn't but the chain of evidence is broken.