[Sema] Some more cleanup now that we migrated all of code completion to solver-based #68363
Conversation
Since we migrated all of code completion to solver-based, this flag is no longer needed.
ahoppen
force-pushed
the
ahoppen/more-cc-cleanup
branch
from
c97ed7a to
52e5d1a
Compare
ahoppen
requested review from
hborla,
xedin,
slavapestov,
bnbarham,
rintaro and
hamishknight
as code owners
|
@swift-ci Please smoke test |
|
@swift-ci Please SourceKit stress test |
…equest` Since we migrated all of code completion to solver-based, this is no longer needed.
…nce of `CompletionCallback` This shouldn’t really change anything but is just a little cleaner.
…rticipatesInInference` It looks like we no longer need this check.
ahoppen
force-pushed
the
ahoppen/more-cc-cleanup
branch
from
52e5d1a to
66a0b78
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please SourceKit stress test |
| @@ -7311,24 +7311,13 @@ bool ConstraintSystem::participatesInInference(ClosureExpr *closure) const { | |||
| if (getAppliedResultBuilderTransform(closure)) | |||
| return true; | |||
|
|
|||
| if (closure->hasSingleExpressionBody()) | |||
| return true; | |||
|
|
|||
| if (Options.contains(ConstraintSystemFlags::LeaveClosureBodyUnchecked)) | |||
| return false; | |||
There was a problem hiding this comment.
Ah, I think if we're keeping the LeaveClosureBodyUnchecked flag, then we'll probably want to restore the hasSingleExpressionBody check since I assume LeaveClosureBodyUnchecked is only meant to apply to multi-statement closures?
There was a problem hiding this comment.
You might be right. But things are looking good right now even without the check. Let’s see what the stress tester has to say
There was a problem hiding this comment.
Appears to be fine even with closure->hasSingleExpressionBody removed.
There was a problem hiding this comment.
I don't think hasSingleExpressionBody is necessary and you can remove isInResultBuilderContext method too since this was the last use of it. The only question I have is why do we have this setup in a way that getAppliedResultBuilderTransform doesn't respect LeaveClosureBodyUnchecked...
I think this method should look as follows:
return !(Options.contains(ConstraintSystemFlags::LeaveClosureBodyUnchecked) || closure->hasEmptyBody());There was a problem hiding this comment.
Isn't getAppliedResultBuilderTransform needed because we still generate a result builder body that needs type-checking even if the parsed body is empty?
There was a problem hiding this comment.
Good point, I think we should add an method to get a body for a closure and use the result to check whether it's empty or not, seems like that would minimize the risk of making mistakes like that...
ClosuresInResultBuilderDontParticipateInInferenceflag from the constraint systemTypeCheckASTNodeAtLocRequestcs.isForCodeCompletion()instead of checking for presence ofCompletionCallback