Skip to content

[Clang importer] Initial support for swift_newtype / swift_wrapper #2272

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
Apr 23, 2016

Conversation

milseman
Copy link
Member

What's in this pull request?

Adds initial support for swift_newtype(struct), and swift_newtype(enum) by treating as swift_newtype(struct).

Resolved bug number: (SR-)


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
OS X platform @swift-ci Please test OS X platform
Linux platform @swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

Adds makeStructRawValued utility function, which shares common code
for raw-value-backed structs from both OptionSets and many imported
enums. NFC.
This introduces support for swift_newtype(struct) attribute (also
known as swift_wrapper). The Clang importer will create a brand new
struct corresponding to the annotated typedefs, which has a backing
raw value. Globals of that type are imported as static members on the
struct.

Additionally, this interacts seamlessly with prior import-as-member
work, meaning that the newly created type can be imported onto. Tests
included.
This is a temporary solution that implements swift_newtype(enum) as
though it were written swift_newtype(struct). This is to work around
to the fact that a String-backed enum does not actually have a String
stored, and a struct is closer to reflecting that storage
properly. Struct provides most of the functionality and appearance for
now, though it does not allow for switching over the values.

Full support for swift_newtype(enum) as a Swift enum is forthcoming.
@milseman
Copy link
Member Author

@swift-ci please test

@milseman
Copy link
Member Author

milseman commented Apr 22, 2016

pexpect.TIMEOUT: Timeout exceeded.
Failing Tests (1):
swift-package-tests :: repl/test-repl-glibc.py

This seems unrelated and a problem with the CI. @shahmishal what do you think? Should I kick it off again? Who is familiar with this test?

@shahmishal
Copy link
Member

@milseman This test failure is related to https://bugs.swift.org/browse/SR-1109

@milseman
Copy link
Member Author

@DougGregor, can you please review these changes?


// Scan the decl name, and drop any words that appear in order in the type
// name
for (auto wI = declNameWords.begin(), skipWI = newtypeNameWords.begin();
Copy link
Member

Choose a reason for hiding this comment

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

Is there truly no utility within the enum-handling code to compare common prefixes between two names?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in the spirit of SE-0033, in which there's the example:
extern NSErrorDomain const NSCocoaErrorDomain ===> NSErrorDomain.cocoa

In this case, it's not prefix stripping but rather common-word stripping. I'm happy to make a utility in StringExtras or wherever that can help expose, share, or unify these tasks, but I wasn't able to find a direct analogue to what I was trying to do.

In general, I am interested in more camel_case utilities that can also help unify with the import-as-member inference system (which has its own handful of helpers) and enum handling (which is just prefix).

Copy link
Member Author

Choose a reason for hiding this comment

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

In the meantime, it's worth me extracting this into a static local function, to not overly pollute importFullName and give it a meaningful name.

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed offline, and we're going to go against the proposal and only do prefix stripping. I'll add this to the list of things to review for the proposal amend.

@DougGregor
Copy link
Member

Please add a test that uses the imported results, so we're certain that name lookup finds these implicitly-imported members.

Rather than do whole-word common-word stripping, we only strip common
prefixes. This is not as powerful as what was originally intended, but
much less magical in the mapping into Swift.

Test case adjusted, and common utility functions exposed.
A test case to make sure name lookup succeeds for swift_newtype.
@milseman
Copy link
Member Author

@swift-ci please test

@milseman milseman merged commit 83ed4bc into swiftlang:master Apr 23, 2016
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