swiftlang/swift-syntax

Reduce number of `raw:` interpolations in CodeGeneration

Closed this issue · 5 comments

ahoppen commented

CodeGeneration still uses many raw: interpolations, which should be avoided unless it’s really necessary. Some of these raw: interpolations don’t need to the raw: label anymore. For some others, we could restructure how nodes are represented in SyntaxSupport to use TokenSyntax instead of String, similar to what I’ve done in #1914.

ahoppen commented

Tracked in Apple’s issue tracker as rdar://113237456

Matejkob commented

I've grabbed the lowest-hanging fruit and removed all the no-longer-needed raw: interpolations. Further reductions in the use of raw: will require changing types.

  • ❤️1
Matejkob commented

So actually, if I've already started doing things related to this issue, I can do the whole thing.

  • 👍1
Matejkob commented

We've already eliminated most of the raw: interpolations in CodeGeneration. I have a few more potential places in mind where we could eliminate them, but some of them seem to be incorrect. I would love to hear your feedback @ahoppen @kimdv

  • Node.getter:parserFunction - I'm thinking of changing the type to TokenSyntax. If I'm not mistaken, the function identifier is of this type, right?
  • let optionalMark = child.isOptional ? "?" : "" into let optionalMark: TokenSyntax = child.isOptional ? TokenSyntax.postfixQuestionMarkToken() : TokenSyntax(""). The second branch of this condition seems to be incorrect. TokenSyntax = "" doesn't feel right, does it?
  • let iuoMark = child.isOptional ? "" : "!" into let iuoMark: TokenSyntax = child.isOptional ? TokenSyntax("") : TokenSyntax.exclamationMarkToken().

Let me know if there are any issues with these changes or if you can identify other potential usages of raw: that could be eliminated but I've missed them.

ahoppen commented
  • Node.getter:parserFunction - I'm thinking of changing the type to TokenSyntax. If I'm not mistaken, the function identifier is of this type, right?

Yes, that seems like a good change to me

  • let optionalMark = child.isOptional ? "?" : "" into let optionalMark: TokenSyntax = child.isOptional ? TokenSyntax.postfixQuestionMarkToken() : TokenSyntax(""). The second branch of this condition seems to be incorrect. TokenSyntax = "" doesn't feel right, does it?
  • let iuoMark = child.isOptional ? "" : "!" into let iuoMark: TokenSyntax = child.isOptional ? TokenSyntax("") : TokenSyntax.exclamationMarkToken().

You are correct, TokenSyntax("") doesn’t make any sense. What we could do, is add a new appendInterpolation method to SyntaxStringInterpolation that takes an optional syntax nodes. If the node is nil, it just doesn’t add any interpolation. In that case you could have let optionalMark = child.isOptional ? TokenSyntax.postfixQuestionMarkToken() : nil.

  • ❤️1