Skip to content

[cxx-interop][SwiftCompilerSources] Fix conversion between std::string and Swift.String #59152

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
Jun 8, 2022

Conversation

egorzhdan
Copy link
Contributor

This fixes a dangling pointer issue when creating a Swift.String from std::string.

Also fixes a warning:

warning: variable 's' was never mutated; consider changing to 'let' constant
    var s = SILBasicBlock_debugDescription(bridged)
    ~~~ ^
    let

rdar://92963081
rdar://93053488

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label May 30, 2022
@egorzhdan egorzhdan force-pushed the egorzhdan/libswift-string-init branch 2 times, most recently from 3d27915 to c570984 Compare May 30, 2022 12:07
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan requested a review from eeckstein May 30, 2022 13:06
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

/// Underscored to avoid name collision with the std overlay.
/// To be replaced with an overlay call once the CI uses SDKs built with Swift 5.7.
public init(_cxxString s: std.string) {
self.init(cString: s.c_str())
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not required anymore that s is a var for calling c_str? (I can't remember why we had to do this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were using var to avoid breaking the hosttools build for people using Xcode 13.2 or older. Now that Xcode 13.4 is released, this is probably no longer necessary.
We could produce a friendly CMake error for an outdated Swift compiler version if we merge #59020.

…ng` and `Swift.String`

This fixes a dangling pointer issue when creating a `Swift.String` from `std::string`.

Also fixes a warning:
```
warning: variable 's' was never mutated; consider changing to 'let' constant
    var s = SILBasicBlock_debugDescription(bridged)
    ~~~ ^
    let
```

rdar://92963081
rdar://93053488
@egorzhdan egorzhdan force-pushed the egorzhdan/libswift-string-init branch from c570984 to be711e3 Compare May 30, 2022 19:06
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Linux

@egorzhdan egorzhdan merged commit c6a95c6 into main Jun 8, 2022
@egorzhdan egorzhdan deleted the egorzhdan/libswift-string-init branch June 8, 2022 11:52
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