Skip to content

[cxx-interop] Adding std.string initializer for UnsafePointer<CChar>? #65057

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
Apr 13, 2023

Conversation

plotfi
Copy link
Contributor

@plotfi plotfi commented Apr 11, 2023

Currently without an initializer for the unsafe char pointer type swiftc hits an assert around not being able to handle conversions of unsafe pointers with Any type. This patch adds the ability to convert to a std::string.

This is to address issue #61218

@plotfi plotfi requested review from zoecarver, drodriguez and NuriAmari and removed request for zoecarver April 11, 2023 06:05
@plotfi plotfi added the c++ interop Feature: Interoperability with C++ label Apr 11, 2023
@egorzhdan
Copy link
Contributor

@swift-ci please smoke test

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

Thanks @plotfi!
Could you please also add an execution test to make sure that the std::string that is created has the correct contents?
Otherwise LGTM.

@plotfi
Copy link
Contributor Author

plotfi commented Apr 12, 2023

Thanks @plotfi!
Could you please also add an execution test to make sure that the std::string that is created has the correct contents?
Otherwise LGTM.

Hi @egorzhdan can you point me to some similar execution tests that are already in tree? I am a little lost, sorry.

@egorzhdan
Copy link
Contributor

@plotfi sure! Just adding a new test to test/Interop/Cxx/stdlib/overlay/std-string-overlay.swift should work.
Something like:

StdStringOverlayTestSuite.test("std::string from C string") {
  let str = "abc".withCString { ptr in
    std.string(ptr)
  }
  expectEqual(str, std.string("abc"))
}

Currently without an initializer for the unsafe char pointer type swiftc
hits an assert around not being able to handle conversions of unsafe
pointers with Any type. This patch adds the ability to convert to a
std::string.

This is to address issue swiftlang#61218
@plotfi plotfi force-pushed the init-String-with-NSString-utf8String branch from c237f9f to 6180387 Compare April 13, 2023 02:04
@plotfi
Copy link
Contributor Author

plotfi commented Apr 13, 2023

@swift-ci please smoke test

1 similar comment
@NuriAmari
Copy link
Contributor

@swift-ci please smoke test

@plotfi
Copy link
Contributor Author

plotfi commented Apr 13, 2023

@plotfi sure! Just adding a new test to test/Interop/Cxx/stdlib/overlay/std-string-overlay.swift should work. Something like:

StdStringOverlayTestSuite.test("std::string from C string") {
  let str = "abc".withCString { ptr in
    std.string(ptr)
  }
  expectEqual(str, std.string("abc"))
}

Thank You Egor! I will remember this in the future.

@plotfi plotfi merged commit da42b43 into swiftlang:main Apr 13, 2023
egorzhdan added a commit that referenced this pull request Apr 13, 2023
This fixes a build failure that started occurring on CentOS after #65057:
```
error: cannot find 'strlen' in scope
```

rdar://107987115
@hyp
Copy link
Contributor

hyp commented Apr 13, 2023

@plotfi this should also go to 5.9

meg-gupta pushed a commit to meg-gupta/swift that referenced this pull request Apr 14, 2023
This fixes a build failure that started occurring on CentOS after swiftlang#65057:
```
error: cannot find 'strlen' in scope
```

rdar://107987115
plotfi pushed a commit to plotfi/swift that referenced this pull request Apr 18, 2023
This fixes a build failure that started occurring on CentOS after swiftlang#65057:
```
error: cannot find 'strlen' in scope
```

rdar://107987115
(cherry picked from commit 7774650)
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.

4 participants