Skip to content

[llvm][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility #124752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Michael137
Copy link
Member

When creating EnumDecls from DWARF for Objective-C NS_ENUMs, the Swift compiler tries to figure out if it should perform "swiftification" of that enum (which involves renaming the enumerator cases, etc.). The heuristics by which it determines whether we want to swiftify an enum is by checking the enum_extensibility attribute (because that's what NS_ENUM pretty much are). Currently LLDB fails to attach the EnumExtensibilityAttr to EnumDecls it creates (because there's not enough info in DWARF to derive it), which means we have to fall back to re-building Swift modules on-the-fly, slowing down expression evaluation substantially. This happens around https://github.com/swiftlang/swift/blob/4b3931c8ce437b3f13f245e6423f95c94a5876ac/lib/ClangImporter/ImportEnumInfo.cpp#L37-L59

To speed up Swift exression evaluation, this patch proposes encoding the C/C++/Objective-C enum_extensibility attribute in DWARF via a new DW_AT_APPLE_ENUM_KIND. This would currently be only used from the LLDB Swift plugin. But may be of interest to other language plugins as well (though I haven't come up with a concrete use-case for it outside of Swift).

I'm open to naming suggestions of the various new attributes/attribute constants proposed here. I tried to be as generic as possible if we wanted to extend it to other kinds of enum properties (e.g., flag enums).

The new attribute would look as follows:

DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Closed)
  DW_AT_name      ("ClosedEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (23)

DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Open)
  DW_AT_name      ("OpenEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (27)

Absence of the attribute means the extensibility of the enum is unknown and abides by whatever the language rules of that CU dictate.

This does feel like a big hammer for quite a specific use-case, so I'm happy to discuss alternatives.

Alternatives considered:

  • Re-using an existing DWARF attribute to express extensibility. E.g., a DW_TAG_enumeration_type could have a DW_AT_count or DW_AT_upper_bound indicating the number of enumerators, which could imply closed-ness. I felt like a dedicated attribute (which could be generalized further) seemed more applicable. But I'm open to re-using existing attributes.
  • Encoding the entire attribute string (i.e., DW_TAG_LLVM_annotation ("enum_extensibility((open))")) on the DW_TAG_enumeration_type. Then in LLDB somehow parse that out into a EnumExtensibilityAttr. I haven't found a great API in Clang to parse arbitrary strings into AST nodes (the ones I've found required fully formed C++ constructs). Though if someone knows of a good way to do this, happy to consider that too.

@Michael137 Michael137 changed the title [WIP][clang][DebugInfo] Add new DW_AT_APPLE_ENUM_KIND to encode enum_extensibility [WIP][clang][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility Jan 28, 2025
@dwblaikie
Copy link
Collaborator

(hmm, can't use the foundation library on godbolt/compiler explorer)
Can you show a small example of the code you're interested in and the DWARF it produces? The documentation I can find seems to suggest that the extensible enums have ctors that take the underlying integer type, but the non-extensible ones don't, so I'd have thought that might be adequate to differentiate without an extra attribute?

@Michael137
Copy link
Member Author

Michael137 commented Jan 29, 2025

(hmm, can't use the foundation library on godbolt/compiler explorer) Can you show a small example of the code you're interested in and the DWARF it produces? The documentation I can find seems to suggest that the extensible enums have ctors that take the underlying integer type, but the non-extensible ones don't, so I'd have thought that might be adequate to differentiate without an extra attribute?

NS_ENUM is just a #define that adds the enum_extensibility(open) attribute to a plain C enum. Given following example:

#import <Foundation/Foundation.h>

enum Enum {
    e1
} e;

enum class EnumClass : unsigned {
    ec1
} ec;

enum __attribute__((enum_extensibility(open))) OpenEnum {
  oe1
} oe;

enum class __attribute__((enum_extensibility(open))) OpenEnumClass : unsigned {
  oec1
} oec;

typedef NS_ENUM(unsigned, ns_enum) {
  ns1
} ns;

Here are all the DW_TAG_enumeration_types:

0x0000002c:   DW_TAG_enumeration_type
                DW_AT_type	(0x00000039 "unsigned int")
                DW_AT_name	("Enum")
                DW_AT_byte_size	(0x04)
                DW_AT_decl_file	("/Users/michaelbuch/ns_enum.m")
                DW_AT_decl_line	(3)

0x00000035:     DW_TAG_enumerator
                  DW_AT_name	("e1")
                  DW_AT_const_value	(0)

0x00000048:   DW_TAG_enumeration_type
                DW_AT_type	(0x00000039 "unsigned int")
                DW_AT_enum_class	(true)
                DW_AT_name	("EnumClass")
                DW_AT_byte_size	(0x04)
                DW_AT_decl_file	("/Users/michaelbuch/ns_enum.m")
                DW_AT_decl_line	(7)

0x00000051:     DW_TAG_enumerator
                  DW_AT_name	("ec1")
                  DW_AT_const_value	(0)

0x00000060:   DW_TAG_enumeration_type
                DW_AT_type	(0x00000039 "unsigned int")
                DW_AT_name	("OpenEnum")
                DW_AT_byte_size	(0x04)
                DW_AT_decl_file	("/Users/michaelbuch/ns_enum.m")
                DW_AT_decl_line	(11)

0x00000069:     DW_TAG_enumerator
                  DW_AT_name	("oe1")
                  DW_AT_const_value	(0)

0x00000078:   DW_TAG_enumeration_type
                DW_AT_type	(0x00000039 "unsigned int")
                DW_AT_enum_class	(true)
                DW_AT_name	("OpenEnumClass")
                DW_AT_byte_size	(0x04)
                DW_AT_decl_file	("/Users/michaelbuch/ns_enum.m")
                DW_AT_decl_line	(15)

0x00000081:     DW_TAG_enumerator
                  DW_AT_name	("oec1")
                  DW_AT_const_value	(0)

0x00000090:   DW_TAG_enumeration_type
                DW_AT_type	(0x00000039 "unsigned int")
                DW_AT_name	("ns_enum")
                DW_AT_byte_size	(0x04)
                DW_AT_decl_file	("/Users/michaelbuch/ns_enum.m")
                DW_AT_decl_line	(19)

0x00000099:     DW_TAG_enumerator
                  DW_AT_name	("ns1")
                  DW_AT_const_value	(0)

So nothing about the extensibility is reflected here

@dwblaikie
Copy link
Collaborator

Is my thing about the ctor understanding correct, and it's that the ctor exists theoretically/abstractly, but not in the AST or in the generated IR/DWARF? Could it be added/would that be sufficient?

But, yeah, probably fine to just add the attribute, since that's the real feature you want to know about.

@Michael137
Copy link
Member Author

Is my thing about the ctor understanding correct, and it's that the ctor exists theoretically/abstractly, but not in the AST or in the generated IR/DWARF? Could it be added/would that be sufficient?

But, yeah, probably fine to just add the attribute, since that's the real feature you want to know about.

Hmmm this is what the AST looks like:

|-EnumDecl 0xc9accd6d0 <enum.m:3:1, line:5:1> line:3:6 Enum
| `-EnumConstantDecl 0xc9accd790 <line:4:5> col:5 e1 'Enum'
|-VarDecl 0xc9accd848 <line:3:1, line:5:3> col:3 e 'enum Enum':'Enum'
|-EnumDecl 0xc9accd8e0 <line:7:1, line:9:1> line:7:12 class EnumClass 'unsigned int'
| `-EnumConstantDecl 0xc9accd9a0 <line:8:5> col:5 ec1 'EnumClass'
|-VarDecl 0xc9accda58 <line:7:1, line:9:3> col:3 ec 'enum EnumClass':'EnumClass'
|-EnumDecl 0xc9accdae8 <line:11:1, line:13:1> line:11:48 OpenEnum
| |-EnumExtensibilityAttr 0xc9accdbb0 <col:21, col:44> Open
| `-EnumConstantDecl 0xc9accdc08 <line:12:3> col:3 oe1 'OpenEnum'
|-VarDecl 0xc9accdcb8 <line:11:1, line:13:3> col:3 oe 'enum OpenEnum':'OpenEnum'
|-EnumDecl 0xc9accdd60 <line:15:1, line:17:1> line:15:54 class OpenEnumClass 'unsigned int'
| |-EnumExtensibilityAttr 0xc9accde20 <col:27, col:50> Open
| `-EnumConstantDecl 0xc9accde78 <line:16:3> col:3 oec1 'OpenEnumClass'
|-VarDecl 0xc9accdf28 <line:15:1, line:17:3> col:3 oec 'enum OpenEnumClass':'OpenEnumClass'
|-EnumDecl 0xc9accdfd0 <CFAvailability.h:142:43, enum.m:19:17> col:27 ns_enum 'unsigned int'
| `-EnumExtensibilityAttr 0xc9acce090 <CFAvailability.h:125:45, col:68> Open
|-TypedefDecl 0xc9acce148 <enum.m:19:1, col:27> col:27 ns_enum 'enum ns_enum':'ns_enum'
| `-ElaboratedType 0xc9acce0f0 'enum ns_enum' sugar
|   `-EnumType 0xc9acce070 'ns_enum'
|     `-Enum 0xc9acce210 'ns_enum'
|-EnumDecl 0xc9acce210 prev 0xc9accdfd0 <CFAvailability.h:142:90, enum.m:21:1> line:19:27 ns_enum 'unsigned int'
| |-EnumExtensibilityAttr 0xc9acce2e0 <CFAvailability.h:125:45, col:68> Inherited Open
| `-EnumConstantDecl 0xc9acce308 <enum.m:20:3> col:3 ns1 'ns_enum'
`-VarDecl 0xc9acce3b8 <CFAvailability.h:142:90, enum.m:21:3> col:3 ns 'enum ns_enum':'ns_enum'

So yea no sign of a constructor, in either AST, IR or DWARF. So it does sound more like a hypothetical construct than something that is modelled in Clang (looking at how the EnumExtensibilityAttr is used in Clang I don't see any mention of constructors). Seems non-trivial to generate one, particularly just for this purpose. Also if enum constructors ever become a thing then we'll need to disambiguate this case somehow. So a dedicated attribute seemed like the only way

@Michael137 Michael137 changed the title [WIP][clang][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility [llvm][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility Feb 5, 2025
@Michael137
Copy link
Member Author

I've only included the LLVM changes here now (anything metadata and DWARF attribute related). Will do the plumbing from the frontend in a separate patch

@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Michael Buch (Michael137)

Changes

When creating EnumDecls from DWARF for Objective-C NS_ENUMs, the Swift compiler tries to figure out if it should perform "swiftification" of that enum (which involves renaming the enumerator cases, etc.). The heuristics by which it determines whether we want to swiftify an enum is by checking the enum_extensibility attribute (because that's what NS_ENUM pretty much are). Currently LLDB fails to attach the EnumExtensibilityAttr to EnumDecls it creates (because there's not enough info in DWARF to derive it), which means we have to fall back to re-building Swift modules on-the-fly, slowing down expression evaluation substantially. This happens around https://github.com/swiftlang/swift/blob/4b3931c8ce437b3f13f245e6423f95c94a5876ac/lib/ClangImporter/ImportEnumInfo.cpp#L37-L59

To speed up Swift exression evaluation, this patch proposes encoding the C/C++/Objective-C enum_extensibility attribute in DWARF via a new DW_AT_APPLE_ENUM_KIND. This would currently be only used from the LLDB Swift plugin. But may be of interest to other language plugins as well (though I haven't come up with a concrete use-case for it outside of Swift).

I'm open to naming suggestions of the various new attributes/attribute constants proposed here. I tried to be as generic as possible if we wanted to extend it to other kinds of enum properties (e.g., flag enums).

The new attribute would look as follows:

DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Closed)
  DW_AT_name      ("ClosedEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (23)

DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Open)
  DW_AT_name      ("OpenEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (27)

Absence of the attribute means the extensibility of the enum is unknown and abides by whatever the language rules of that CU dictate.

This does feel like a big hammer for quite a specific use-case, so I'm happy to discuss alternatives.

Alternatives considered:

  • Re-using an existing DWARF attribute to express extensibility. E.g., a DW_TAG_enumeration_type could have a DW_AT_count or DW_AT_upper_bound indicating the number of enumerators, which could imply closed-ness. I felt like a dedicated attribute (which could be generalized further) seemed more applicable. But I'm open to re-using existing attributes.
  • Encoding the entire attribute string (i.e., DW_TAG_LLVM_annotation ("enum_extensibility((open))")) on the DW_TAG_enumeration_type. Then in LLDB somehow parse that out into a EnumExtensibilityAttr. I haven't found a great API in Clang to parse arbitrary strings into AST nodes (the ones I've found required fully formed C++ constructs). Though if someone knows of a good way to do this, happy to consider that too.

Patch is 74.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124752.diff

19 Files Affected:

  • (modified) llvm/include/llvm/AsmParser/LLToken.h (+1)
  • (modified) llvm/include/llvm/BinaryFormat/Dwarf.def (+13-1)
  • (modified) llvm/include/llvm/BinaryFormat/Dwarf.h (+12-3)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+10-8)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+43-34)
  • (modified) llvm/lib/AsmParser/LLLexer.cpp (+1)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+37-6)
  • (modified) llvm/lib/BinaryFormat/Dwarf.cpp (+21)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+14-8)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+3)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+3)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+5)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+29-28)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+23-21)
  • (added) llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll (+56)
  • (added) llvm/test/tools/llvm-dwarfdump/AArch64/DW_AT_APPLE_enum_kind.s (+44)
  • (modified) llvm/unittests/IR/DebugTypeODRUniquingTest.cpp (+43-37)
  • (modified) llvm/unittests/IR/MetadataTest.cpp (+99-85)
  • (modified) llvm/unittests/Transforms/Utils/CloningTest.cpp (+2-1)
