Skip to content

Clang function types v2: Electric Boogaloo (parsing + printing) #28737

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 4 commits into from
Jan 6, 2020
Merged

Clang function types v2: Electric Boogaloo (parsing + printing) #28737

merged 4 commits into from
Jan 6, 2020

Conversation

varungandhi-apple
Copy link
Contributor

@varungandhi-apple varungandhi-apple commented Dec 12, 2019

First three commits are from #28726. Tried to get it out soon for review. Some things are still TODO but the core things are done:

  • Add some test cases to check that diagnostics trigger. I tested manually and it seemed to work, but still.
  • Add some test cases for SIL printing. AST and SIL logic are very close, so this should be minimal work. We can't add test cases for this right now, because we aren't storing the type in SIL
  • Add some test cases when the Clang type is coming from the ClangImporter. Technically, that shouldn't have any effect, but still.
  • Add some complex test cases using typedefs, struct pointers etc?

@varungandhi-apple
Copy link
Contributor Author

We should run still run the tests now though.

@swift-ci please test

@varungandhi-apple
Copy link
Contributor Author

Still haven't had the time to write the extra tests, will do that soon 😐.

@swift-ci please test

@varungandhi-apple
Copy link
Contributor Author

Tests are kinda' done. Seems minimal, but there's only so many things you can do with type declarations, I guess. When we return a function pointer through a typealias, then the type of the variable is ctypes.fptr?, without any @convention, so we don't have anything to test there.

@varungandhi-apple
Copy link
Contributor Author

@swift-ci please test

@swift-ci please test source compatibility

@varungandhi-apple
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b3247951dff767d46fb119d9629683b8ebfd3be1

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - b3247951dff767d46fb119d9629683b8ebfd3be1

@varungandhi-apple
Copy link
Contributor Author

@rjmccall LGTY?

@varungandhi-apple
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Jan 4, 2020

Build failed
Swift Test Linux Platform
Git Sha - 20ee1594da9531067fba601292afa559478f9f39

@swift-ci
Copy link
Contributor

swift-ci commented Jan 4, 2020

Build failed
Swift Test OS X Platform
Git Sha - 20ee1594da9531067fba601292afa559478f9f39

@varungandhi-apple
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

LGTM

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