[IDE] Perform extension binding after AST mutation #83613
Conversation
hamishknight
requested review from
bnbarham,
rintaro,
CodaFi,
DougGregor,
artemcm,
tshortli,
hborla,
slavapestov and
xedin
as code owners
This ought to be requestified, but for now let's split it into its own function.
This allows us to lazily bind extensions after mutating the AST, ensuring we don't prematurely kick the building of lookup tables.
Now that we correctly bind extensions after mutating the AST, this is no longer necessary.
hamishknight
force-pushed
the
next-step
branch
from
3746520 to
deecd46
Compare
|
@swift-ci please test |
|
@swift-ci please SourceKit stress test |
hamishknight
changed the title
| @@ -5389,6 +5389,25 @@ class DefaultIsolationInSourceFileRequest | |||
| bool isCached() const { return true; } | |||
| }; | |||
|
|
|||
| /// A request that allows IDE inspection to lazily kick extension binding after | |||
| /// it has finished mutating the AST. This will eventually be subsumed when we | |||
| /// properly requestify extension binding. | |||
There was a problem hiding this comment.
If I understand correctly, this is safe because we currently only perform cached operations in function bodies, right? It's kinda scary to use request evaluator to make sure this being executed only in the initial operation, and to hope cached operations don't add extensions.
But as an intermediate solution, I think this is fine.
There was a problem hiding this comment.
A cached operation should never add new extensions to the same SourceFile (and therefore ModuleDecl), instead when we do top-level cached completions we create a new main module within the existing ASTContext. This is fine though since this request is keyed on the ModuleDecl.
There was a problem hiding this comment.
Yeah I know. I just thought this looked scary. 😅
IDEInspectionSecondPassRequestunfortunately currently relies on mutating the AST after-the-fact, make sure we delay extension binding until after that has happened to ensure we don't attempt to prematurely build incomplete lookup tables. This also allows us to get rid of the ad-hoc extension binding logic intypeCheckContextAt.