Skip to content
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

[ASTGen] Eliminate ASTGen.ASTNode #79368

Merged
merged 1 commit into from
Feb 14, 2025
Merged

[ASTGen] Eliminate ASTGen.ASTNode #79368

merged 1 commit into from
Feb 14, 2025

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Feb 13, 2025

We can simply use BridgedASTNode, no need to have its own ASTNode

Rework BridgedASTNode to make it a tagged pointer.

@rintaro
Copy link
Member Author

rintaro commented Feb 13, 2025

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Feb 13, 2025

Is this 5.8 issue?

home/build-user/swift/lib/ASTGen/Sources/ASTGen/Stmts.swift:100:16: error: value of type 'BridgedASTNode' has no member 'castToDecl'
          item.castToDecl().forEachDeclToHoist(handle)
          ~~~~ ^~~~~~~~~~
/home/build-user/swift/lib/ASTGen/Sources/ASTGen/Stmts.swift:100:16: note: C++ method 'castToDecl' that returns unsafe projection of type 'BridgedDecl' not imported
          item.castToDecl().forEachDeclToHoist(handle)
               ^

Unfortunate 😢

@rintaro rintaro marked this pull request as draft February 13, 2025 21:41
@hamishknight
Copy link
Contributor

@rintaro Does marking it SWIFT_IMPORT_UNSAFE help?

@rintaro
Copy link
Member Author

rintaro commented Feb 13, 2025

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Feb 14, 2025

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Feb 14, 2025

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Feb 14, 2025

SWIFT_IMPORT_UNSAFE did the trick. Thank you @hamishknight !

@rintaro rintaro marked this pull request as ready for review February 14, 2025 04:11
hamishknight
hamishknight approved these changes Feb 14, 2025
View reviewed changes
Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Nice!

@rintaro
[ASTGen] Eliminate ASTGen.ASTNode
f2db96c 
We can simply use `BridgedASTNode`, no need to have its own ASTNode
@rintaro
Copy link
Member Author

rintaro commented Feb 14, 2025

@swift-ci Please smoke test

@rintaro rintaro enabled auto-merge February 14, 2025 17:41
@rintaro rintaro merged commit 137e279 into swiftlang:main Feb 14, 2025
3 checks passed
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

@ahoppen ahoppen Awaiting requested review from ahoppen ahoppen is a code owner

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@CodaFi CodaFi Awaiting requested review from CodaFi CodaFi is a code owner

@hborla hborla Awaiting requested review from hborla hborla is a code owner

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

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