Skip to content
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

[DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility #9956

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

Michael137
Copy link

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.

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 requested a review from a team as a code owner February 5, 2025 12:17
@Michael137
Copy link
Author

@swift-ci test

@Michael137 Michael137 force-pushed the michael137/enum-extensibility-final branch from 9c8836e to d3fb604 Compare February 5, 2025 13:30
@Michael137
Copy link
Author

@swift-ci test

1 similar comment
@Michael137
Copy link
Author

@swift-ci test

Copy link

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!

@Michael137 Michael137 merged commit d864437 into stable/20240723 Feb 5, 2025
3 checks passed
@Michael137 Michael137 deleted the michael137/enum-extensibility-final branch February 5, 2025 22:41
Michael137 added a commit that referenced this pull request Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants