Skip to content

Conversation

@rintaro
Copy link
Member

@rintaro rintaro commented Nov 15, 2023

Explicit argument labels are useful for "Open Quickly..." in Xcode, reduce the fear of miss overload resolution, and better crash backtrace as they might not show parameter types.

notes:

  • generate(xxxExpr:) instead of generateXXXExpr(_:) because it's an argument information. It's' possible that we'd need to generate different things from the same argument. I'd like to reserve the base name for the return value. For example:
      func generateExpr(tupleExpr: TupleExprSyntax) -> BridgedTupleExpr
      func generatePattern(tupleExpr: TupleExprSyntax) -> BridgedTuplePattern
  • I haven't decided what todo with generate(_: Syntax) -> ASTNode maybe generateASTNode(syntax:)?
  • All generate(_: T?) are now generate(optional:) It's still overloaded, but probably enough for disambiguation

Explicit argument labels are useful for "Open Quickly..." in Xcode,
reduce the fear of miss overload resolution, and better crash
backtrace as they might not show parameter types.
@rintaro
Copy link
Member Author

rintaro commented Nov 15, 2023

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Nov 15, 2023

@swift-ci Please smoke test

@ahoppen
Copy link
Member

ahoppen commented Nov 15, 2023

  • I haven't decided what todo with generate(_: Syntax) -> ASTNode maybe generateASTNode(syntax:)?

Since we also have generate(choices:) -> ASTNode, why not have generate(syntax:) -> ASTNode?

  • All generate(_: T?) are now generate(optional:) It's still overloaded, but probably enough for disambiguation

I would have overloaded the generate(xxxExpr:) with the optional variant instead of overloading generate(optional:) but don’t have very strong feelings.

@rintaro rintaro merged commit 553e55f into swiftlang:main Nov 15, 2023
@rintaro
Copy link
Member Author

rintaro commented Nov 15, 2023

I haven't decided what todo with generate(_: Syntax) -> ASTNode maybe generateASTNode(syntax:)?

Since we also have generate(choices:) -> ASTNode, why not have generate(syntax:) -> ASTNode?

I decided to remove generate(_: Syntax) -> ASTNode. It's just unsafe (i.e. not all node kind can generate ASTNode), and I believe it's actually not needed.

All generate(_: T?) are now generate(optional:) It's still overloaded, but probably enough for disambiguation

I would have overloaded the generate(xxxExpr:) with the optional variant instead of overloading generate(optional:) but don’t have very strong feelings.

I'm going to generalize optional node handling in followups. Defining this for each optional syntax kind is too much boilerplate.

@ahoppen
Copy link
Member

ahoppen commented Nov 15, 2023

I'm going to generalize optional node handling in followups. Defining this for each optional syntax kind is too much boilerplate.

If you have ideas for that, I’m happy to see them.

@rintaro
Copy link
Member Author

rintaro commented Nov 15, 2023

#69894

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.

2 participants