Skip to content

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Sep 23, 2020

Port frontend changes for LTO from swiftlang/swift#32430

CC: @DougGregor

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

This all looks pretty reasonable to me, just added a couple minor comments and it looks like there's a merge conflict that needs to be resolved. Thanks for porting this!

@kateinoigakukun
Copy link
Member Author

@owenv Thank you for your review! I addressed your comments 👍

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you @kateinoigakukun.

CC @steveapple, this PR and relevant portions of swiftlang/swift#32430 might be of interest to: #252

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@owenv
Copy link
Contributor

owenv commented Sep 24, 2020

@swift-ci test

@owenv
Copy link
Contributor

owenv commented Sep 24, 2020

error: 'lto' is inaccessible due to 'internal' protection level
21:22:54     XCTAssertEqual(try Driver(args: ["swiftc"]).lto, nil)

@kateinoigakukun I didn't notice the lto property is used in one of the tests, sorry about that! If it's changed to @_spi(Testing) public everything should work.

@kateinoigakukun
Copy link
Member Author

@owensd Thanks for teaching me!

@owenv
Copy link
Contributor

owenv commented Sep 24, 2020

@swift-ci test

1 similar comment
@owenv
Copy link
Contributor

owenv commented Sep 24, 2020

@swift-ci test

@owenv owenv merged commit a04d98e into swiftlang:main Sep 24, 2020
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.

3 participants