[PATCH] D59288: [DebugInfoMetadata] Move main subprogram DIFlag into DISPFlags

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

IIUC this upgrade is sweeping the variant of DISubprograms that have SPflags and do not yet have this patch under the rug? The way the Swift compiler branches work out I can live with that but other downstream users may have a problem with that as it could break LTO builds that include object files versions of clang between 347239 and this patch.



================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1421
+    // subprogram specific flags are placed in DIFlags.
+#define DIFlagMainSubprogram (1 << 21)
+    bool HasOldMainSubprogramFlag = Flags & DIFlagMainSubprogram;
----------------
Better to use a local variable instead of a macro here.


================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1423
+    bool HasOldMainSubprogramFlag = Flags & DIFlagMainSubprogram;
+    if (HasOldMainSubprogramFlag) {
+      // Remove old DIFlagMainSubprogram from DIFlags.
----------------
I think it would be more clear to write this as:
```
if (!HasSPFlags) {
  const unsigned DIFlagMainSubprogram = 1 << 21;
  bool HasOldMainSubprogramFlag = Flags & DIFlagMainSubprogram;
  // Remove old DIFlagMainSubprogram from DIFlags.
  // Note: This assumes that any future use of bit 21 defaults to it being 0.
  Flags &= ~static_cast<DINode::DIFlags>(DIFlagMainSubprogram);

  DISubprogram::toSPFlags(
                    /*IsLocalToUnit=*/Record[7], /*IsDefinition=*/Record[8],
                    /*IsOptimized=*/Record[14], /*Virtuality=*/Record[11],
                    /*DIFlagMainSubprogram*/HasOldMainSubprogramFlag);
}
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59288/new/

https://reviews.llvm.org/D59288