Skip to content

[RemoteAST] Fix function generation to form type from multiple params #12375

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

Closed
wants to merge 1 commit into from

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Oct 10, 2017

If function type has multiple parameters make sure that correct
tuple type is formed before constructing function type.

@xedin
Copy link
Contributor Author

xedin commented Oct 10, 2017

/cc @slavapestov @DougGregor that you going to love this one.

@xedin xedin changed the title [RemoteAST] Fix function generation to form type from updated inout params [RemoteAST] Fix function generation to form type from multiple params Oct 10, 2017
@xedin xedin force-pushed the fix-remote-ast-func-type branch from fd81a6d to 5b59fb4 Compare October 10, 2017 22:36
If function type has multiple parameters make sure that correct
tuple type is formed before constructing function type.
@xedin xedin force-pushed the fix-remote-ast-func-type branch from 5b59fb4 to ae5c72e Compare October 10, 2017 22:37
@xedin
Copy link
Contributor Author

xedin commented Oct 10, 2017

@swift-ci please smoke test

@slavapestov
Copy link
Contributor

Oh no :(

Can you add a test please?

@DougGregor
Copy link
Member

@slavapestov, I suspect that @xedin might appreciate a pointer to figure out how to write such a test

@xedin
Copy link
Contributor Author

xedin commented Oct 11, 2017

Yes, I most definitely would appreciate that! :)

@slavapestov
Copy link
Contributor

@xedin Take a look at test/RemoteAST/structural_types.swift. It already has tests for functions with multiple inputs but since they're materializable it must be going through the one-parameter code path.

Try adding a test case for a function with two inout arguments, and see if you can get it to fail before your fix.

@xedin
Copy link
Contributor Author

xedin commented Oct 16, 2017

Closing this one because the problem is going to be fixed by #12421

@xedin xedin closed this Oct 16, 2017
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