[llvm] [ObjectYAML] Fold layoutCOFF into writeCOFF (PR #221683)

[James Henderson via llvm-commits]

================
@@ -451,41 +345,121 @@ static bool writeCOFF(COFFParser &CP, ContiguousBlobAccumulator &CBA) {
     CurSymbol += 1 + Sym.Header.NumberOfAuxSymbols;
   }
 
+  // Collect the CodeView strings and checksums shared by all .debug$S sections.
+  for (COFFYAML::Section &S : CP.Obj.Sections) {
+    // We support specifying exactly one of SectionData or Subsections. So if
+    // there is already some SectionData, then we don't need to do any of this.
+    if (S.Name == ".debug$S" && S.SectionData.binary_size() == 0) {
+      CodeViewYAML::initializeStringsAndChecksums(S.DebugS,
+                                                  CP.StringsAndChecksums);
+      if (CP.StringsAndChecksums.hasChecksums() &&
+          CP.StringsAndChecksums.hasStrings())
+        break;
+    }
+  }
+
   // Output section data.
-  for (const COFFYAML::Section &S : CP.Obj.Sections) {
-    if (S.Header.SizeOfRawData == 0 || S.Header.PointerToRawData == 0)
-      continue;
-    assert(S.Header.PointerToRawData >= CBA.getOffset());
-    CBA.writeZeros(S.Header.PointerToRawData - CBA.getOffset());
-    for (auto E : S.StructuredData)
-      E.writeAsBinary(CBA);
-    CBA.writeAsBinary(S.SectionData);
-    assert(S.Header.PointerToRawData + S.Header.SizeOfRawData >=
-           CBA.getOffset());
-    CBA.writeZeros(S.Header.PointerToRawData + S.Header.SizeOfRawData -
-                   CBA.getOffset());
-    if (S.Header.Characteristics & COFF::IMAGE_SCN_LNK_NRELOC_OVFL) {
-      CBA.write<uint32_t>(/*VirtualAddress=*/S.Relocations.size() + 1,
-                          LittleEndian);
-      CBA.write<uint32_t>(/*SymbolTableIndex=*/0, LittleEndian);
-      CBA.write<uint16_t>(/*Type=*/0, LittleEndian);
+  for (COFFYAML::Section &S : CP.Obj.Sections) {
+    if (S.Name == ".debug$S") {
----------------
jh7370 wrote:

I think it would be good if this sequence of ifs for the debug sections were to be moved into a function. This would help label the thing being done (via the function name), making this rather monolithic function that much easier to read.

Also, perhaps worth considering a `StringSwitch`?

https://github.com/llvm/llvm-project/pull/221683