Skip to content

[6.0][PackageCMO] Don't allow modifying AST #74670

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.

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

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Conversation

elsh
Copy link
Contributor

@elsh elsh commented Jun 24, 2024

This PR prevents @usableFromInline attribute from being added to an internal type, which modifies AST and should not be allowed in PackageCMO. Also ensures that the generated interfaces with PackageCMO enabled are not affected by the optimization and do not contain modified AST.

rdar://130292190

main: #74641

@elsh elsh requested a review from a team as a code owner June 24, 2024 23:42
@elsh elsh added the swift 6.0 label Jun 24, 2024
@elsh
Copy link
Contributor Author

elsh commented Jun 24, 2024

@swift-ci test

@elsh elsh requested a review from nkcsgexi June 24, 2024 23:45
@tbkka
Copy link
Contributor

tbkka commented Jun 24, 2024

Looks like the main PR still has a "requested changes" review from @eeckstein. You should address that before cherry-picking to the 6.0 branch.

@nkcsgexi nkcsgexi requested a review from eeckstein June 25, 2024 02:06
@elsh elsh force-pushed the elsh/rel-pcmo-ufi branch from 937adaf to 2d91ead Compare June 26, 2024 00:24
@elsh
Copy link
Contributor Author

elsh commented Jun 26, 2024

@swift-ci test

@elsh elsh force-pushed the elsh/rel-pcmo-ufi branch from 2d91ead to f029bfa Compare June 26, 2024 00:30
@elsh
Copy link
Contributor Author

elsh commented Jun 26, 2024

@swift-ci test

@elsh elsh force-pushed the elsh/rel-pcmo-ufi branch from f029bfa to 7594bda Compare June 26, 2024 01:11
 Currently not all types are visited in canSerialize* calls, sometimes
 resulting in an internal type getting @usableFromInline, which is
 incorrect.

 For example, for `let q = P() as? Q`, where Q is an internal class
 inherting a public class P, Q is not visited in the canSerialize*
 checks, thus resulting in `@usableFromInline class Q`; this is not
 the intended behavior in the conservative mode used by PackageCMO
 as it modifies AST.

 To properly fix, instruction visitor needs to be refactored to do
 both the "canSerialize" check (that visits all types) and serialize
 or update visibility (modify AST in non-conservative modes).

 This PR provides a short-term fix that prevents modifying AST, and
 also ensures that the generated interfaces with PackageCMO flags
 are not affected by the optimization or contain modified AST.

rdar://130292190
@elsh elsh force-pushed the elsh/rel-pcmo-ufi branch from 7594bda to 827280a Compare June 26, 2024 01:16
@elsh
Copy link
Contributor Author

elsh commented Jun 26, 2024

@swift-ci test

@nkcsgexi nkcsgexi merged commit 14b3b61 into release/6.0 Jun 26, 2024
5 checks passed
@nkcsgexi nkcsgexi deleted the elsh/rel-pcmo-ufi branch June 26, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants