Skip to content

[RemoteAST] Fix metadata reader to properly read tuple type metadata #12766

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
Nov 6, 2017

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Nov 5, 2017

Because readMetadata correctly reads tuple type metadata bounds,
there is no need to call for remote read of the memory allocated
for element types, since they are going to be laid out right after
tuple metadata itself and could be accessed using accessors in
TargetTupleTypeMetadata.

Because `readMetadata` correctly reads tuple type metadata bounds,
there is no need to call for remote read of the memory allocated
for element types, since they are going to be laid out right after
tuple metadata itself and could be accessed using accessors in
`TargetTupleTypeMetadata`.
@xedin xedin requested a review from rjmccall November 5, 2017 00:57
@xedin
Copy link
Contributor Author

xedin commented Nov 5, 2017

@rjmccall In PR #12500 you mentioned that function metadata should be read the same way as tuple metadata, I've noticed that reader might be improved for tuple metadata as well which was still reading using remote mechanism. WDYT?

@xedin
Copy link
Contributor Author

xedin commented Nov 5, 2017

@swift-ci please smoke test

@rjmccall
Copy link
Contributor

rjmccall commented Nov 6, 2017

Yes, this looks like a very nice improvement (and a good catch about tuple elements).

@xedin
Copy link
Contributor Author

xedin commented Nov 6, 2017

@swift-ci please test and merge

@swift-ci swift-ci merged commit b4899f1 into swiftlang:master Nov 6, 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