Add filter method for ordered and unordered map

[mohanadkandil]

Implemented a filter method like the Swift stdlib one
Resolves #67400

[mohanadkandil]

@swift-ci Please test

[finagolfin]

If you modify the original comment to add "Resolves #67400," as shown when you submitted this pull, that issue will be linked to this pull.

[egorzhdan]

@mohanadkandil thanks for the PR! You won't be able to run the CI just yet, this is fine, running the CI requires write access to the repo. I can run the CI for you in the future.

Let me know if you need any help with compiling this locally on your machine.

[egorzhdan]

@mohanadkandil please try building and running this on your machine

[CrazyFanFan]

I have some personal insights regarding filter that I would like to share. Given that CxxDictionary does not require its Key to conform to Hashable, I believe it would be more reasonable for the filter function to return Self. Consequently, we need a method to create a new instance of Self using either std::map or std::unordered_map. I have attempted to write the code for this; however, I have yet to find a suitable method for implementing the init function.

[egorzhdan]

I believe it would be more reasonable for the filter function to return Self

I agree. Another reason why it should return Self is that there is already a way to filter a std::map into a Swift Dictionary: Dictionary(myCxxMap).filter { ... }. However, there is no way to filter a std::map into a std::map, which is what you might want to do when working with a C++ library from Swift.

[mohanadkandil]

Hi @egorzhdan,
A̶f̶t̶e̶r̶ ̶p̶o̶s̶t̶i̶n̶g̶ ̶o̶n̶ ̶f̶o̶r̶u̶m̶s̶ ̶a̶n̶d̶ ̶c̶h̶a̶n̶g̶i̶n̶g̶ ̶t̶h̶e̶ ̶c̶o̶d̶e̶,̶ ̶b̶u̶i̶l̶d̶ ̶e̶r̶r̶o̶r̶s̶ ̶a̶r̶e̶ ̶n̶o̶t̶ ̶r̶e̶l̶a̶t̶e̶d̶ ̶t̶o̶ ̶m̶y̶ ̶c̶h̶a̶n̶g̶e̶s̶,̶ ̶I̶ ̶j̶u̶s̶t̶ ̶t̶r̶i̶e̶d̶ ̶a̶ ̶c̶o̶m̶p̶l̶e̶t̶e̶ ̶d̶i̶f̶f̶e̶r̶e̶n̶t̶ ̶b̶r̶a̶n̶c̶h̶ ̶a̶n̶d̶ ̶i̶t̶'̶s̶ ̶s̶t̶i̶l̶l̶ ̶g̶e̶t̶t̶i̶n̶g̶ ̶̶9̶ ̶e̶r̶r̶o̶r̶s̶ ̶g̶e̶n̶e̶r̶a̶t̶e̶d̶̶.̶ ̶H̶o̶w̶e̶v̶e̶r̶,̶ ̶t̶h̶e̶ ̶c̶h̶a̶n̶g̶e̶s̶ ̶a̶b̶o̶v̶e̶ ̶l̶i̶k̶e̶ ̶r̶e̶t̶u̶r̶n̶i̶n̶g̶ ̶s̶e̶l̶f̶ ̶a̶r̶e̶ ̶a̶l̶r̶e̶a̶d̶y̶ ̶c̶o̶m̶p̶l̶e̶t̶e̶d̶.̶ ̶

Also, I posted a more detailed post on the forum so maybe anyone could tell me about a workaround to sole DAK errors. It will be solved asap

All issues resolved ✅

[mohanadkandil]

Hi @egorzhdan
Just added a test for the filter and passed it.

Should we run the checks ?

[egorzhdan]

@mohanadkandil awesome, let's run the CI!

[egorzhdan]

@swift-ci please smoke test

[mohanadkandil]

@egorzhdan Passed!
Do you have any comments in code before merge ?

[egorzhdan]

This mostly looks good to me, thanks @mohanadkandil!
Left a couple comments.

[egorzhdan]

@swift-ci please smoke test

[egorzhdan]

@swift-ci please smoke test

[egorzhdan]

@swift-ci please smoke test

[egorzhdan]

This looks great to me now, thank you @mohanadkandil!

[egorzhdan]

@swift-ci please smoke test

[egorzhdan]

@swift-ci please smoke test macOS

[egorzhdan]

@swift-ci please smoke test Linux

[egorzhdan]

I'm going to merge this now. @mohanadkandil next time could you please squash all your commits in a PR into one commit 😉

[mrousavy]

Sorry this is maybe the wrong place to ask this, but does Swift's umbrella header expose a swift::Dictionary<K, V> type, or how should passing around maps/dictionaries between Swift <-> C++ be handled?

[egorzhdan]

does Swift's umbrella header expose a swift::Dictionary<K, V> type

Not at the moment, no.

how should passing around maps/dictionaries between Swift <-> C++ be handled

You could pass around an instance of std::map, that way you could access it from both Swift and C++.