Skip to content

Conversation

@drodriguez
Copy link
Contributor

@drodriguez drodriguez commented Jul 10, 2019

When the header was used in Android, the usage of __ANDROID_API__ was
not set if the compiler wasn't setting it externally. There was a check
for __ANDROID_API__, which defaulted to zero, and so it didn't pass. The
external function definition was not being done, but in C mode, it
didn't matter because implicit functions are allowed. That's not true in
C++ mode, which fails to compile any code that tries to include this
header.

The solution is including android/api-level.h which will define
__ANDROID_API__ to a very high value if it is not defined already (which
is the default behaviour in the NDK).

When the header was used in Android, the usage of __ANDROID_API__ was
not set if the compiler wasn't setting it externally. There was a check
for __ANDROID_API__, which defaulted to zero, and so it didn't pass. The
external function definition was not being done, but in C mode, it
didn't matter because implicit functions are allowed. That's not true in
C++ mode, which fails to compile any code that tries to include this
header.

The solution is including android/api-level.h which will define
__ANDROID_API__ to a very high value if it is not defined already (which
is the default behaviour in the NDK).
@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@pschuh pschuh left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but won't this just set it to __ANDROID_API_FUTURE__(10000) as per: https://android.googlesource.com/platform/bionic/+/master/libc/include/android/api-level.h ?

@drodriguez
Copy link
Contributor Author

Yes. It does. That's the behaviour when compiling the NDK. If you don’t provide an API level, the build system understand that you are targeting the latest API level available. When this file is compiled as part of Swift, the API level is actually provided as 21, but since this is a test, the definition is not set.

I might look into changing the test invocations to provide the API level used during building the stdlib (since it might reflect better how Android is compiled), but this should be needed in case someone decides to compile for Android without providing their own value for __ANDROID_API__.

@drodriguez drodriguez merged commit b77499f into swiftlang:master Jul 10, 2019
@drodriguez drodriguez deleted the android-fix-cxx-interop-2 branch July 10, 2019 01:57
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