[TableGen] Resolve concrete but not complete field access initializers

[dsanders]

Summary

This fixes the resolution of Rec10.Zero in ListSlices.td.

As part of this, correct the definition of complete for ListInit such that
it's complete iff all the elements in the list are complete rather than
always being complete regardless of the elements. This is the reason
Rec10.TwoFive from ListSlices.td previously resolved despite being
incomplete like Rec10.Zero was

Depends on D100247

[dsanders]

llvm/lib/TableGen/Record.cpp

658

One thing to mention about this function. Downstream, our test for this uses our !iscomplete() predicate but I'm not aware of a good way to test this upstream as the FieldInit::Fold change means it no longer controls whether FieldInit's successfully resolve.

[Paul-C-Anagnostopoulos]

I presume you ran all the TableGen tests and they pass?

[dsanders]

Yep, ./bin/llvm-lit -svv ../llvm-project/llvm/test/TableGen and ninja check passes

[Paul-C-Anagnostopoulos]

I just pushed my bug fix. Let's wait a day until I know it doesn't bust the build, then push this one.

[Paul-C-Anagnostopoulos]

Oh, and LGTM.

[dsanders]

I just pushed my bug fix. Let's wait a day until I know it doesn't bust the build, then push this one.

Sounds good to me, thanks. I've also unreverted the original downstream and applied your fix there too.

[Paul-C-Anagnostopoulos]

Go for it!

llvm/include/llvm/TableGen/Record.h

public:
 
   /// This method is used by classes that refer to other
   /// variables which may not be defined at the time they expression is formed.
   /// If a value is set for the variable later, this method will be called on
   /// users of the value to allow the value to propagate out.
   ///
   Init *resolveReferences(Resolver &R) const override;
 
+  bool isComplete() const override;
   bool isConcrete() const override;
   std::string getAsString() const override;
 
   ArrayRef<Init*> getValues() const {
     return makeArrayRef(getTrailingObjects<Init *>(), NumValues);
   }
 
   const_iterator begin() const { return getTrailingObjects<Init *>(); }

llvm/lib/TableGen/Record.cpp

for (Init *CurElt : getValues()) {
     Resolved.push_back(E);
   }
 
   if (Changed)
     return ListInit::get(Resolved, getElementType());
   return const_cast<ListInit *>(this);
 }
 
+bool ListInit::isComplete() const {
+  for (Init *Element : *this) {
+    if (!Element->isComplete())
+      return false;
+  }
+  return true;
+}
+
 bool ListInit::isConcrete() const {
   for (Init *Element : *this) {
     if (!Element->isConcrete())
       return false;
   }
   return true;
 }
 
Init *FieldInit::Fold(Record *CurRec) const {
   if (DefInit *DI = dyn_cast<DefInit>(Rec)) {
     Record *Def = DI->getDef();
     if (Def == CurRec)
       PrintFatalError(CurRec->getLoc(),
                       Twine("Attempting to access field '") +
                       FieldName->getAsUnquotedString() + "' of '" +
                       Rec->getAsString() + "' is a forbidden self-reference");
     Init *FieldVal = Def->getValue(FieldName)->getValue();
-    if (FieldVal->isComplete())
+    if (FieldVal->isConcrete())
       return FieldVal;
   }
   return const_cast<FieldInit *>(this);
 }
 
 bool FieldInit::isConcrete() const {
   if (DefInit *DI = dyn_cast<DefInit>(Rec)) {
     Init *FieldVal = DI->getDef()->getValue(FieldName)->getValue();

llvm/test/TableGen/ListSlices.td

def Rec08 {
   list<string> ZooQuux = Array[1][0...1];
 }
 
 // Test uninitialized list elements.
 
 // CHECK: def Rec09
 // CHECK:   int Zero = ?;
 // CHECK:   list<int> TwoFive = [2, 3, ?, 5];
-// CHECK: def Rec10
-// CHECK:   list<int> TwoFive = [2, 3, ?, 5];
+// We need CHECK-NEXT for these because otherwise it will match anonymous defs
+// that appear later.
+// CHECK: def Rec10 {
+// CHECK-NEXT:   int Zero = ?;
+// CHECK-NEXT:   list<int> TwoFive = [2, 3, ?, 5];
 
 def Rec09 : Class1<[?, ?, 2, 3, ?, 5, ?]>;
 
 def Rec10 {
   int Zero = Class1<[?, ?, 2, 3, ?, 5, ?]>.Zero;
   list<int> TwoFive = Class1<[?, ?, 2, 3, ?, 5, ?]>.TwoFive;
 }
-
-// TO-DO: Notice that the first field in Rec10 is not checked. 
-// It is not fully resolved for reasons that need to be investigated.

llvm/test/TableGen/field-access-initializers.td

 // RUN: llvm-tblgen %s | FileCheck %s
 // XFAIL: vg_leak
 
+// CHECK: class B<A B:impl = ?> {
+// CHECK:   string value = B:impl.value;
+// CHECK: }
+
 // CHECK: --- Defs ---
 
 // CHECK: def A1 {
 // CHECK:   string value = ?;
 // CHECK: }
 
 // CHECK: def B1 {
-// CHECK:   string value = A1.value;
+// CHECK:   string value = ?;
 // CHECK: }
 
 class A {
   string value = ?;
 }
 
 class B<A impl> : A {
   let value = impl.value;
 }
 
 def A1 : A;
 def B1 : B<A1>;