Skip to content

Add _mangledTypeName to allow round trips T->mangledName->T #30318

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 3 commits into from
Mar 31, 2020

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 10, 2020

This introduces a new _underscored function that allows to perform name mangling of types in-process. This can be used to perform round-trips from a type, to a mangled name, and back again.

This is useful in certain serialization scenarios, and just generally "completes the package" - since we can go from mangled names to Types, why not go the other way around in runtime as well.

cc @DougGregor @jckarter
cc @milseman in case there's something stdlib specific I should adjust here (naming, attributes etc)


While this does_ NOT_ address any of the API needs as discussed in SE-0262: Demangle function, it adds another piece to eventually complete the puzzle. I.e. perhaps in the future we can revise SE-262 to be about mangling in general, and then would also include this ability there?

This is out of scope of this PR and would be taken through the Swift Evolution process however.

@drexin
Copy link
Contributor

drexin commented Mar 10, 2020

@swift-ci smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2020

Whoop, fixed the:

13:37:28 +Func _getMangledTypeName(_:) is a new API without @available attribute

@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2020

@swift-ci smoke test

1 similar comment
@drexin
Copy link
Contributor

drexin commented Mar 10, 2020

@swift-ci smoke test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@ktoso
Copy link
Contributor Author

ktoso commented Mar 11, 2020

@swift-ci test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 11, 2020

Cool, thanks, my understanding is I should also run all tests -- doing so then :)

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 3344448

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 3344448

@ktoso
Copy link
Contributor Author

ktoso commented Mar 12, 2020

Failure just on windows and does not seem related to this change.
Anything else I should do here? :-)


return String._fromUTF8Repairing(
UnsafeBufferPointer(start: stringPtr, count: count)).0
}
Copy link
Member

Choose a reason for hiding this comment

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

What do we want to do here if the input contains invalid UTF-8? It seems like our mangling should only produce ASCII, so you can use String._fromASCIIValidating which returns nil of non-ASCII, and trap in that scenario.

Copy link
Member

Choose a reason for hiding this comment

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

CC @eeckstein @DougGregor @jckarter though to verify the ASCII-ness of mangled names for types with non-ASCII names

Copy link
Member

Choose a reason for hiding this comment

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

CC @eeckstein @DougGregor @jckarter though to verify the ASCII-ness of mangled names for types with non-ASCII names

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's the case. Non-ASCII characters in identifiers are encoded to ASCII with the Punycode algorithm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Punycode encoding is not used in all contexts; we also support disabling it for runtime mangled names, since we aren't constrained by linker limitations there. The mangled string should always be valid UTF-8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So... sounds like nothing to be done here then?

Anything else I can help to get this merged?

I would be happy to backport as well if possible (?), which branches should I target for "next" 5.1 / 5.2?

Copy link
Member

Choose a reason for hiding this comment

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

This LGTM, but I would suggest adding a precondition that the repairsMade in the return value was false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 34b6e4f

@jckarter
Copy link
Contributor

jckarter commented Mar 17, 2020

This looks ready to merge to me. @milseman Any other concerns before we merge this?

@milseman
Copy link
Member

LGTM, with a preference for a precondition that no repairs were made

@milseman
Copy link
Member

@swift-ci please test windows platform

@ktoso
Copy link
Contributor Author

ktoso commented Mar 18, 2020

Ok let me have a look at the precondition then 👍

@ktoso
Copy link
Contributor Author

ktoso commented Mar 18, 2020

Addressed the precondition feedback @milseman: 34b6e4f :-)

Once merged I'll submit a backport to swift-5.1-branch as well 👍

Thanks everyone!

@ktoso
Copy link
Contributor Author

ktoso commented Mar 18, 2020

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ea6b69e

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ea6b69e

@ktoso ktoso requested review from jckarter and milseman March 20, 2020 02:10
Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

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

LGTM

@ktoso
Copy link
Contributor Author

ktoso commented Mar 31, 2020

Hey guys, any chance to get this merged @jckarter @milseman ?

@jckarter jckarter merged commit 026b8b3 into swiftlang:master Mar 31, 2020
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.

7 participants