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.

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

[ConstraintSystem] Remove implementation of operator designated types in the solver. #34315

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

hborla
Copy link
Member

@hborla hborla commented Oct 14, 2020

Remove the -solver-enable-operator-designated-types flag and implementation in the solver. This experimental feature is no longer being explored, and we're implementing other approaches to improving constraint solver performance that break this feature.

There's a little more to remove (the -enable-operator-designated-types flag enables designated types in the parser, and designated type syntax is adopted in the standard library) but this lets me make more progress on chipping away at solver performance hacks.

@hborla hborla requested a review from xedin October 14, 2020 23:19
@hborla
Copy link
Member Author

hborla commented Oct 14, 2020

@swift-ci please smoke test

// us to eliminate behavior that is exponential in the number of
// operators in the expression.
if (getASTContext().TypeCheckerOpts.SolverEnableOperatorDesignatedTypes) {
if (auto *disjunction = selectApplyDisjunction())
Copy link
Contributor

Choose a reason for hiding this comment

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

I think selectApplyDisjunction could also be removed now right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah you're right, thanks for catching that!

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.

LGTM! I left a comment inline about selectApplyDisjunction.

by the operator designated types implementation.
@hborla
Copy link
Member Author

hborla commented Oct 15, 2020

@swift-ci please smoke test

@hborla hborla merged commit 529660d into swiftlang:main Oct 15, 2020
@hborla hborla deleted the remove-operator-designated-types branch October 15, 2020 16:02
beccadax added a commit to beccadax/swift that referenced this pull request Aug 3, 2021
Designated types were removed from the constraint solver in swiftlang#34315, but they are currently still represented in the AST and fully checked. This change removes them as completely as possible without breaking source compatibility (mainly with old swiftinterfaces) or changing the SwiftSyntax tree. Designated types are still parsed, but they are dropped immediately and a warning is diagnosed. During decl checking we also still check if the precedence group is really a designated type, but only so that we can diagnose a warning and fall back to DefaultPrecedence.

This change also fixes an apparent bug in the parser where we did not diagnose operator declarations that contained a `:` followed by a non-identifier token.
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.

2 participants