Skip to content

[AST] Change some builtins to conform their result to Escapable #72594

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 1 commit into from
Mar 26, 2024
Merged

[AST] Change some builtins to conform their result to Escapable #72594

merged 1 commit into from
Mar 26, 2024

Conversation

Azoy
Copy link
Contributor

@Azoy Azoy commented Mar 26, 2024

Some of these builtins were not constraining their result type to conform to Escapable. This caused some inference failures when building the standard library.

Specifically I changed:

  • Builtin.load
  • Builtin.take
  • Builtin.copy
  • Builtin.allocWithTailElems_N
  • Builtin.COWBufferForReading
  • Builtin.castReference
  • Builtin.reinterpretCast
  • Builtin.convertUnownedUnsafeToGuaranteed

@Azoy Azoy requested review from atrick and meg-gupta March 26, 2024 18:05
@Azoy
Copy link
Contributor Author

Azoy commented Mar 26, 2024

@swift-ci please smoke test

meg-gupta
meg-gupta approved these changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@meg-gupta meg-gupta left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

atrick
atrick approved these changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Thanks!

Eventually, we'll need to support Builtin.load and Builtin.take with ~Escapable elements. We'll need to figure out a way around the type checker when that time comes.

@atrick
Copy link
Contributor

atrick commented Mar 26, 2024

Merging because I want to start filing downstream bugs. Apologies for not waiting for a full CI test.

@atrick atrick merged commit e5bf933 into swiftlang:main Mar 26, 2024
@Azoy Azoy deleted the fix-builtins-escapable branch April 18, 2024 16:14
Azoy pushed a commit to Azoy/swift that referenced this pull request Apr 18, 2024
@atrick @Azoy
Merge pull request swiftlang#72594 from Azoy/fix-builtins-escapable
3ec307d 
[AST] Change some builtins to conform their result to Escapable
@Azoy Azoy mentioned this pull request Apr 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atrick atrick atrick approved these changes

@meg-gupta meg-gupta meg-gupta approved these changes

@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.
3 participants
@Azoy @atrick @meg-gupta