Skip to content

SR-12022: refactor LiteralExpr to combine common initializer code #34132

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
Oct 12, 2020
Merged

SR-12022: refactor LiteralExpr to combine common initializer code #34132

merged 1 commit into from
Oct 12, 2020

Conversation

Interfere
Copy link
Contributor

Revisit inheritance chain for LiteralExpr.

  • Hoist get/setInitializer() methods to abstract class LiteralExpr;
  • Introduce new abstract class BuiltinLiteralExpr and hoist get/setBuiltinInitializer methods to it;
  • Simplify implementation of SILGenFunction::emitLiteral();
  • Rename get/setResultInit() ~> get/setInitializer();
  • Update include/swift/AST/ExprNodes.def.

Resolves SR-12022.

@Interfere
Copy link
Contributor Author

@brentdax could you please review?

@theblixguy theblixguy requested a review from beccadax September 30, 2020 20:50
xedin
xedin approved these changes Sep 30, 2020
View reviewed changes
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Type-checker changes look good to me!

theblixguy
theblixguy reviewed Sep 30, 2020
View reviewed changes
@Interfere Interfere force-pushed the SR-12022-LiteralExpr-refactoring branch from 60ef4b6 to b908e9e Compare October 1, 2020 12:37
@Interfere Interfere requested a review from theblixguy October 1, 2020 12:38
theblixguy
theblixguy reviewed Oct 1, 2020
View reviewed changes
@Interfere Interfere force-pushed the SR-12022-LiteralExpr-refactoring branch from b908e9e to 4fa17bf Compare October 1, 2020 12:45
@Interfere Interfere requested a review from theblixguy October 1, 2020 12:45
@CodaFi
Copy link
Contributor

CodaFi commented Oct 1, 2020

@swift-ci test

@CodaFi
Copy link
Contributor

CodaFi commented Oct 1, 2020

@swift-ci test source compatibility

@swift-ci
Copy link
Contributor

swift-ci commented Oct 1, 2020

Build failed
Swift Test OS X Platform
Git Sha - 4fa17bf

@swift-ci
Copy link
Contributor

swift-ci commented Oct 1, 2020

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

@Interfere
Copy link
Contributor Author

Build failed
Swift Test OS X Platform
Git Sha - 4fa17bf

Screenshot 2020-10-05 at 12 19 16

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

Screenshot 2020-10-05 at 12 18 58

@CodaFi Is this a CI related errors, or have I missed something in launching the test fixtures locally? Could you please relaunch? Thanks.

@xedin
Copy link
Contributor

xedin commented Oct 5, 2020

That’s a CI related issue. Fixed by updating cmake. Let’s re-run everything.

@xedin
Copy link
Contributor

xedin commented Oct 5, 2020

@swift-ci please test

@xedin
Copy link
Contributor

xedin commented Oct 5, 2020

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

swift-ci commented Oct 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

@Interfere
Copy link
Contributor Author

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

Screenshot 2020-10-05 at 19 34 01

@xedin
Copy link
Contributor

xedin commented Oct 5, 2020

@swift-ci please test Linux platform

@swift-ci
Copy link
Contributor

swift-ci commented Oct 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

@xedin
Copy link
Contributor

xedin commented Oct 5, 2020

@swift-ci please test Linux platform

@swift-ci
Copy link
Contributor

swift-ci commented Oct 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

@swift-ci
Copy link
Contributor

swift-ci commented Oct 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 4fa17bf

@xedin
Copy link
Contributor

xedin commented Oct 11, 2020

@swift-ci please test

@xedin
Copy link
Contributor

xedin commented Oct 12, 2020

I'm just going to land this, we could always revert later if something comes up. Thank you, @Interfere!

@xedin xedin merged commit 64d4465 into swiftlang:main Oct 12, 2020
@swift-ci swift-ci mentioned this pull request Sep 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin approved these changes

@beccadax beccadax Awaiting requested review from beccadax

@theblixguy theblixguy Awaiting requested review from theblixguy

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

Successfully merging this pull request may close these issues.
5 participants
@Interfere @CodaFi @swift-ci @xedin @theblixguy