Skip to content

[ASTGen] Implement some expressions and types #70281

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Dec 8, 2023
Merged

[ASTGen] Implement some expressions and types #70281

merged 5 commits into from
Dec 8, 2023

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Dec 6, 2023 •
edited
Loading

  • <expr>.self expression
  • dictionary literal
  • bunch of unary expression try, copy, _borrow, await, etc.
  • each repeat expression and type
  • ~ type
  • class constraint

@rintaro
Copy link
Member Author

rintaro commented Dec 6, 2023

@swift-ci Please smoke test

rintaro
rintaro commented Dec 7, 2023
View reviewed changes
lib/AST/ASTBridging.cpp
Comment on lines +1331 to +1332
BridgedTypeRepr
BridgedInverseTypeRepr_createParsed(BridgedASTContext cContext,
Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed all the current BridgedXXXTyprRepr_createParsed() returns BridgedTypeRepr instead of the concrete wrapper types. Since we haven't implemented checked down casting, I think it'd be better to keep the sub type as long as possible (and that's consistent with Decl/Stmt/Expr). WDYT @hamishknight ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this was just done for convenience since nothing on the ASTGen side cared about the kind of TypeRepr at the time, changing it to use the subclasses makes sense to me

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Will do in a followup

hamishknight
hamishknight approved these changes Dec 7, 2023
View reviewed changes
test/ASTGen/verify-parse.swift
@@ -209,7 +209,7 @@ struct TestStruct {

func test() {
_ = method(arg:_:)
_ = self.method(arg:_:)
_ = self.method(arg:_:).self
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding a test for .`self`?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea 👍 Added

include/swift/AST/ASTBridging.h Outdated
Comment on lines 708 to 709
SWIFT_NAME("BridgedDictionaryExpr.createParsed(_:lBracketLoc:elements:"
"colonLocs:RBracketLoc:)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SWIFT_NAME("BridgedDictionaryExpr.createParsed(_:lBracketLoc:elements:"
"colonLocs:RBracketLoc:)")
SWIFT_NAME("BridgedDictionaryExpr.createParsed(_:lBracketLoc:elements:"
"colonLocs:rBracketLoc:)")

lib/ASTGen/Sources/ASTGen/Types.swift
break
case // Known unimplemented kinds.
.suppressedType, .packElementType, .missingType:
.missingType:
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all the types migrated now? MissingTypeSyntax should only occur in invalid syntax trees AFAIK, I guess we just need some diagnostic logic for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

For valid syntax, yeah I think so. As for the diagnostics, I think the parser already emitted some. I feel like just generating ErrorTypeRepr might be enough, but I'm not sure. Maybe we need to emit nullptr for the entire type syntax.

@rintaro rintaro force-pushed the astgen-dotselfexpr branch from 901b3eb to 8f70dc3 Compare December 8, 2023 20:15
@rintaro
Copy link
Member Author

rintaro commented Dec 8, 2023

@swift-ci Please smoke test

@rintaro rintaro merged commit d3bf844 into swiftlang:main Dec 8, 2023
drodriguez added a commit to drodriguez/swift that referenced this pull request Dec 15, 2023
@drodriguez
[test] Mark some tests as requiring Swift Syntax
fdcdb42 
Some of the tests now depend on features provided by Swift Syntax, and
will fail to pass if building without Swift Syntax support.

This commits marks all the tests I know fail to pass when Swift Syntax
is disabled.

- swiftlang#70281 modified `ASTGen/verify-parse.swift`
- swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift`
- swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly
  modified all `refactoring/ExtractFunction/*` tests.
@drodriguez drodriguez mentioned this pull request Dec 15, 2023
Merged
drodriguez added a commit to drodriguez/swift that referenced this pull request Dec 15, 2023
@drodriguez
[test] Mark some tests as requiring Swift Syntax
618a77d 
Some of the tests now depend on features provided by Swift Syntax, and
will fail to pass if building without Swift Syntax support.

This commits marks all the tests I know fail to pass when Swift Syntax
is disabled.

- swiftlang#70281 modified `ASTGen/verify-parse.swift`
- swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift`
- swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly
  modified all `refactoring/ExtractFunction/*` tests.
drodriguez added a commit that referenced this pull request Dec 15, 2023
@drodriguez
[test] Mark some tests as requiring Swift Syntax (#70493)
e60785e 
Some of the tests now depend on features provided by Swift Syntax, and
will fail to pass if building without Swift Syntax support.

This commits marks all the tests I know fail to pass when Swift Syntax
is disabled.

- #70281 modified `ASTGen/verify-parse.swift`
- #70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift`
- #70389 modified `SourceKit/Refactoring/basic.swift` and indirectly
  modified all `refactoring/ExtractFunction/*` tests.
meg-gupta pushed a commit to meg-gupta/swift that referenced this pull request Dec 21, 2023
@drodriguez @meg-gupta
[test] Mark some tests as requiring Swift Syntax (swiftlang#70493)
af5a60d 
Some of the tests now depend on features provided by Swift Syntax, and
will fail to pass if building without Swift Syntax support.

This commits marks all the tests I know fail to pass when Swift Syntax
is disabled.

- swiftlang#70281 modified `ASTGen/verify-parse.swift`
- swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift`
- swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly
  modified all `refactoring/ExtractFunction/*` tests.
Catfish-Man pushed a commit to Catfish-Man/swift that referenced this pull request Jan 19, 2024
@drodriguez @Catfish-Man
[test] Mark some tests as requiring Swift Syntax (swiftlang#70493)
9637e52 
Some of the tests now depend on features provided by Swift Syntax, and
will fail to pass if building without Swift Syntax support.

This commits marks all the tests I know fail to pass when Swift Syntax
is disabled.

- swiftlang#70281 modified `ASTGen/verify-parse.swift`
- swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift`
- swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly
  modified all `refactoring/ExtractFunction/*` tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hamishknight hamishknight hamishknight approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rintaro @hamishknight