Skip to content

[cxx-interop] Allow initializing std::map from Swift Dictionary #75877

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
merged 1 commit into from
Aug 14, 2024

Conversation

egorzhdan
Copy link
Contributor

This adds initializers for std::map and std::unordered_map that take a Swift dictionary as a single parameter.

rdar://133691563

This adds initializers for `std::map` and `std::unordered_map` that take a Swift dictionary as a single parameter.

rdar://133691563
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Aug 14, 2024
@egorzhdan egorzhdan requested a review from Xazax-hun August 14, 2024 12:34
public init(_ dictionary: Dictionary<Key, Value>) where Key: Hashable {
self.init()
for (key, value) in dictionary {
self[key] = value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a future performance optimization, we could use __insertUnsafe here to skip the check for the presence of the element in subscript.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to do it in this PR? Are there any concerns about that approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No strong reasons, but I wanted to add a simple benchmark first (in a different PR) to see if the unsafe APIs actually give us any speedup compared to the subscript.

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

LGTM!

public init(_ dictionary: Dictionary<Key, Value>) where Key: Hashable {
self.init()
for (key, value) in dictionary {
self[key] = value
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to do it in this PR? Are there any concerns about that approach?

@egorzhdan
Copy link
Contributor Author

@swift-ci please test macOS

1 similar comment
@egorzhdan
Copy link
Contributor Author

@swift-ci please test macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan egorzhdan merged commit 8e2f0e2 into main Aug 14, 2024
5 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/std-map-init branch August 14, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants