assign and bind to deal with different const of functions

[AdityaAtulTewari]

assign is used when the function is const wrt to the first level of the referenced object.
bind is it's counterpart.

Both should only be used in very isolated cases.

[AdityaAtulTewari]

@ywwu928 please make sure this doesn't do extra stores.

[AdityaAtulTewari]

• Add an assert to applyFunc to check that the object structure actually has not changed. Should be a byte level comparison of the original and the final.

[aneeshdurg]

Would it be beneficial to introduce a wrapper around references to arrays that has a const ref, but uses const_cast to allow interior mutability? Something like this:

template <typename T>
class ConstVecWrapper {
public:
  ConstVecWrapper(std::vector<T>& input) : ref(input) {}

  T& operator[](size_t idx) {
    return const_cast<std::vector<T>&>(ref)[idx];
  }

private:
  const std::vector<T>& ref;
}

Using a wrapper like this would allow the compiler to prevent any mutation of the input reference, and you'd use const_cast as a sort of "unsafe" marker to say that you're explicitly ok with the access there.

[AdityaAtulTewari]

I think that approach makes sense. However, the goal is to completely reduce the burden on the programmer for the provided methods. If the programmer has to keep track of that how is it different from knowing when to use applyFunc?

[aneeshdurg]

The idea is that by having some kind of class like this wrap the reference to the array most users don't need to know about how the const semantics are handled internally - they just have a reference that only exposes interior mutability. I think the risk with applyFunc as it is now is that if the function does mutate the outer object it could get silently lost right?

[AdityaAtulTewari]

Correct this would loose the internal mutations. How about you write out an example use case of what you are saying and we examine that.

[aneeshdurg]

I don't think this should replace applyFunc, it's there to ensure that applyFunc isn't called with a functor that treats the input as having exterior mutability with no extra cost.

// let's say we have a function f that attempts to do something that mutates the ptr itself
void f(GlobalRef<T> ref) {
  ref->size += 1; // contrived example, but whatever
}
// If I call the applyFunc with f, my understanding is that the write to ref->size won't persist
// what if applyFunc was defined as follows:
void applyFunc(ConstWrapper<T> ref, Func f) {
  ...
}

// When calling applyFunc if I just did...
GlobalRef<T> ref;
ConstVecWrapper<T> mut_ref(ref); // (this isn't necessary I think you can make it implicit cast)
applyFunc(ref, f); // ...I'll get a compiler error about the type of f

// If I just naively refactor this to...
void f(ConstWrapper<T> ref) {
  ref->size += 1; // ...now this will fail to compile because `ref->size` will have type `const size_t&`
  // Only operator[] allows mutable access

  // now I know that f shouldn't be called via `applyFunc` but with `bindFunc`/`fmap` 
}

[AdityaAtulTewari]

Separate Idea: At that point why not just have two impls of bind. One with a ConstWrapper and one without?

Let's consider a more involved example. Because I think what you are saying is a good idea.

template<typename VT, typename ET>
struct CSR {
  Array<uint64_t> edgeIndexes;
  Array<uint64_t> edgeDsts;
  Array<VT> vertexData;
  Array<ET> edgeData;
  uint64_t numVertices;
  uint64_t numEdges;
};

Very obviously when accessing this data structure certain things (for example those wrapped in arrays) can be made mutable without any cost. Perhaps we can implicitly promote things like Arrays and other non-resizeable DS as automatically ConstWrapped. For example calling get on a hashmap should be ConstWrapped that function itself should have that property.

It would be much nicer to be able to do this with two overloaded versions of bind:

void f(const CSR& graph) {
   graph.vertexData[0] = 5;
}

void g(CSR graph) {
 graph.numVertices ++;
}

globalRef<CSR> csr = distGraph.getLocalCSR();
bind(csr, f); // No writeback
bind(csr, g); // Yes writeback

I think what we are saying is the same, just instead of using mutable, you want to use an explicit separate type.

[aneeshdurg]

Thar definitely makes sense, but it wouldn't work for the case where we want to be able to mutate array elements right?
I think having an overload for const would be a good solution for the time being though

[AdityaAtulTewari]

We will merge this for now and I'll update on llvm patches in today's meeting.