diff --git a/llvm/include/llvm/AsmParser/LLToken.h b/llvm/include/llvm/AsmParser/LLToken.h
index 7b47bc88ddb25f..c52622879b885e 100644
--- a/llvm/include/llvm/AsmParser/LLToken.h
+++ b/llvm/include/llvm/AsmParser/LLToken.h
@@ -497,6 +497,7 @@ enum Kind {
   DwarfMacinfo,     // DW_MACINFO_foo
   ChecksumKind,     // CSK_foo
   DbgRecordType,    // dbg_foo
+  DwarfEnumKind,    // DW_APPLE_ENUM_KIND_foo
 
   // Type valued tokens (TyVal).
   Type,
diff --git a/llvm/include/llvm/BinaryFormat/Dwarf.def b/llvm/include/llvm/BinaryFormat/Dwarf.def
index 2bb84fbc864d8e..724a14ccc7aeaf 100644
--- a/llvm/include/llvm/BinaryFormat/Dwarf.def
+++ b/llvm/include/llvm/BinaryFormat/Dwarf.def
@@ -24,7 +24,8 @@
       (defined HANDLE_DW_CFA && defined HANDLE_DW_CFA_PRED) ||                 \
       defined HANDLE_DW_APPLE_PROPERTY || defined HANDLE_DW_UT ||              \
       defined HANDLE_DWARF_SECTION || defined HANDLE_DW_IDX ||                 \
-      defined HANDLE_DW_END || defined HANDLE_DW_SECT)
+      defined HANDLE_DW_END || defined HANDLE_DW_SECT ||                       \
+      defined HANDLE_DW_APPLE_ENUM_KIND)
 #error "Missing macro definition of HANDLE_DW*"
 #endif
 
