Skip to content

Add support for embedded Swift mode#494

Merged
lorentey merged 1 commit intoapple:mainfrom
parkera:parkera/embedded_collections
Jul 22, 2025
Merged

Add support for embedded Swift mode#494
lorentey merged 1 commit intoapple:mainfrom
parkera:parkera/embedded_collections

Conversation

@parkera
Copy link
Copy Markdown
Member

@parkera parkera commented Jul 17, 2025

Remove calls to debugPrint (which does not exist), and ifdef out support for Codable and Reflection.

Note that testing is not available in embedded yet, as far as I know.

@parkera parkera requested review from kubamracek and rauhul July 17, 2025 22:13
@parkera parkera requested a review from lorentey as a code owner July 17, 2025 22:13
Copy link
Copy Markdown
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

Looks good, with the nit noted below -- I think we should just #if out all Custom[Debug]StringConvertible conformances, like we do mirrors and serialization

}
#if !$Embedded
debugPrint(item, terminator: "", to: &result)
#endif
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, this replaces [1, 2, 3] with [, , ] in description properties, and that's not very nice.

In this case, the stdlib is currently substituting a generic message for each item, like this:

Suggested change
#endif
#else
result += "(unprintable)"
#endif

This results in [(unprintable), (unprintable), (unprintable)]. To be fair, that isn't that useful, either. I think it would be preferable to just replace the entire body with a short summary like "[\(count) values]". We can also choose to entirely omit the CustomStringConvertible conformances under $Embedded.

(Ditto for _dictionaryDescription and all other collection printing code below.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This turns out to be a good idea because the LosslessStringConvertible conformance also relied on description.

@vanvoorden
Copy link
Copy Markdown
Contributor

@parkera non-blocking question… do we have build jobs for embedded mode?

Remove calls to `debugPrint` (which does not exist), and ifdef out support for `Codable` and `Reflection`.
@parkera parkera force-pushed the parkera/embedded_collections branch from e07dc96 to c883a17 Compare July 17, 2025 23:37
@parkera
Copy link
Copy Markdown
Member Author

parkera commented Jul 17, 2025

@parkera non-blocking question… do we have build jobs for embedded mode?

I'm not sure if we have integrated embedded support into the GitHub actions - Swift CI is certainly building toolchains though. cc @shahmishal

@rauhul
Copy link
Copy Markdown

rauhul commented Jul 17, 2025

@parkera thanks for taking such a quick move on this, I'll try to figure out a common CI solution for the swiftlang repos next week, so we can merge this with confidence of it not breaking in the future.

@parkera
Copy link
Copy Markdown
Member Author

parkera commented Jul 18, 2025

@swift-ci test

@lorentey lorentey merged commit 8fb8949 into apple:main Jul 22, 2025
22 checks passed
@lorentey lorentey added this to the 1.3.0 milestone Jul 23, 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.

4 participants