[PATCH] D43563: TableGen: Get rid of Init::getFieldInit

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added inline comments.


================
Comment at: lib/TableGen/Record.cpp:1377
 Init *FieldInit::resolveReferences(Record &R, const RecordVal *RV) const {
-  Init *NewRec = RV ? Rec->resolveReferences(R, RV) : Rec;
+  Init *NewRec = Rec->resolveReferences(R, RV);
 
----------------
tra wrote:
> assert(RV) as we no longer allow it to be nullptr.
Actually no, this is on purpose. resolveReferences works fine with RV == nullptr, it just has subtly different semantics (resolving via R.getValue() instead of resolving only RV). All the other recursive resolving calls look like this as well, e.g. when resolving the list in a VarListElementInit or operands of OpInit subclasses. The whole point of this change is to streamline the resolving process in FieldInit as well.

I haven't looked into the full history of this, but I suspect somebody mistakenly decided to skip the recursive resolveReferences call in the !RV call, and then somebody else added the special code in VarInit::getFieldInit to fix one particular case that ended up being broken by the lack of resolve calls.

Whatever the history, the Right Thing to do is for all composite Inits to recursively resolve their components in a standardized way. I have an upcoming change that does a similar thing with BitsInit.


Repository:
  rL LLVM

https://reviews.llvm.org/D43563