So now the Dict behavior is different from List or Tuple (both of which while inheriting from intrusive_ptr_target, are actually Impl's not the reference-like wrappers). They should behave the same.

But then there's the question of how they should behave - as regular C++ types (so you'd pass const Dict& and intrusive_ptr<Dict>) or like magical wrapper like you're proposing here and how at::Tensor is implemented.

I personally don't like the idea of magical wrappers proliferation as it's non-C++ and is pretty confusing. Tensor is kind of an exception, but in other cases, would it be so bad to explicitly write wrapper? (or we could define explicit DictPtr)

IValue stores Dicts as some kind of GenericDict (i.e. Dict<IValue, IValue>) but we want to pass concrete types to the kernel (i.e. Dict<Key, Value>). If Dict was a value type, we would need to copy or move move the contents from Dict<IValue, IValue> to the Dict<Key, Value>. Hopefully avoid the copy and do move, but that invalidates the Dict<IValue, IValue>. This is actually how it works currently (i.e. before this PR).

Unfortunately, this breaks with IValue because there might be multiple IValues holding a pointer to the same Dict<IValue, IValue>, so we cannot invalidate it and have to copy the contents from Dict<IValue, IValue> into Dict<Key, Value>. This is bad. The only way out I see is to make Dict<IValue, IValue> and Dict<Key, Value> share the underlying data and making Dict a reference type.

I'm open to alternative ideas.

Thanks for the detailed description, it wasn't clear from the PR description originally!

In that case indeed, whatever C++ type we pass into the kernel has to be "reference semantics" in C++. In that case - how about we just rename Dict in your PR into DictRef? It'd make it explicit to user that it's a reference and thus behaves differently from const vector&. I'm just worried that Dict x = y; is too different from regular C++ semantics. (note that Tensor is not a C++ class so it's ok to follow Python, but for dict, it's easy to be confused with unordered_map).

Dict<Key,Value> can also be returned from kernels. ...Ref types in my mind have the connotation of being non-owning, like ArrayRef. Returning an ArrayRef from a function would be deadly for example. DictPtr might be better, but that then somehow makes me think about nullability, which also isn't true for Dict. After doing DictPtr<Key, Value> var;, I expect to have to point it to some Dict and initialize the pointer before I can use it. Opinions?

@dzhulgakov proposed to call it DictPtr and remove the default constructor, adding a make_dict function to create dicts. This sounds reasonable to me. It's a bit more tedious to use, but it's much clearer what happens and harder to get wrong for users. I'll do that.

changes done. Please re-review.

@zdevito ArrayRef is non-owning, DictPtr is owning similar to shared_ptr or Tensor.

This pull request has been merged in d5b7138.