@@ -146,6 +147,10 @@
 #define HANDLE_DW_SECT(ID, NAME)
 #endif
 
+#ifndef HANDLE_DW_APPLE_ENUM_KIND
+#define HANDLE_DW_APPLE_ENUM_KIND(ID, NAME)
+#endif
+
 HANDLE_DW_TAG(0x0000, null, 2, DWARF, DW_KIND_NONE)
 HANDLE_DW_TAG(0x0001, array_type, 2, DWARF, DW_KIND_TYPE)
 HANDLE_DW_TAG(0x0002, class_type, 2, DWARF, DW_KIND_TYPE)
@@ -638,6 +643,7 @@ HANDLE_DW_AT(0x3fed, APPLE_property, 0, APPLE)
 HANDLE_DW_AT(0x3fee, APPLE_objc_direct, 0, APPLE)
 HANDLE_DW_AT(0x3fef, APPLE_sdk, 0, APPLE)
 HANDLE_DW_AT(0x3ff0, APPLE_origin, 0, APPLE)
+HANDLE_DW_AT(0x3ff1, APPLE_enum_kind, 0, APPLE)
 
 // Attribute form encodings.
 HANDLE_DW_FORM(0x01, addr, 2, DWARF)
@@ -1269,6 +1275,11 @@ HANDLE_DW_APPLE_PROPERTY(0x1000, nullability)
 HANDLE_DW_APPLE_PROPERTY(0x2000, null_resettable)
 HANDLE_DW_APPLE_PROPERTY(0x4000, class)
 
