Sema: Fix source location bookkeeping for 'reasonable time' diagnostic #85293
Conversation
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.
|
@swift-ci Please smoke test |
|
|
||
| private: | ||
| ASTContext &Context; | ||
| ExprOrLocator CurrentAnchor; |
There was a problem hiding this comment.
Could we move this to SolverState instead just like we do with indent?
| Constraint *selectConjunction(); | ||
|
|
||
| void diagnoseTooComplex(SourceLoc fallbackLoc, | ||
| SolutionResult &result); |
There was a problem hiding this comment.
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.
|
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 |
|
@swift-ci Please test Windows |
|
Retest on windows? |
3 participants
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.