Skip to content

[cxx-interop] Initial tests for std::function usage #70772

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 5 commits into from
Jan 17, 2024

Conversation

egorzhdan
Copy link
Contributor

This makes sure that std::function is imported consistently on supported platforms, and that it allows basic usage: calling a function with callAsFunction, initializing an empty function, and passing a function retrieved from C++ back to C++ as a parameter.

rdar://103979602

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Jan 8, 2024
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@hyp
Copy link
Contributor

hyp commented Jan 9, 2024

yay!

Copy link
Contributor

@hyp hyp left a comment

Choose a reason for hiding this comment

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

LG, not sure why it's failing :(

@egorzhdan
Copy link
Contributor Author

The Windows failure is interesting, I think we might be treating std::function as an unsafe type there

C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\test\Interop\Cxx\stdlib\use-std-function.swift:12:11: error: cannot find 'FunctionIntToInt' in scope
  let f = FunctionIntToInt()
          ^~~~~~~~~~~~~~~~
C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\test\Interop\Cxx\stdlib\use-std-function.swift:13:14: error: cannot find 'isEmptyFunction' in scope
  expectTrue(isEmptyFunction(f))
             ^~~~~~~~~~~~~~~
C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\test\Interop\Cxx\stdlib/Inputs\std-function.h:12:1: note: function 'isEmptyFunction' unavailable (cannot import)
bool isEmptyFunction(FunctionIntToInt f) { return (bool)f; }
^

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

1 similar comment
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

This makes sure that `std::function` is imported consistently on supported platforms, and that it allows basic usage: calling a function with `callAsFunction`, initializing an empty function, and passing a function retrieved from C++ back to C++ as a parameter.

rdar://103979602
@egorzhdan egorzhdan force-pushed the egorzhdan/std-function-initial-tests branch from c8522eb to 2d0863a Compare January 11, 2024 12:02
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

`clangSema.isCompleteType` checks for decl visibility according to the module visibility rules, which we don't actually need. What we need to check is whether the record has a definition – this is the check we already use elsewhere in ClangImporter.

This makes sure that we can import `std::function` on Windows.

rdar://103979602
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

The new failure on Windows:

<unknown>:0: error: cannot initialize object parameter of type 'const std::_Func_class<int, int>' with an expression of type 'const std::function<int (int)>'
<unknown>:0: error: failed to synthesize call to the base method 'callAsFunction' of type 'callAsFunction'
<unknown>:0: error: missing return in instance method expected to return 'Int32'

The test was passing by accident: we didn't properly check `std::pair` type for completeness, and assumed that it is incomplete.

This updates the test to actually verify the behavior that is specified in the vision document.
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

In libstdc++ 11, `std::pair` has a base class `std::__pair_base`, which defines a copy constructor.

We still continue not treating `std::pair` as an owned type, just like on other platforms.
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

Hmm, adding explicit imports helps with the Windows failure...

import CxxStdlib
import CxxStdlib.functional

It seems there is something wrong with Clang Sema's module visibility check

@egorzhdan
Copy link
Contributor Author

Ahhh we were missing export * in the modulemap... 🤦‍♂️

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan requested a review from hyp January 16, 2024 18:08
@egorzhdan egorzhdan merged commit 2061640 into main Jan 17, 2024
@egorzhdan egorzhdan deleted the egorzhdan/std-function-initial-tests branch January 17, 2024 13:25
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