+// Enum kinds.
+// Keep in sync with EnumExtensibilityAttr::Kind.
+HANDLE_DW_APPLE_ENUM_KIND(0x00, Closed)
+HANDLE_DW_APPLE_ENUM_KIND(0x01, Open)
+
 // DWARF v5 Unit Types.
 HANDLE_DW_UT(0x01, compile)
 HANDLE_DW_UT(0x02, type)
@@ -1367,3 +1378,4 @@ HANDLE_DW_SECT(8, RNGLISTS)
 #undef HANDLE_DW_IDX
 #undef HANDLE_DW_END
 #undef HANDLE_DW_SECT
+#undef HANDLE_DW_APPLE_ENUM_KIND
diff --git a/llvm/include/llvm/BinaryFormat/Dwarf.h b/llvm/include/llvm/BinaryFormat/Dwarf.h
index 3be819c0a76eeb..397b4b164386d8 100644
--- a/llvm/include/llvm/BinaryFormat/Dwarf.h
+++ b/llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -44,9 +44,10 @@ namespace dwarf {
 enum LLVMConstants : uint32_t {
   /// LLVM mock tags (see also llvm/BinaryFormat/Dwarf.def).
   /// \{
-  DW_TAG_invalid = ~0U,        ///< Tag for invalid results.
-  DW_VIRTUALITY_invalid = ~0U, ///< Virtuality for invalid results.
-  DW_MACINFO_invalid = ~0U,    ///< Macinfo type for invalid results.
+  DW_TAG_invalid = ~0U,             ///< Tag for invalid results.
+  DW_VIRTUALITY_invalid = ~0U,      ///< Virtuality for invalid results.
+  DW_MACINFO_invalid = ~0U,         ///< Macinfo type for invalid results.
+  DW_APPLE_ENUM_KIND_invalid = ~0U, ///< Enum kind for invalid results.
   /// \}
 
   /// Special values for an initial length field.
@@ -198,6 +199,12 @@ enum VirtualityAttribute {
   DW_VIRTUALITY_max = 0x02
 };
 
+enum EnumKindAttribute {
+#define HANDLE_DW_APPLE_ENUM_KIND(ID, NAME) DW_APPLE_ENUM_KIND_##NAME = ID,
+#include "llvm/BinaryFormat/Dwarf.def"
+  DW_APPLE_ENUM_KIND_max = 0x01
+};
+
 enum DefaultedMemberAttribute {
 #define HANDLE_DW_DEFAULTED(ID, NAME) DW_DEFAULTED_##NAME = ID,
 #include "llvm/BinaryFormat/Dwarf.def"
@@ -981,6 +988,7 @@ StringRef AccessibilityString(unsigned Access);
 StringRef DefaultedMemberString(unsigned DefaultedEncodings);
 StringRef VisibilityString(unsigned Visibility);
 StringRef VirtualityString(unsigned Virtuality);
+StringRef EnumKindString(unsigned EnumKind);
 StringRef LanguageString(unsigned Language);
 StringRef CaseString(unsigned Case);
 StringRef ConventionString(unsigned Convention);
@@ -1020,6 +1028,7 @@ unsigned getOperationEncoding(StringRef OperationEncodingString);
 unsigned getSubOperationEncoding(unsigned OpEncoding,
                                  StringRef SubOperationEncodingString);
 unsigned getVirtuality(StringRef VirtualityString);
+unsigned getEnumKind(StringRef EnumKindString);
 unsigned getLanguage(StringRef LanguageString);
 unsigned getCallingConvention(StringRef LanguageString);
 unsigned getAttributeEncoding(StringRef EncodingString);
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 6c479415b9ed27..8bee9f4703dd9c 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -632,7 +632,8 @@ namespace llvm {
         DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
         uint64_t SizeInBits, uint32_t AlignInBits, DINodeArray Elements,
         DIType *UnderlyingType, unsigned RunTimeLang = 0,
-        StringRef UniqueIdentifier = "", bool IsScoped = false);
+        StringRef UniqueIdentifier = "", bool IsScoped = false,
+        std::optional<uint32_t> EnumKind = std::nullopt);
     /// Create debugging information entry for a set.
     /// \param Scope          Scope in which this set is defined.
     /// \param Name           Set name.
@@ -667,19 +668,20 @@ namespace llvm {
     static DIType *createObjectPointerType(DIType *Ty, bool Implicit);
 
     /// Create a permanent forward-declared type.
-    DICompositeType *createForwardDecl(unsigned Tag, StringRef Name,
-                                       DIScope *Scope, DIFile *F, unsigned Line,
-                                       unsigned RuntimeLang = 0,
-                                       uint64_t SizeInBits = 0,
-                                       uint32_t AlignInBits = 0,
-                                       StringRef UniqueIdentifier = "");
+    DICompositeType *
+    createForwardDecl(unsigned Tag, StringRef Name, DIScope *Scope, DIFile *F,
+                      unsigned Line, unsigned RuntimeLang = 0,
+                      uint64_t SizeInBits = 0, uint32_t AlignInBits = 0,
+                      StringRef UniqueIdentifier = "",
+                      std::optional<uint32_t> EnumKind = std::nullopt);
 
     /// Create a temporary forward-declared type.
     DICompositeType *createReplaceableCompositeType(
         unsigned Tag, StringRef Name, DIScope *Scope, DIFile *F, unsigned Line,
         unsigned RuntimeLang = 0, uint64_t SizeInBits = 0,
         uint32_t AlignInBits = 0, DINode::DIFlags Flags = DINode::FlagFwdDecl,
-        StringRef UniqueIdentifier = "", DINodeArray Annotations = nullptr);
+        StringRef UniqueIdentifier = "", DINodeArray Annotations = nullptr,
+        std::optional<uint32_t> EnumKind = std::nullopt);
 
     /// Retain DIScope* in a module even if it is not referenced
     /// through debug info anchors.
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 5ea8c0d7b448dc..8515d8eda85686 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1176,24 +1176,28 @@ class DICompositeType : public DIType {
   friend class MDNode;
 
   unsigned RuntimeLang;
+  std::optional<uint32_t> EnumKind;
 
   DICompositeType(LLVMContext &C, StorageType Storage, unsigned Tag,
                   unsigned Line, unsigned RuntimeLang, uint64_t SizeInBits,
                   uint32_t AlignInBits, uint64_t OffsetInBits,
-                  uint32_t NumExtraInhabitants, DIFlags Flags,
+                  uint32_t NumExtraInhabitants,
+                  std::optional<uint32_t> EnumKind, DIFlags Flags,
                   ArrayRef<Metadata *> Ops)
       : DIType(C, DICompositeTypeKind, Storage, Tag, Line, SizeInBits,
                AlignInBits, OffsetInBits, NumExtraInhabitants, Flags, Ops),
-        RuntimeLang(RuntimeLang) {}
+        RuntimeLang(RuntimeLang), EnumKind(EnumKind) {}
   ~DICompositeType() = default;
 
   /// Change fields in place.
   void mutate(unsigned Tag, unsigned Line, unsigned RuntimeLang,
               uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
-              uint32_t NumExtraInhabitants, DIFlags Flags) {
+              uint32_t NumExtraInhabitants, std::optional<uint32_t> EnumKind,
+              DIFlags Flags) {
     assert(isDistinct() && "Only distinct nodes can mutate");
     assert(getRawIdentifier() && "Only ODR-uniqued nodes should mutate");
     this->RuntimeLang = RuntimeLang;
+    this->EnumKind = EnumKind;
     DIType::mutate(Tag, Line, SizeInBits, AlignInBits, OffsetInBits,
                    NumExtraInhabitants, Flags);
   }
@@ -1203,15 +1207,15 @@ class DICompositeType : public DIType {
           unsigned Line, DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
           uint32_t AlignInBits, uint64_t OffsetInBits, DIType *Specification,
           uint32_t NumExtraInhabitants, DIFlags Flags, DINodeArray Elements,
-          unsigned RuntimeLang, DIType *VTableHolder,
-          DITemplateParameterArray TemplateParams, StringRef Identifier,
-          DIDerivedType *Discriminator, Metadata *DataLocation,
-          Metadata *Associated, Metadata *Allocated, Metadata *Rank,
-          DINodeArray Annotations, StorageType Storage,
+          unsigned RuntimeLang, std::optional<uint32_t> EnumKind,
+          DIType *VTableHolder, DITemplateParameterArray TemplateParams,
+          StringRef Identifier, DIDerivedType *Discriminator,
+          Metadata *DataLocation, Metadata *Associated, Metadata *Allocated,
+          Metadata *Rank, DINodeArray Annotations, StorageType Storage,
           bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name), File,
                    Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits,
-                   Flags, Elements.get(), RuntimeLang, VTableHolder,
+                   Flags, Elements.get(), RuntimeLang, EnumKind, VTableHolder,
                    TemplateParams.get(),
                    getCanonicalMDString(Context, Identifier), Discriminator,
                    DataLocation, Associated, Allocated, Rank, Annotations.get(),
@@ -1222,21 +1226,21 @@ class DICompositeType : public DIType {
           unsigned Line, Metadata *Scope, Metadata *BaseType,
           uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
           DIFlags Flags, Metadata *Elements, unsigned RuntimeLang,
-          Metadata *VTableHolder, Metadata *TemplateParams,
-          MDString *Identifier, Metadata *Discriminator, Metadata *DataLocation,
-          Metadata *Associated, Metadata *Allocated, Metadata *Rank,
-          Metadata *Annotations, Metadata *Specification,
-          uint32_t NumExtraInhabitants, StorageType Storage,
-          bool ShouldCreate = true);
+          std::optional<uint32_t> EnumKind, Metadata *VTableHolder,
+          Metadata *TemplateParams, MDString *Identifier,
+          Metadata *Discriminator, Metadata *DataLocation, Metadata *Associated,
+          Metadata *Allocated, Metadata *Rank, Metadata *Annotations,
+          Metadata *Specification, uint32_t NumExtraInhabitants,
+          StorageType Storage, bool ShouldCreate = true);
 
   TempDICompositeType cloneImpl() const {
     return getTemporary(
         getContext(), getTag(), getName(), getFile(), getLine(), getScope(),
         getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
-        getFlags(), getElements(), getRuntimeLang(), getVTableHolder(),
-        getTemplateParams(), getIdentifier(), getDiscriminator(),
-        getRawDataLocation(), getRawAssociated(), getRawAllocated(),
-        getRawRank(), getAnnotations(), getSpecification(),
+        getFlags(), getElements(), getRuntimeLang(), getEnumKind(),
+        getVTableHolder(), getTemplateParams(), getIdentifier(),
+        getDiscriminator(), getRawDataLocation(), getRawAssociated(),
+        getRawAllocated(), getRawRank(), getAnnotations(), getSpecification(),
         getNumExtraInhabitants());
   }
 
@@ -1246,7 +1250,8 @@ class DICompositeType : public DIType {
       (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
        DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
        uint32_t AlignInBits, uint64_t OffsetInBits, DIFlags Flags,
-       DINodeArray Elements, unsigned RuntimeLang, DIType *VTableHolder,
+       DINodeArray Elements, unsigned RuntimeLang,
+       std::optional<uint32_t> EnumKind, DIType *VTableHolder,
        DITemplateParameterArray TemplateParams = nullptr,
        StringRef Identifier = "", DIDerivedType *Discriminator = nullptr,
        Metadata *DataLocation = nullptr, Metadata *Associated = nullptr,
@@ -1255,23 +1260,24 @@ class DICompositeType : public DIType {
        uint32_t NumExtraInhabitants = 0),
       (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
        OffsetInBits, Specification, NumExtraInhabitants, Flags, Elements,
-       RuntimeLang, VTableHolder, TemplateParams, Identifier, Discriminator,
-       DataLocation, Associated, Allocated, Rank, Annotations))
+       RuntimeLang, EnumKind, VTableHolder, TemplateParams, Identifier,
+       Discriminator, DataLocation, Associated, Allocated, Rank, Annotations))
   DEFINE_MDNODE_GET(
       DICompositeType,
       (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
        Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
        uint32_t AlignInBits, uint64_t OffsetInBits, DIFlags Flags,
-       Metadata *Elements, unsigned RuntimeLang, Metadata *VTableHolder,
+       Metadata *Elements, unsigned RuntimeLang,
+       std::optional<uint32_t> EnumKind, Metadata *VTableHolder,
        Metadata *TemplateParams = nullptr, MDString *Identifier = nullptr,
        Metadata *Discriminator = nullptr, Metadata *DataLocation = nullptr,
        Metadata *Associated = nullptr, Metadata *Allocated = nullptr,
        Metadata *Rank = nullptr, Metadata *Annotations = nullptr,
        Metadata *Specification = nullptr, uint32_t NumExtraInhabitants = 0),
       (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
-       OffsetInBits, Flags, Elements, RuntimeLang, VTableHolder, TemplateParams,
-       Identifier, Discriminator, DataLocation, Associated, Allocated, Rank,
-       Annotations, Specification, NumExtraInhabitants))
+       OffsetInBits, Flags, Elements, RuntimeLang, EnumKind, VTableHolder,
+       TemplateParams, Identifier, Discriminator, DataLocation, Associated,
+       Allocated, Rank, Annotations, Specification, NumExtraInhabitants))
 
   TempDICompositeType clone() const { return cloneImpl(); }
 
