Skip to content

[CxxInterop] Import C++ references. #31702

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 13, 2020
Merged

[CxxInterop] Import C++ references. #31702

merged 1 commit into from
May 13, 2020

Conversation

hlopko
Copy link
Contributor

@hlopko hlopko commented May 11, 2020

This PR enables import of C++ references as UnsafePointers. PR follows the implementation of pointers with a difference that for references we don't wrap the UnsafePointer in Optional.

@gribozavr gribozavr added the c++ interop Feature: Interoperability with C++ label May 11, 2020
@hlopko hlopko marked this pull request as ready for review May 11, 2020 11:47
@hlopko hlopko requested a review from gribozavr May 11, 2020 11:47
@hlopko
Copy link
Contributor Author

hlopko commented May 11, 2020

@swift-ci Please test

1 similar comment
@hlopko
Copy link
Contributor Author

hlopko commented May 11, 2020

@swift-ci Please test

gribozavr
gribozavr reviewed May 11, 2020
View reviewed changes
gribozavr
gribozavr reviewed May 11, 2020
View reviewed changes
@compnerd
Copy link
Member

@swift-ci please test Windows platform

1 similar comment
@compnerd
Copy link
Member

@swift-ci please test Windows platform

gribozavr
gribozavr reviewed May 12, 2020
View reviewed changes
@hlopko
Copy link
Contributor Author

hlopko commented May 12, 2020

@swift-ci Please test

@hlopko
Copy link
Contributor Author

hlopko commented May 12, 2020

@swift-ci please test Windows platform

@swiftlang swiftlang deleted a comment from swift-ci May 12, 2020
@gribozavr
Copy link
Contributor

Could you also adjust the incorrect comment in lib/ClangImporter/ImporterImpl.h:

  /// Import the type of a function parameter.
  ///
  /// This provides special treatment for C++ references (which become
  /// [inout] parameters) and C pointers (which become magic [inout]-able types),
  /// among other things, and enables the conversion of bridged types.
  /// Parameters are always considered CF-audited.
  Parameter,

@hlopko hlopko force-pushed the references branch 2 times, most recently from e49c60f to f8d2be1 Compare May 12, 2020 12:45
@hlopko
Copy link
Contributor Author

hlopko commented May 12, 2020

(Adjusted the Parameter comment slightly).

@hlopko
Copy link
Contributor Author

hlopko commented May 12, 2020

@swift-ci Please test

1 similar comment
@hlopko
Copy link
Contributor Author

hlopko commented May 12, 2020

@swift-ci Please test

gribozavr
gribozavr reviewed May 12, 2020
View reviewed changes
gribozavr
gribozavr reviewed May 12, 2020
View reviewed changes
@shahmishal
Copy link
Member

@swift-ci Please test

@hlopko
Copy link
Contributor Author

hlopko commented May 13, 2020

@swift-ci Please test

1 similar comment
@hlopko
Copy link
Contributor Author

hlopko commented May 13, 2020

@swift-ci Please test

@hlopko
Copy link
Contributor Author

hlopko commented May 13, 2020

@swift-ci Please test Windows

1 similar comment
@hlopko
Copy link
Contributor Author

hlopko commented May 13, 2020

@swift-ci Please test Windows

@hlopko
Copy link
Contributor Author

hlopko commented May 13, 2020

@swift-ci Please test Linux

@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@swiftlang swiftlang deleted a comment from swift-ci May 13, 2020
@hlopko hlopko merged commit 30b5fd5 into swiftlang:master May 13, 2020
@hlopko hlopko deleted the references branch May 13, 2020 11:34
@compnerd
Copy link
Member

This broke the Windows VS2017 builds: https://ci-external.swift.org/job/oss-swift-windows-x86_64/3976/consoleText

compnerd added a commit that referenced this pull request May 14, 2020
@compnerd
Revert "[CxxInterop] Import C++ references. (#31702)"
0a3d7a4 
This reverts commit 30b5fd5.
@compnerd compnerd mentioned this pull request May 14, 2020
Merged
@hlopko
Copy link
Contributor Author

hlopko commented May 14, 2020

@compnerd just to double check I understand what happened, this PR passed the windows bot with MSVC19, but then failed on the bot with MSVC17, correct? Is it an option to have MSVC17 bot for presubmits too?

@compnerd
Copy link
Member

Correct, that’s what happened. Sadly, I don’t think that adding VS2017 as presubmit is entirely viable (a matter of cost and effort). For the most part it’s not been an issue - this breakage is the first in ~6 months that’s specific to VS2017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gribozavr gribozavr gribozavr approved these changes

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hlopko @compnerd @gribozavr @shahmishal