Add Identifier wrapper that strips backticks from token text

[adammcarter]

Motivation

Following the issue opened in #1936, take the code below as an example:

let x = y.`z`()

When wanting to pull out the character

`z`

you will have to manually strip the backslashes to get the z character.

The backslashes are completely valid in this case but often using the Swift syntax you might want to remove the backslashes to get the sanitised name of the token for your own purposes which can introduce unnecessary boilerplate.

Solution

Thinking forward, this new Identifier type allows an abstraction over tokens to further sanitise/strip data out, for example, backticks (as in this PR) or future updates like:

• combined characters, e.g. é
• # symbols such as in #if
• comment blocks like /*, /// or //

With the changes in this PR we can take the same example code:

let x = y.`z`()

And by calling [token].identifier.name we can get z without the backticks.

Alternatives considered

Stripping the backslashes when returning the .text is one solution, but the backticks are a valid part of the text and so wouldn't be an accurate way of returning the source code

Trimming the backticks as part of trimmedDescription is a valid and simpler solution IMO, but doesn't allow for future implementations like the above under the Identifier abstraction

[ahoppen]

Thanks for putting up the PR 👍🏽

[ahoppen]

I would prefer to store the name as pure bytes instead of a String. That way the identifier actually represents the raw byte name of the identifier in the source file and doesn’t do any unicode normalization.

The best option would probably to use SyntaxText (TokenSyntax.rawText) but SyntaxText doesn’t own the underlying text buffer, so Identifier would also need to keep the SyntaxArena it was allocated in alive (RetainedSyntaxArena).

CC @rintaro In case you have opinions / thoughts here.

[ahoppen]

I just chatted to @rintaro about this.

What we want is two types:

• An @_spi(RawSyntax) public struct RawIdentifier that stores the name of the identifier without keeping the SyntaxArena alive. This is intended to be used in performance-critical situations that can guarantee that the SyntaxArena stays alive and don’t want to pay the ref-counting overhead for it.
• A public struct Identifier that wraps RawIdentifier and also keeps the syntax arena alive using RetainedSyntaxArena. Identifier should then have a computed property name: String that returns a Unicode-normalized version of the identifier. String is probably what most clients choose to use but for uses in the compiler, we need to be byte accurate because the compiler considers U+00E0 (à LATIN SMALL LETTER A WITH GRAVE) to be different than U+0061 U+0300 (a LATIN SMALL LETTER A followed by ̀ COMBINING GRAVE ACCENT) while Swift’s String performs Unicode normalization and considers them the same (print("\u{e0}" == "\u{61}\u{300}") prints true)

[adammcarter]

Hey @ahoppen thanks for following up on this!

I'm looking in to the above and wanted to clarify whether the RawIdentifier.name needs to be a SyntaxText or a RawSyntax

I've pushed a WIP commit which covers the above and uses a SyntaxText which mostly works (except for the test testRawIdentifier() which I'll look in to after the conclusion of this conversation)

However on attempting to use RawSyntax type I'm getting a bit lost in the code from me being so new to this codebase.

Are you able to:

• clarify if we want RawIdentifier.name to be a SyntaxText or a RawSyntax
• look at my latest WIP commit and provide some input on my attempts so far?

Thanks!

[ahoppen]

RawIdentifier.name should be a SyntaxText. Sorry if that wasn’t clear from my last comment.

[ahoppen]

Thank you! This is going in a great direction. It’s about the design that I had in mind.

[plemarquand]

@adammcarter Are you planning on coming back to this? Do you mind if I put up a patch that addresses these comments?

[plemarquand]

@adammcarter @ahoppen I took a shot at addressing the review comments.

[ahoppen]

Thanks for picking this up @plemarquand! I’ve got a few comments inline. Could you also add an entry to the release notes?

[bnbarham]

@swift-ci please test

[ahoppen]

Nice. Looks great!

[ahoppen]

@swift-ci Please test

[ahoppen]

@swift-ci Please test Windows

[ahoppen]

Could you add Identifier.swift to CMakeLists.txt. swift-syntax is built using CMake when it’s linked into the compiler.

[ahoppen]

@swift-ci Please test

[ahoppen]

@swift-ci Please test