[Async Refactoring] Better handle bool flag parameter bindings #39159
Conversation
|
@swift-ci please test |
lib/IDE/Refactoring.cpp
Outdated
| SmallVector<const ParamDecl *, 4> Scratch; | ||
| auto SuccessBindings = CallbackParams.getSuccessParamsToBind(Scratch); |
There was a problem hiding this comment.
Given this is the only use I'd prefer just returning the SmallVector from getSuccessParamsToBind.
And not sure if this part is worth it or not but...
- Could name
SuccessBindingsjustBindings - Add an
ArrayRefSuccessBindingsofBindings - Remove
AllBindingsand just addErrParamtoBindingsin the if
There was a problem hiding this comment.
Hmm, returning a SmallVector directly would mean the callee would have to specify the inline buffer size, which feels odd – maybe we could return a llvm::TinyPtrVector or std::vector?
Regarding mutating a single Bindings vector directly, I tend to prefer reducing mutability where I can, but this is all pretty localised so I don't feel too strongly about it
There was a problem hiding this comment.
I ended up generalising this logic to not need the mutation. I did switch over using llvm::TinyPtrVector, I don't really feel too strongly about the return type tho
There was a problem hiding this comment.
TinyPtrVector is optimised for 0/1 right? I guess that is probably almost always true for the success/error. I'm fine with specifying the inline buffer size in the return type, but I don't really mind. Personally I'd still use SmallVector<Thing, 0> over a vector, even if I was returning it.
There was a problem hiding this comment.
Yeah TinyPtrVector is optimized for the single element (and empty) case, which as you say is fairly likely for the success/error params. Out of curiosity, what benefits would SmallVector<Thing, 0> have over a regular vector?
Refactor some closure parameter handling logic into a new ClosureCallbackParams type, which lets us clean up some of the parameters passed to CallbackClassifier, and better distinguish between an error param and a Result param.
If we have a known bool flag parameter, drop it from the success parameters being bound in a refactored await call, as the async variant drops it. rdar://81896460
Make sure to convert any references to parameters that get dropped in the fallback case to placeholders.
Generalize the logic to handle different BlockKinds, and add binding logic that lets us assign `true` or `false` to the given bool success param in the fallback case.
hamishknight
force-pushed
the
conditions-apply
branch
from
a0f6f0c to
72070b7
Compare
|
@swift-ci please test |
hamishknight
changed the title
If we have a known bool flag parameter, drop it from the success parameters being bound in a refactored await call, as the async variant drops it. And in the fallback case, add a binding for it in the success and error block.
rdar://81896460