@@ -1288,10 +1294,11 @@ class DICompositeType : public DIType {
              Metadata *BaseType, uint64_t SizeInBits, uint32_t AlignInBits,
              uint64_t OffsetInBits, Metadata *Specification,
              uint32_t NumExtraInhabitants, DIFlags Flags, Metadata *Elements,
-             unsigned RuntimeLang, Metadata *VTableHolder,
-             Metadata *TemplateParams, Metadata *Discriminator,
-             Metadata *DataLocation, Metadata *Associated, Metadata *Allocated,
-             Metadata *Rank, Metadata *Annotations);
+             unsigned RuntimeLang, std::optional<uint32_t> EnumKind,
+             Metadata *VTableHolder, Metadata *TemplateParams,
+             Metadata *Discriminator, Metadata *DataLocation,
+             Metadata *Associated, Metadata *Allocated, Metadata *Rank,
+             Metadata *Annotations);
   static DICompositeType *getODRTypeIfExists(LLVMContext &Context,
                                              MDString &Identifier);
 
@@ -1310,10 +1317,11 @@ class DICompositeType : public DIType {
                Metadata *BaseType, uint64_t SizeInBits, uint32_t AlignInBits,
                uint64_t OffsetInBits, Metadata *Specification,
                uint32_t NumExtraInhabitants, DIFlags Flags, Metadata *Elements,
-               unsigned RuntimeLang, Metadata *VTableHolder,
-               Metadata *TemplateParams, Metadata *Discriminator,
-               Metadata *DataLocation, Metadata *Associated,
-               Metadata *Allocated, Metadata *Rank, Metadata *Annotations);
+               unsigned RuntimeLang, std::optional<uint32_t> EnumKind,
+               Metadata *VTableHolder, Metadata *TemplateParams,
+               Metadata *Discriminator, Metadata *DataLocation,
+               Metadata *Associated, Metadata *Allocated, Metadata *Rank,
+               Metadata *Annotations);
 
   DIType *getBaseType() const { return cast_or_null<DIType>(getRawBaseType()); }
   DINodeArray getElements() const {
@@ -1327,6 +1335,7 @@ class DICompositeType : public DIType {
   }
   StringRef getIdentifier() const { return getStringOperand(7); }
   unsigned getRuntimeLang() const { return RuntimeLang; }
+  std::optional<uint32_t> getEnumKind() const { return EnumKind; }
 
   Metadata *getRawBaseType() const { return getOperand(3); }
   Metadata *getRawElements() const { return getOperand(4); }
diff --git a/llvm/lib/AsmParser/LLLexer.cpp b/llvm/lib/AsmParser/LLLexer.cpp
index 5ea507c009bdc6..9bc5a79da49c61 100644
--- a/llvm/lib/AsmParser/LLLexer.cpp
+++ b/llvm/lib/AsmParser/LLLexer.cpp
@@ -975,6 +975,7 @@ lltok::Kind LLLexer::LexIdentifier() {
   DWKEYWORD(CC, DwarfCC);
   DWKEYWORD(OP, DwarfOp);
   DWKEYWORD(MACINFO, DwarfMacinfo);
+  DWKEYWORD(APPLE_ENUM_KIND, DwarfEnumKind);
 
 #undef DWKEYWORD
 
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index fa0079bac435c1..610ffe7d69be49 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -4697,6 +4697,12 @@ struct DwarfCCField : public MDUnsignedField {
   DwarfCCField() : MDUnsignedField(0, dwarf::DW_CC_hi_user) {}
 };
 
+struct DwarfEnumKindField : public MDUnsignedField {
+  DwarfEnumKindField()
+      : MDUnsignedField(dwarf::DW_APPLE_ENUM_KIND_invalid,
+                        dwarf::DW_APPLE_ENUM_KIND_max) {}
+};
+
 struct EmissionKindField : public MDUnsignedField {
   EmissionKindField() : MDUnsignedField(0, DICompileUnit::LastEmissionKind) {}
 };
@@ -4870,6 +4876,25 @@ bool LLParser::parseMDField(LocTy Loc, StringRef Name,
   return false;
 }
 
+template <>
+bool LLParser::parseMDField(LocTy Loc, StringRef Name,
+                            DwarfEnumKindField &Result) {
+  if (Lex.getKind() == lltok::APSInt)
+    return parseMDField(Loc, Name, static_cast<MDUnsignedField &>(Result));
+
+  if (Lex.getKind() != lltok::DwarfEnumKind)
+    return tokError("expected DWARF enum kind code");
+
+  unsigned EnumKind = dwarf::getEnumKind(Lex.getStrVal());
+  if (EnumKind == dwarf::DW_APPLE_ENUM_KIND_invalid)
+    return tokError("invalid DWARF enum kind code" + Twine(" '") +
+                    Lex.getStrVal() + "'");
+  assert(EnumKind <= Result.Max && "Expected valid DWARF enum kind code");
+  Result.assign(EnumKind);
+  Lex.Lex();
+  return false;
+}
+
 template <>
 bool LLParser::parseMDField(LocTy Loc, StringRef Name, DwarfLangField &Result) {
   if (Lex.getKind() == lltok::APSInt)
@@ -5489,6 +5514,7 @@ bool LLParser::parseDICompositeType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(elements, MDField, );                                               \
   OPTIONAL(runtimeLang, DwarfLangField, );                                     \
+  OPTIONAL(enumKind, DwarfEnumKindField, );                                    \
   OPTIONAL(vtableHolder, MDField, );                                           \
   OPTIONAL(templateParams, MDField, );                                         \
   OPTIONAL(identifier, MDStringField, );                                       \
@@ -5510,15 +5536,19 @@ bool LLParser::parseDICompositeType(MDNode *&Result, bool IsDistinct) {
   else if (rank.isMDField())
     Rank = rank.getMDFieldValue();
 
+  std::optional<unsigned> EnumKind;
+  if (enumKind.Val != dwarf::DW_APPLE_ENUM_KIND_invalid)
+    EnumKind = enumKind.Val;
+
   // If this has an identifier try to build an ODR type.
   if (identifier.Val)
     if (auto *CT = DICompositeType::buildODRType(
             Context, *identifier.Val, tag.Val, name.Val, file.Val, line.Val,
             scope.Val, baseType.Val, size.Val, align.Val, offset.Val,
             specificatio...
[truncated]

@Michael137 Michael137 merged commit eb8901b into llvm:main Feb 6, 2025
10 checks passed
@Michael137 Michael137 deleted the clang/debug-info-enum-extensibility branch February 6, 2025 08:58
Michael137 added a commit to Michael137/llvm-project that referenced this pull request Feb 6, 2025
This is the 2nd part to llvm#124752. Here we make sure to set the `DICompositeType` `EnumKind` if the enum was declared with `__attribute__((enum_extensibility(...)))`. In DWARF this will be rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when creating `clang::EnumDecl`s from debug-info.
Michael137 added a commit that referenced this pull request Feb 7, 2025
…#126045)

This is the 2nd part to
#124752. Here we make sure to
set the `DICompositeType` `EnumKind` if the enum was declared with
`__attribute__((enum_extensibility(...)))`. In DWARF this will be
rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when
creating `clang::EnumDecl`s from debug-info.
 
Depends on #126044
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 7, 2025
…y attribute (#126045)

This is the 2nd part to
llvm/llvm-project#124752. Here we make sure to
set the `DICompositeType` `EnumKind` if the enum was declared with
`__attribute__((enum_extensibility(...)))`. In DWARF this will be
rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when
creating `clang::EnumDecl`s from debug-info.

Depends on llvm/llvm-project#126044
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Feb 7, 2025
…_enum_kind

This patch consumes the `DW_AT_APPLE_enum_kind` attribute added in llvm#124752 and turns it into a Clang attribute in the AST. This will currently be used by the Swift language plugin when it creates `EnumDecl`s from debug-info and passes it to Swift compiler, which expects these attributes
Michael137 added a commit that referenced this pull request Feb 8, 2025
…_enum_kind (#126221)

This patch consumes the `DW_AT_APPLE_enum_kind` attribute added in
#124752 and turns it into a
Clang attribute in the AST. This will currently be used by the Swift
language plugin when it creates `EnumDecl`s from debug-info and passes
it to Swift compiler, which expects these attributes
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 8, 2025
…DW_AT_APPLE_enum_kind (#126221)

This patch consumes the `DW_AT_APPLE_enum_kind` attribute added in
llvm/llvm-project#124752 and turns it into a
Clang attribute in the AST. This will currently be used by the Swift
language plugin when it creates `EnumDecl`s from debug-info and passes
it to Swift compiler, which expects these attributes
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…ibility (llvm#124752)

When creating `EnumDecl`s from DWARF for Objective-C `NS_ENUM`s, the
Swift compiler tries to figure out if it should perform "swiftification"
of that enum (which involves renaming the enumerator cases, etc.). The
heuristics by which it determines whether we want to swiftify an enum is
by checking the `enum_extensibility` attribute (because that's what
`NS_ENUM` pretty much are). Currently LLDB fails to attach the
`EnumExtensibilityAttr` to `EnumDecl`s it creates (because there's not
enough info in DWARF to derive it), which means we have to fall back to
re-building Swift modules on-the-fly, slowing down expression evaluation
substantially. This happens around
https://github.com/swiftlang/swift/blob/4b3931c8ce437b3f13f245e6423f95c94a5876ac/lib/ClangImporter/ImportEnumInfo.cpp#L37-L59

To speed up Swift exression evaluation, this patch proposes encoding the
C/C++/Objective-C `enum_extensibility` attribute in DWARF via a new
`DW_AT_APPLE_ENUM_KIND`. This would currently be only used from the LLDB
Swift plugin. But may be of interest to other language plugins as well
(though I haven't come up with a concrete use-case for it outside of
Swift).

I'm open to naming suggestions of the various new attributes/attribute
constants proposed here. I tried to be as generic as possible if we
wanted to extend it to other kinds of enum properties (e.g., flag
enums).

The new attribute would look as follows:
```
DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Closed)
  DW_AT_name      ("ClosedEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (23)

DW_TAG_enumeration_type
  DW_AT_type      (0x0000003a "unsigned int")
  DW_AT_APPLE_enum_kind   (DW_APPLE_ENUM_KIND_Open)
  DW_AT_name      ("OpenEnum")
  DW_AT_byte_size (0x04)
  DW_AT_decl_file ("enum.c")
  DW_AT_decl_line (27)
```
Absence of the attribute means the extensibility of the enum is unknown
and abides by whatever the language rules of that CU dictate.

This does feel like a big hammer for quite a specific use-case, so I'm
happy to discuss alternatives.

Alternatives considered:
* Re-using an existing DWARF attribute to express extensibility. E.g., a
`DW_TAG_enumeration_type` could have a `DW_AT_count` or
`DW_AT_upper_bound` indicating the number of enumerators, which could
imply closed-ness. I felt like a dedicated attribute (which could be
generalized further) seemed more applicable. But I'm open to re-using
existing attributes.
* Encoding the entire attribute string (i.e., `DW_TAG_LLVM_annotation
("enum_extensibility((open))")`) on the `DW_TAG_enumeration_type`. Then
in LLDB somehow parse that out into a `EnumExtensibilityAttr`. I haven't
found a great API in Clang to parse arbitrary strings into AST nodes
(the ones I've found required fully formed C++ constructs). Though if
someone knows of a good way to do this, happy to consider that too.
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…llvm#126045)

This is the 2nd part to
llvm#124752. Here we make sure to
set the `DICompositeType` `EnumKind` if the enum was declared with
`__attribute__((enum_extensibility(...)))`. In DWARF this will be
rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when
creating `clang::EnumDecl`s from debug-info.
 
Depends on llvm#126044
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…_enum_kind (llvm#126221)

This patch consumes the `DW_AT_APPLE_enum_kind` attribute added in
llvm#124752 and turns it into a
Clang attribute in the AST. This will currently be used by the Swift
language plugin when it creates `EnumDecl`s from debug-info and passes
it to Swift compiler, which expects these attributes
@tromey
Copy link
Contributor

tromey commented Mar 17, 2025

This already landed, but I noticed an oddity in the MetadataLoader.cpp code. The patch does:

  case bitc::METADATA_COMPOSITE_TYPE: {
    if (Record.size() < 16 || Record.size() > 25)
      return error("Invalid record");
...
    if (Record.size() > 25 && Record[25] != dwarf::DW_APPLE_ENUM_KIND_invalid)
      EnumKind = Record[25];

I don't think that second if can ever be true, and that using 24 there would be correct.

@tromey
Copy link
Contributor

tromey commented Mar 17, 2025

I have a patch that touches this same area, so I will include the fix there. I don't really know how to write a test case for it, but I think the test case for my new feature will fail if the code remains as-is.

tromey added a commit to tromey/llvm-project that referenced this pull request Mar 17, 2025
In Ada, an array can be packed and the elements can take less space
than their natural object size.  For example, for this type:

   type Packed_Array is array (4 .. 8) of Boolean;
   pragma pack (Packed_Array);

... each element of the array occupies a single bit, even though the
"natural" size for a Boolean in memory is a byte.

In DWARF, this is represented by putting a DW_AT_bit_stride onto the
array type itself.

This patch adds a bit stride to DICompositeType so that gnat-llvm can
emit DWARF for these sorts of arrays.

This also fixes a bug in MetadataLoader.cpp, introduced in llvm#124752.  I
don't know how to write a separate test case for that, but I believe
the new array-bitstride.ll will fail if that fix is backed out.
@Michael137
Copy link
Member Author

This already landed, but I noticed an oddity in the MetadataLoader.cpp code. The patch does:

  case bitc::METADATA_COMPOSITE_TYPE: {
    if (Record.size() < 16 || Record.size() > 25)
      return error("Invalid record");
...
    if (Record.size() > 25 && Record[25] != dwarf::DW_APPLE_ENUM_KIND_invalid)
      EnumKind = Record[25];

I don't think that second if can ever be true, and that using 24 there would be correct.

Yup thanks for catching that. Indeed that should be 24. Surprised none of the new tests actually caught that 🤔

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Mar 21, 2025
… from bitcode

This was pointed out in
llvm#124752 (comment)

There was not test that roundtrips this attribute through LLVM bitcode, so this was never caught.
@Michael137
Copy link
Member Author

@tromey btw i put up #132374 which fixes this with a test

Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Mar 21, 2025
…ND from bitcode

This was pointed out in llvm#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode, so this was never caught.
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Mar 21, 2025
…ND from bitcode

This was pointed out in llvm#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode, so this was never caught.
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Mar 21, 2025
…ND from bitcode

This was pointed out in llvm#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode, so this was never caught.
Michael137 added a commit that referenced this pull request Mar 22, 2025
… from bitcode (#132374)

This was pointed out in
#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode,
so this was never caught.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Mar 22, 2025
…E_ENUM_KIND from bitcode (#132374)

This was pointed out in
llvm/llvm-project#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode,
so this was never caught.
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Apr 3, 2025
…ND from bitcode

This was pointed out in llvm#124752 (comment)

There was no test that roundtrips this attribute through LLVM bitcode, so this was never caught.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants