Const-correctness

[ezyang]

ATen is presently has no const modifiers, even for methods which are obviously const (e.g. virtual bool isCuda() in Type). Make it more const-correct.

[ebetica]

On a separate note, I really don't think things like https://github.com/zdevito/ATen/blob/master/doc/Functions.h#L451 should be const in the output.

[gchanan]

Could all methods on Type be const (I haven't checked)? One thing that is more painful than necessary is passing around Types (that you know exist, so don't want the possible null allowed by pointer types); since they aren't const-correct, passing them by const reference doesn't work well; the next logical thing to try is by value, but that doesn't work either because they are abstract. So you end up passing by reference.

[zdevito]

Yes, I think all things on Type can be const.

[zdevito]

(Also, because Tensor is actually a shared pointer to the tensor implementation most things on Tensor can be const too! But it seems disingenuous to mark them so since 'const Tensor' is assumed to mean 'won't modify this tensor's data')

[ezyang]

Pointed out to me by @achernya: what makes things tricky is that there is really common confusion about the precedence of const in an expression like const int*. The correct parsing is not "this is a const (int pointer)" but "this is a pointer to a (const int)". And this is why int* const is the actual way you spell "const pointer to an int".

Now, the problem is that Tensor morally expands to pointer to a (non-const) TensorImpl. And so if you say const Tensor, now the precedence goes the way you expect: you have a const (pointer to a (non-const) TensorImpl), which is not at all the same thing as const TensorImpl*. But this makes all the difference: the correct semantics for const Tensor lets you still mutate the underlying TensorImpl: it's just the pointer itself that's const.

This also explains why the following idea doesn't work: what if we hid the (mutable) TensorImpl pointer as private data, and provided only methods which enforced const-correctness? You could, but you'd be forced to take out the copy constructor. Otherwise, you can write this:

Tensor unconst(const Tensor& x) {
  return x;
}

C++ will happily copy construct a non-const Tensor from a const one, because, hey, it's a copy constructor, it does a semantic copy, right? But Tensor's copy constructor does nothing of this kind.

[ezyang]

I'll also add, if we got rid of the copy constructor, then we could close this loophole, but I would assume giving up Tensor x = y.add(z) is too much to bear... (Another possibility: track constness dynamically, and raise an error if you call a non-const method when the dynamic constness is true.)

[zdevito]

Yes, the whole point of having Tensor rather than shared_ptr<Tensor> is to make sure the libraries API stays simple. It is intended to be directly usable as a lower-overhead way of accessing tensors compared to using a scripting language.

So I would caution against trying to get const-ness right at the cost of clarity or usability (for instance, adding a ConstTensor wrapper). All methods that modify a tensor are already annotated with _ or _out, so it is pretty clear what is going on from the API.

So far I've been accepting commits that improve usability because they make it possible to do things even though you ended up with something 'const Type &' due to C++ libraries or language behavior.