Skip to content

[Reflection] Remove DEPENDENT_TEMPLATE2 macro #16372

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
May 4, 2018

Conversation

bob-wilson
Copy link
Contributor

The Clang version checks introduced in #16212 and #16221 do not work well.
The clang_major macro is defined differently for Apple builds of Clang,
and the APPLE_CC macro is obsolete and not useful. The good news is that
those checks appear to be unnecessary. The version of Clang used with
Swift 4.2 (based on LLVM 6.0) accepts the code without the template keyword
in the places where the DEPENDENT_TEMPLATE2 macro was being used.

This also corrects a mistake in #16221 where the non-Clang/GCC definition
of DEPENDENT_TEMPLATE was changed to use the template keyword.

The Clang version checks introduced in swiftlang#16212 and swiftlang#16221 do not work well.
The __clang_major__ macro is defined differently for Apple builds of Clang,
and the __APPLE_CC__ macro is obsolete and not useful. The good news is that
those checks appear to be unnecessary. The version of Clang used with
Swift 4.2 (based on LLVM 6.0) accepts the code without the template keyword
in the places where the DEPENDENT_TEMPLATE2 macro was being used.

This also corrects a mistake in swiftlang#16221 where the non-Clang/GCC definition
of DEPENDENT_TEMPLATE was changed to use the template keyword.
@bob-wilson
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 4, 2018

Build failed
Swift Test Linux Platform
Git Sha - 62b9375

@davezarzycki
Copy link
Contributor

Thanks Bob!

@bob-wilson
Copy link
Contributor Author

The Linux failures here are happening on other PRs and are not related to this change. I'm going to go ahead and merge this to unblock master-next testing.

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.

3 participants