Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Mx-Iris, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on bug fixes and improvements related to demangling Swift symbols and parsing MachO files, with specific attention to compatibility and correctness on iOS 26. It includes significant internal refactoring of the demangling and context resolution logic, enhances the command-line tool's robustness, adds serialization capabilities to semantic types, and updates the testing infrastructure to support newer iOS versions.
Highlights
- Context Demangling Improvements: Refactored the logic for building context manglings, including handling anonymous contexts and symbolic references, likely addressing bugs related to resolving type and protocol names.
- Codable Conformance: Added
Codableconformance toDemangleOptions,AnyComponent, andSemanticType, enabling serialization/deserialization of these types. - New Demangle Preset: Introduced a new
.interfacepreset option for demangling, available both programmatically and via the command-line tool. - NamedDumpable Protocol: Defined a new
NamedDumpableprotocol for types that can provide a demangled name, and refactoredClass,Enum, andProtocolto conform and use this for name dumping. - Improved CLI Error Handling: Added
do/catchblocks around the dumping of individual types in theswift-section dumpcommand to prevent a single error from stopping the entire dump process and to print specific error messages. - iOS 26 Testing Updates: Added new test MachO files for iOS 26 simulator runtimes (
SwiftUI.framework,SwiftUICore.framework) and updated test logic to use the appropriate CPU subtype (arm64_all).
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces several bug fixes and enhancements to the MachOFile processing and demangling logic. It adds Codable conformance to several structs and enums, introduces a new DemangleOptions preset, refactors the MetadataReader, and improves error handling in the DumpCommand. The changes also include updates to file paths in the test suite and the introduction of a NamedDumpable protocol.
| package func address<MachO: MachORepresentableWithCache>(of fileOffset: Int, in machO: MachO) -> UInt64 { | ||
| if let cache = machO.cache { | ||
| return .init(cache.mainCacheHeader.sharedRegionStart.cast() + fileOffset) | ||
| } else { | ||
| return .init(0x100000000 + fileOffset) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding a comment explaining the significance of 0x100000000 in the else block. What does this address offset represent?
| private static func demangleAnonymousContextName(context: SymbolOrElement<ContextDescriptorWrapper>, in machOFile: MachOFile) throws -> Node? { | ||
| guard case let .element(.anonymous(context)) = context, let mangledName = try context.mangledName(in: machOFile) else { return nil } | ||
| return try demangle(for: mangledName, kind: .symbol, in: machOFile) |
There was a problem hiding this comment.
Consider adding a comment explaining the purpose of unwrapping the context as .element(.anonymous(context)).
| switch file { | ||
| case .fat(let fatFile): | ||
| self.machOFile = try #require(fatFile.machOFiles().first(where: { $0.header.cpu.subtype == .arm64(.arm64e) })) | ||
| self.machOFile = try #require(fatFile.machOFiles().first(where: { $0.header.cpu.subtype == .arm64(.arm64_all) })) |
There was a problem hiding this comment.
Consider adding a comment explaining why .arm64_all is used instead of .arm64e.
There was a problem hiding this comment.
Pull Request Overview
This PR updates support for iOS 26 MachO files, enhances error handling in the dump command, refactors dumping interfaces with a new NamedDumpable protocol, and adds Codable conformance to core types.
- Tests now load the iOS 26 SwiftUICore simulator image and match on the
.arm64_allCPU subtype. - Each dump step in
DumpCommandis wrapped indo-catchwith a newdumpError(_:)helper. - A
NamedDumpableprotocol is introduced and applied to struct, enum, class, and protocol dumping to factor out name rendering. Codableconformance is added toSemanticType,SemanticString,AnyComponent, andDemangleOptions; presets are reformatted as arrays.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/SwiftDumpTests/SwiftDumpTests.swift | Load iOS 26 SwiftUICore simulator and match .arm64_all. |
| Sources/swift-section/DumpCommand.swift | Wrap dumps in do/catch, add dumpError(_:). |
| Sources/SwiftDump/Dumpable.swift | Introduce NamedDumpable protocol. |
| Sources/SwiftDump/Dumpable+/…+Dumpable.swift | Refactor each type’s dump into dumpName + dump(using:). |
| Sources/MachOTestingSupport/MachOFileName.swift | Add new enum cases for iOS 26 simulator images. |
Comments suppressed due to low confidence (1)
Sources/MachOTestingSupport/MachOFileName.swift:6
- [nitpick] The suffix
23A5260luses a lowercasel, which is easily confused with1. Consider renaming to23A5260Lor another unambiguous identifier to avoid confusion.
case iOS_23A5260l_Simulator_SwiftUI = "/.../SwiftUI.framework"
| @@ -7,9 +7,7 @@ import Semantic | |||
| extension ProtocolConformance: Dumpable { | |||
| @MachOImageGenerator | |||
| @SemanticStringBuilder | |||
There was a problem hiding this comment.
The new dumpTypeName method is missing the @MachOImageGenerator and @SemanticStringBuilder annotations, which may prevent it from generating the correct semantic output. Add those attributes above the signature.
| @SemanticStringBuilder | |
| @SemanticStringBuilder | |
| @MachOImageGenerator | |
| @SemanticStringBuilder |
| do { | ||
| let enumType = try Enum(descriptor: enumDescriptor, in: machO) | ||
| try dumpOrPrint(enumType.dump(using: options, in: machO)) | ||
| } catch { | ||
| dumpError(error) | ||
| } | ||
| case .struct(let structDescriptor): | ||
| let structType = try Struct(descriptor: structDescriptor, in: machO) | ||
| try dumpOrPrint(structType.dump(using: options, in: machO)) | ||
| if let metadata = try metadataFinder?.metadata(for: structDescriptor) as StructMetadata? { | ||
| try dumpOrPrint(metadata.fieldOffsets(for: structDescriptor, in: machO)) | ||
| do { | ||
| let structType = try Struct(descriptor: structDescriptor, in: machO) | ||
| try dumpOrPrint(structType.dump(using: options, in: machO)) | ||
| if let metadata = try metadataFinder?.metadata(for: structDescriptor) as StructMetadata? { | ||
| try dumpOrPrint(metadata.fieldOffsets(for: structDescriptor, in: machO)) | ||
| } | ||
| } catch { | ||
| dumpError(error) | ||
| } | ||
|
|
||
| case .class(let classDescriptor): | ||
| let classType = try Class(descriptor: classDescriptor, in: machO) | ||
| try dumpOrPrint(classType.dump(using: options, in: machO)) | ||
| if let metadata = try metadataFinder?.metadata(for: classDescriptor) as ClassMetadataObjCInterop? { | ||
| try dumpOrPrint(metadata.fieldOffsets(for: classDescriptor, in: machO)) | ||
| do { | ||
| let classType = try Class(descriptor: classDescriptor, in: machO) | ||
| try dumpOrPrint(classType.dump(using: options, in: machO)) | ||
| if let metadata = try metadataFinder?.metadata(for: classDescriptor) as ClassMetadataObjCInterop? { | ||
| try dumpOrPrint(metadata.fieldOffsets(for: classDescriptor, in: machO)) | ||
| } | ||
| } catch { | ||
| dumpError(error) |
There was a problem hiding this comment.
[nitpick] There are multiple identical do/catch blocks around each type dump. Consider abstracting the repeated pattern into a helper function to reduce boilerplate and ensure consistent error handling.
| do { | |
| let enumType = try Enum(descriptor: enumDescriptor, in: machO) | |
| try dumpOrPrint(enumType.dump(using: options, in: machO)) | |
| } catch { | |
| dumpError(error) | |
| } | |
| case .struct(let structDescriptor): | |
| let structType = try Struct(descriptor: structDescriptor, in: machO) | |
| try dumpOrPrint(structType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: structDescriptor) as StructMetadata? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: structDescriptor, in: machO)) | |
| do { | |
| let structType = try Struct(descriptor: structDescriptor, in: machO) | |
| try dumpOrPrint(structType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: structDescriptor) as StructMetadata? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: structDescriptor, in: machO)) | |
| } | |
| } catch { | |
| dumpError(error) | |
| } | |
| case .class(let classDescriptor): | |
| let classType = try Class(descriptor: classDescriptor, in: machO) | |
| try dumpOrPrint(classType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: classDescriptor) as ClassMetadataObjCInterop? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: classDescriptor, in: machO)) | |
| do { | |
| let classType = try Class(descriptor: classDescriptor, in: machO) | |
| try dumpOrPrint(classType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: classDescriptor) as ClassMetadataObjCInterop? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: classDescriptor, in: machO)) | |
| } | |
| } catch { | |
| dumpError(error) | |
| performWithErrorHandling { | |
| let enumType = try Enum(descriptor: enumDescriptor, in: machO) | |
| try dumpOrPrint(enumType.dump(using: options, in: machO)) | |
| } | |
| case .struct(let structDescriptor): | |
| performWithErrorHandling { | |
| let structType = try Struct(descriptor: structDescriptor, in: machO) | |
| try dumpOrPrint(structType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: structDescriptor) as StructMetadata? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: structDescriptor, in: machO)) | |
| } | |
| } | |
| case .class(let classDescriptor): | |
| performWithErrorHandling { | |
| let classType = try Class(descriptor: classDescriptor, in: machO) | |
| try dumpOrPrint(classType.dump(using: options, in: machO)) | |
| if let metadata = try metadataFinder?.metadata(for: classDescriptor) as ClassMetadataObjCInterop? { | |
| try dumpOrPrint(metadata.fieldOffsets(for: classDescriptor, in: machO)) | |
| } |
Co-authored-by: Copilot <[email protected]>
No description provided.