Undefined behavior in DataDepGraph: nodes_ = (GraphNode **)insts_

[Quincunx271]

C++ does not have covariant array types. That line of code is equivalent to nodes_ = reinterpret_cast<GraphNode**>(insts_), which is undefined behavior. It happens to work in our case, though.

In C++, covariant array types are not a thing. insts_ has type SchedInstruction** and SchedInstruction inherits from GraphNode, but that doesn't mean this is okay. The fact that we had to reinterpret_cast makes it clear that this is not okay.

In fact, we should go through the code and remove all uses of C-style casts, as C-style casts are bad practice in C++ because they are one of several different casts, including a potential reinterpret_cast, and every sane cast can be done without a C-style cast.

---

In this case, the solution is to remove the nodes_ member variable entirely and instead directly access insts_.

[vangthao95]

I have only taken a quick look but if we can safely remove nodes_ and use insts_ instead then I agree we should remove it. Not sure why we have two pointers to essentially the same thing.

[Quincunx271]

It turns out this is less simple that I realized. nodes_ is in a parent class, namely DirAcycGraph, meaning we can't simply get rid of it, but must provide a way for the parent class to access it.

Although, DirAcycGraph's only subclass is DataDepGraph