Skip to content

Conversation

@slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Nov 3, 2025

We already had bookkeeping to track which statement in a multi-statement closure we were looking at, but this was only used for the 'reasonable time' diagnostic in the case that we hit the expression timer, which was almost never hit, and is now off by default. The scope, memory, and trail limits couldn't use this information, so they would always diagnose the entire target being type checked.

Move it up from ExpressionTimer to ConstraintSystem, so that we get the right source location there too. Also, factor out some code duplication in BuilderTransform to ensure we get the same benefit for result builders applied to function bodies too.

We already had bookkeeping to track which statement in a multi-statement
closure we were looking at, but this was only used for the 'reasonable time'
diagnostic in the case that we hit the expression timer, which was almost
never hit, and is now off by default. The scope, memory, and trial limits
couldn't use this information, so they would always diagnose the entire
target being type checked.

Move it up from ExpressionTimer to ConstraintSystem, so that we get the
right source location there too. Also, factor out some code duplication
in BuilderTransform to ensure we get the same benefit for result builders
applied to function bodies too.
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test


private:
ASTContext &Context;
ExprOrLocator CurrentAnchor;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we move this to SolverState instead just like we do with indent?

Constraint *selectConjunction();

void diagnoseTooComplex(SourceLoc fallbackLoc,
SolutionResult &result);
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are willing to work on this I think we should move all this logic into steps instead of the constraint system. In general I think constraint system should have to deal with anything but constraints. The context of "too complex" belongs to the solver itself IMHO, the algorithm that drives the constraint processing.

@slavapestov
Copy link
Contributor Author

We discussed some future refactoring plans offline here, going to land this PR as-is since it fixes the immediate issue of ExpressionTimer being too specific of a place for this logic, and then move some more stuff around later

@slavapestov slavapestov enabled auto-merge November 4, 2025 01:19
@slavapestov
Copy link
Contributor Author

@swift-ci Please test Windows

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