-
Notifications
You must be signed in to change notification settings - Fork 104
Synthesize display names for de facto suites with raw identifiers. #1105
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
base: main
Are you sure you want to change the base?
Conversation
This PR ensures that suite types that don't have the `@Suite` attribute but which _do_ have raw identifiers for names are correctly given display names the same way those with `@Suite` would be. This PR also ensures that we transform spaces in raw identifiers after they are demangled by the runtime--namely, the runtime replaces ASCII spaces (as typed by the user) with Unicode non-breaking spaces (which aren't otherwise valid in raw identifers) in order to avoid issues with existing uses of spaces in demangled names. We want to make sure that identifiers as presented to the user match what the user has typed, so we need to transform these spaces back. No changes in this area are needed for display names derived during macro expansion because we do the relevant work based on the source text which still has the original ASCII spaces. This PR also deletes the "raw$" hack that I put in place when originally implementing raw identifier support as the entire toolchain supports them now. Resolves #1104.
@swift-ci test |
@swift-ci test |
https://ci-external.swift.org/job/swift-testing-pull-request-windows/1474/consoleText Looks like the Windows toolchain doesn't support these identifiers yet @allevato @compnerd. |
@swift-ci test |
@swift-ci test |
@swift-ci test |
I'm not sure what's going on there; I wouldn't have been able to merge the original support if it didn't work on Windows. Both the old C++ parser and the Swift parser handle them. Any ideas @compnerd ? |
return nil | ||
} | ||
|
||
let result = rawIdentifier.lazy.map { c in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important: My experience has led me to operate on Unicode scalar views in situations like this where I don't need grapheme clustering so I don't pay the extra cost of it. It's probably not going to be a big bottleneck here, but that's just my habit 🤷🏻♂️
This PR ensures that suite types that don't have the
@Suite
attribute but which do have raw identifiers for names are correctly given display names the same way those with@Suite
would be. This PR also ensures that we transform spaces in raw identifiers after they are demangled by the runtime--namely, the runtime replaces ASCII spaces (as typed by the user) with Unicode non-breaking spaces (which aren't otherwise valid in raw identifers) in order to avoid issues with existing uses of spaces in demangled names. We want to make sure that identifiers as presented to the user match what the user has typed, so we need to transform these spaces back. No changes in this area are needed for display names derived during macro expansion because we do the relevant work based on the source text which still has the original ASCII spaces.This PR also deletes the "
raw$
" hack that I put in place when originally implementing raw identifier support as the entire toolchain supports them now.Resolves #1104.
Checklist: