Skip to content

[cxx-interop] Fix test header to declare linkage for a implicit template #72522

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
Mar 23, 2024

Conversation

drodriguez
Copy link
Contributor

The header was defining a function, the function created a lambda, and the lambda was transformed into a std::function. This transformation is incorrect because the function scope does not have linkage, so the instantiated types will not have linkage either, causing the error below.

.../include/c++/11.2.0/bits/invoke.h:104:5:
error: function 'std::__invoke_r<int, (lambda at .../swift/test/Interop/Cxx/stdlib/Inputs/std-function.h:9:10) &, int>' is used but not defined in this translation unit, and cannot be defined in any other translation unit because its type does not have linkage
102 │   template<typename _Res, typename _Callable, typename... _Args>
103 │     constexpr enable_if_t<is_invocable_r_v<_Res, _Callable,
    _Args...>, _Res>
    104 │     __invoke_r(_Callable&& __fn, _Args&&... __args)
        │     ╰─ error: function 'std::__invoke_r<int, (lambda at .../swift/test/Interop/Cxx/stdlib/Inputs/std-function.h:9:10) &, int>' is used but not defined in this translation unit, and cannot be defined in any other translation unit because its type does not have linkage
        105 │     noexcept(is_nothrow_invocable_r_v<_Res, _Callable,
            _Args...>)
  106 │     {

  }

Declaring the function inline forces each TU to have their own copies of the function, which avoids the instantiated templates from being in a different TU than the one using them.

The header would not have worked in a normal C++ program, since none of the functions declare linkage, they would have been defined in each TU that included the header and it would fail linking as soon as two of those TU tried to be linked together. Declaring the functions inline avoids the problem (a more normal header, only with declarations, would also worked).

The header was defining a function, the function created a lambda, and
the lambda was transformed into a `std::function`. This transformation
is incorrect because the function scope does not have linkage, so the
instantiated types will not have linkage either, causing the error
below.

```
.../include/c++/11.2.0/bits/invoke.h:104:5:
error: function 'std::__invoke_r<int, (lambda at .../swift/test/Interop/Cxx/stdlib/Inputs/std-function.h:9:10) &, int>' is used but not defined in this translation unit, and cannot be defined in any other translation unit because its type does not have linkage
102 │   template<typename _Res, typename _Callable, typename... _Args>
103 │     constexpr enable_if_t<is_invocable_r_v<_Res, _Callable,
    _Args...>, _Res>
    104 │     __invoke_r(_Callable&& __fn, _Args&&... __args)
        │     ╰─ error: function 'std::__invoke_r<int, (lambda at .../swift/test/Interop/Cxx/stdlib/Inputs/std-function.h:9:10) &, int>' is used but not defined in this translation unit, and cannot be defined in any other translation unit because its type does not have linkage
        105 │     noexcept(is_nothrow_invocable_r_v<_Res, _Callable,
            _Args...>)
  106 │     {

  }
```

Declaring the function `inline` forces each TU to have their own copies
of the function, which avoids the instantiated templates from being in a
different TU than the one using them.

The header would not have worked in a normal C++ program, since none of
the functions declare linkage, they would have been defined in each TU
that included the header and it would fail linking as soon as two of
those TU tried to be linked together. Declaring the functions `inline`
avoids the problem (a more normal header, only with declarations, would
also worked).
@drodriguez
Copy link
Contributor Author

@egorzhdan this is what I was talking about.

I don't think this is because any changes in our headers, but I don't understand why you are not seeing it on your side.

If you try to use that header in a C++ program you would had hit some of those issues as well. They are simply hidden by the fact that only one TU is being used in the test.

@drodriguez
Copy link
Contributor Author

@swift-ci please 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.

Oh, interesting! Thank you for investigating this.

@egorzhdan
Copy link
Contributor

@swift-ci please test

@drodriguez drodriguez merged commit 8b4821f into swiftlang:main Mar 23, 2024
@drodriguez drodriguez deleted the inline-fixes-everythiing branch March 23, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants