[MC] Avoid calling vector<FieldInfo> members before FieldInfo is defined

[ilya-biryukov]

Summary

The C++ Standard requires a complete type T when using any members of
vector<T>, see
https://eel.is/c++draft/vector#overview-4.

This only breaks with latest libc++ in C++20 mode and does not show up
in the common configurations.
We have an internal experimental configuration that discovered this.

[alexfh]

Looks good! Thanks for the fix!

[mstorsjo]

After this commit, GCC 9 produces warnings on this file:

../lib/MC/MCParser/MasmParser.cpp:227:1: warning: ‘{anonymous}::StructFieldInfo::StructFieldInfo(std::vector<{anonymous}::StructInitializer>&&, {anonymous}::StructInfo)’ defined but not used [-Wunused-function]
  227 | StructFieldInfo::StructFieldInfo(std::vector<StructInitializer> &&V,
      | ^~~~~~~~~~~~~~~

[ilya-biryukov]

This change did not really change anything in that regard, I'm not sure why this warning was suppressed before.
It's possible that moving the constructor out of the class body had an effect of making it non-inline, which had an effect on the warning, but I'm not sure.

Sent a small refactoring rG7c80c4d67716308dc0f382be88bbe73dd293892c that should fix it.

llvm/lib/MC/MCParser/MasmParser.cpp

struct StructInfo {
   unsigned Size = 0;
   std::vector<FieldInfo> Fields;
   StringMap<size_t> FieldsByName;
 
   FieldInfo &addField(StringRef FieldName, FieldType FT,
                       unsigned FieldAlignmentSize);
 
   StructInfo() = default;
-
-  StructInfo(StringRef StructName, bool Union, unsigned AlignmentValue)
-      : Name(StructName), IsUnion(Union), Alignment(AlignmentValue) {}
+  StructInfo(StringRef StructName, bool Union, unsigned AlignmentValue);
 };
 
 // FIXME: This should probably use a class hierarchy, raw pointers between the
 // objects, and dynamic type resolution instead of a union. On the other hand,
 // ownership then becomes much more complicated; the obvious thing would be to
 // use BumpPtrAllocator, but the lack of a destructor makes that messy.
 
 struct StructInitializer;
struct RealFieldInfo {
   RealFieldInfo(const SmallVector<APInt, 1> &V) { AsIntValues = V; }
   RealFieldInfo(SmallVector<APInt, 1> &&V) { AsIntValues = V; }
 };
 struct StructFieldInfo {
   std::vector<StructInitializer> Initializers;
   StructInfo Structure;
 
   StructFieldInfo() = default;
-  StructFieldInfo(const std::vector<StructInitializer> &V, StructInfo S) {
-    Initializers = V;
-    Structure = S;
-  }
-  StructFieldInfo(std::vector<StructInitializer> &&V, StructInfo S) {
-    Initializers = V;
-    Structure = S;
-  }
+  StructFieldInfo(const std::vector<StructInitializer> &V, StructInfo S);
+  StructFieldInfo(std::vector<StructInitializer> &&V, StructInfo S);
 };
 
 class FieldInitializer {
 public:
   FieldType FT;
   union {
     IntFieldInfo IntInfo;
     RealFieldInfo RealInfo;
     StructFieldInfo StructInfo;
   };
 
-  ~FieldInitializer() {
-    switch (FT) {
-    case FT_INTEGRAL:
-      IntInfo.~IntFieldInfo();
-      break;
-    case FT_REAL:
-      RealInfo.~RealFieldInfo();
-      break;
-    case FT_STRUCT:
-      StructInfo.~StructFieldInfo();
-      break;
-    }
-  }
+  ~FieldInitializer();
+  FieldInitializer(FieldType FT);
+
+  FieldInitializer(SmallVector<const MCExpr *, 1> &&Values);
+  FieldInitializer(SmallVector<APInt, 1> &&AsIntValues);
+  FieldInitializer(std::vector<StructInitializer> &&Initializers,
+                   struct StructInfo Structure);
+
+  FieldInitializer(const FieldInitializer &Initializer);
+  FieldInitializer(FieldInitializer &&Initializer);
+
+  FieldInitializer &operator=(const FieldInitializer &Initializer);
+  FieldInitializer &operator=(FieldInitializer &&Initializer);
+};
+
+struct StructInitializer {
+  std::vector<FieldInitializer> FieldInitializers;
+};
+
+struct FieldInfo {
+  // Offset of the field within the containing STRUCT.
+  unsigned Offset = 0;
+
+  // Total size of the field (= LengthOf * Type).
+  unsigned SizeOf = 0;
+
+  // Number of elements in the field (1 if scalar, >1 if an array).
+  unsigned LengthOf = 0;
+
+  // Size of a single entry in this field, in bytes ("type" in MASM standards).
+  unsigned Type = 0;
+
+  FieldInitializer Contents;
+
+  FieldInfo(FieldType FT) : Contents(FT) {}
+};
+
+StructFieldInfo::StructFieldInfo(const std::vector<StructInitializer> &V,
+                                 StructInfo S) {
+  Initializers = V;
+  Structure = S;
+}
+
+StructFieldInfo::StructFieldInfo(std::vector<StructInitializer> &&V,
+                                 StructInfo S) {
+  Initializers = V;
+  Structure = S;
+}
+
+StructInfo::StructInfo(StringRef StructName, bool Union,
+                       unsigned AlignmentValue)
+    : Name(StructName), IsUnion(Union), Alignment(AlignmentValue) {}
+
+FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT,
+                                unsigned FieldAlignmentSize) {
+  if (!FieldName.empty())
+    FieldsByName[FieldName.lower()] = Fields.size();
+  Fields.emplace_back(FT);
+  FieldInfo &Field = Fields.back();
+  Field.Offset =
+      llvm::alignTo(NextOffset, std::min(Alignment, FieldAlignmentSize));
+  if (!IsUnion) {
+    NextOffset = std::max(NextOffset, Field.Offset);
+  }
+  AlignmentSize = std::max(AlignmentSize, FieldAlignmentSize);
+  return Field;
+}
+
+FieldInitializer::~FieldInitializer() {
 
-  FieldInitializer(FieldType FT) : FT(FT) {
-    switch (FT) {
-    case FT_INTEGRAL:
-      new (&IntInfo) IntFieldInfo();
-      break;
-    case FT_REAL:
-      new (&RealInfo) RealFieldInfo();
-      break;
-    case FT_STRUCT:
-      new (&StructInfo) StructFieldInfo();
-      break;
-    }
-  }
+FieldInitializer::FieldInitializer(FieldType FT) : FT(FT) {
 
-  FieldInitializer(SmallVector<const MCExpr *, 1> &&Values) : FT(FT_INTEGRAL) {
-    new (&IntInfo) IntFieldInfo(Values);
-  }
+FieldInitializer::FieldInitializer(SmallVector<const MCExpr *, 1> &&Values)
+    : FT(FT_INTEGRAL) {
 
-  FieldInitializer(SmallVector<APInt, 1> &&AsIntValues) : FT(FT_REAL) {
-    new (&RealInfo) RealFieldInfo(AsIntValues);
-  }
+FieldInitializer::FieldInitializer(SmallVector<APInt, 1> &&AsIntValues)
+    : FT(FT_REAL) {
 
-  FieldInitializer(std::vector<StructInitializer> &&Initializers,
-                   struct StructInfo Structure)
-      : FT(FT_STRUCT) {
-    new (&StructInfo) StructFieldInfo(Initializers, Structure);
-  }
+FieldInitializer::FieldInitializer(
+    std::vector<StructInitializer> &&Initializers, struct StructInfo Structure)
 
-  FieldInitializer(const FieldInitializer &Initializer) : FT(Initializer.FT) {
-    switch (FT) {
-    case FT_INTEGRAL:
-      new (&IntInfo) IntFieldInfo(Initializer.IntInfo);
-      break;
-    case FT_REAL:
-      new (&RealInfo) RealFieldInfo(Initializer.RealInfo);
-      break;
-    case FT_STRUCT:
-      new (&StructInfo) StructFieldInfo(Initializer.StructInfo);
-      break;
-    }
-  }
+FieldInitializer::FieldInitializer(const FieldInitializer &Initializer)
+    : FT(Initializer.FT) {
 
-  FieldInitializer(FieldInitializer &&Initializer) : FT(Initializer.FT) {
-    switch (FT) {
-    case FT_INTEGRAL:
-      new (&IntInfo) IntFieldInfo(Initializer.IntInfo);
-      break;
-    case FT_REAL:
-      new (&RealInfo) RealFieldInfo(Initializer.RealInfo);
-      break;
-    case FT_STRUCT:
-      new (&StructInfo) StructFieldInfo(Initializer.StructInfo);
-      break;
-    }
-  }
+FieldInitializer::FieldInitializer(FieldInitializer &&Initializer)
+    : FT(Initializer.FT) {
 
-  FieldInitializer &operator=(const FieldInitializer &Initializer) {
-    if (FT != Initializer.FT) {
-      switch (FT) {
-      case FT_INTEGRAL:
-        IntInfo.~IntFieldInfo();
-        break;
-      case FT_REAL:
-        RealInfo.~RealFieldInfo();
-        break;
-      case FT_STRUCT:
-        StructInfo.~StructFieldInfo();
-        break;
-      }
-    }
-    FT = Initializer.FT;
-    switch (FT) {
-    case FT_INTEGRAL:
-      IntInfo = Initializer.IntInfo;
-      break;
-    case FT_REAL:
-      RealInfo = Initializer.RealInfo;
-      break;
-    case FT_STRUCT:
-      StructInfo = Initializer.StructInfo;
-      break;
-    }
-    return *this;
-  }
+FieldInitializer &
+FieldInitializer::operator=(const FieldInitializer &Initializer) {
 
-  FieldInitializer &operator=(FieldInitializer &&Initializer) {
-    if (FT != Initializer.FT) {
-      switch (FT) {
-      case FT_INTEGRAL:
-        IntInfo.~IntFieldInfo();
-        break;
-      case FT_REAL:
-        RealInfo.~RealFieldInfo();
-        break;
-      case FT_STRUCT:
-        StructInfo.~StructFieldInfo();
-        break;
-      }
-    }
-    FT = Initializer.FT;
-    switch (FT) {
-    case FT_INTEGRAL:
-      IntInfo = Initializer.IntInfo;
-      break;
-    case FT_REAL:
-      RealInfo = Initializer.RealInfo;
-      break;
-    case FT_STRUCT:
-      StructInfo = Initializer.StructInfo;
-      break;
-    }
-    return *this;
-  }
-};
-
-struct StructInitializer {
-  std::vector<FieldInitializer> FieldInitializers;
-};
-
-struct FieldInfo {
-  // Offset of the field within the containing STRUCT.
-  unsigned Offset = 0;
-
-  // Total size of the field (= LengthOf * Type).
-  unsigned SizeOf = 0;
-
-  // Number of elements in the field (1 if scalar, >1 if an array).
-  unsigned LengthOf = 0;
-
-  // Size of a single entry in this field, in bytes ("type" in MASM standards).
-  unsigned Type = 0;
-
-  FieldInitializer Contents;
-
-  FieldInfo(FieldType FT) : Contents(FT) {}
-};
-
-FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT,
-                                unsigned FieldAlignmentSize) {
-  if (!FieldName.empty())
-    FieldsByName[FieldName.lower()] = Fields.size();
-  Fields.emplace_back(FT);
-  FieldInfo &Field = Fields.back();
-  Field.Offset =
-      llvm::alignTo(NextOffset, std::min(Alignment, FieldAlignmentSize));
-  if (!IsUnion) {
-    NextOffset = std::max(NextOffset, Field.Offset);
-  }
-  AlignmentSize = std::max(AlignmentSize, FieldAlignmentSize);
-  return Field;
-}
+FieldInitializer &FieldInitializer::operator=(FieldInitializer &&Initializer) {
 
 /// The concrete assembly parser instance.
 // Note that this is a full MCAsmParser, not an MCAsmParserExtension!
 // It's a peer of AsmParser, not of COFFAsmParser, WasmAsmParser, etc.
 class MasmParser : public MCAsmParser {
 private:
   AsmLexer Lexer;
   MCContext &Ctx;