[PATCH] D17407: [Sema] PR25755 Fix crash when initializing out-of-order struct references

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Sema/SemaInit.cpp:1818-1823
@@ +1817,8 @@
+  RecordDecl::field_iterator FieldStart = Field;
+  unsigned FieldSize = [FieldStart, FieldEnd]() mutable -> unsigned {
+    unsigned i = 0;
+    for (; FieldStart != FieldEnd; ++FieldStart, ++i)
+      /*empty*/;
+    return i;
+  }();
+  llvm::BitVector SeenFields(FieldSize);
----------------
`FieldStart` isn't necessarily the first field (in particular, if we got here from a nested field designator). Instead, how about using

  FieldSize = std::distance(RD->field_begin(), RD->field_end());

================
Comment at: lib/Sema/SemaInit.cpp:1922
@@ +1921,3 @@
+  if (VerifyOnly && !DeclType->isUnionType() &&
+      (Field != FieldEnd || !CheckForMissingFields)) {
+    Field = FieldStart;
----------------
The `CheckForMissingFields` test here doesn't make much sense with this name. Please reverse the sense of this flag, rename it to `HadAnyDesignators` or similar, and initialize it to `true` if `Field != RD->field_begin()` before you enter the field initialization loop.

================
Comment at: lib/Sema/SemaInit.cpp:1923
@@ +1922,3 @@
+      (Field != FieldEnd || !CheckForMissingFields)) {
+    Field = FieldStart;
+    for (unsigned i = 0; i < FieldSize && !hadError; ++i, ++Field) {
----------------
Start from `RD->field_begin()` here (or track the position at which you first set `HadAnyDesignators` to `true` and start from there). In particular, if `FieldStart != RD->field_begin()`, then an initial sequence of fields is uninitialized unless a later designator jumped back to them.

================
Comment at: lib/Sema/SemaInit.cpp:1926-1927
@@ +1925,4 @@
+      assert(Field != FieldEnd);
+      if (SeenFields[i])
+        continue;
+      if (!Field->isUnnamedBitfield() && !Field->hasInClassInitializer() &&
----------------
Use `SeenFields[Field->getFieldIndex()]` here and then remove the variable `i`. We don't need to hardcode assumptions about how field indexes are computed here.


http://reviews.llvm.org/D17407