Hello all,
For context, I spent the weekend seeing what I could do about SE-0075 <https://bugs.swift.org/browse/SR-1560>\. From the last conversation I had with Jordan (which is summed up in the linked JIRA ticket pretty well), this kind of operation is currently very expensive and hence not as simple a matter as just making Parse ask the ASTContext to load modules. I wound up with a patch <https://github.com/apple/swift/compare/master...CodaFi:fine-imported-goods> that mostly divests Parse of the responsibility of resolving compilation condition blocks. This defers the bulk of the resolution work to a walker in NameBinding (increasingly, a file becoming less and less about binding names!). Problems arose when I thought about how best to resolve these condition clauses in the walker and I tried replicating the gist of what Parse was doing anyway.
For reference, when the parser encounters a #if in the middle of parsing a statement it parses the conditions and their bodies then evaluates them on the spot and reparents the freshly-parsed statements under the “active clause” to whatever enclosing aggregate statement they belong to (usually the top-level or a braced statement block). For decls, the logic is a bit more convoluted due to the tangled web of callbacks in parseDecl, but it does much the same thing wherein active decls are reparented as members of the enclosing aggregate declaration.
From the Walker’s perspective, this means traversing the AST, evaluating any latent #if-conditions, and reparenting statements and declarations thereby “collapsing” the config (or marking it evaluated, etc.). The approach in the NameBinding part of the patch is messy, but it works for simple cases. It gets into trouble when in parse-as-library mode tries to reparent decls to the SourceFile itself because there is no longer a TopLevelCodeDecl they can be a part of. The approach in the patch causes serious issues with decl scoping as the reparenting process doesn’t actually switch the scopes of top-level decls and they wind up being deallocated prematurely. This Crashes The Compiler™.
I know there are more principled ways of approaching this, and I’m wondering what ideas anybody might have to help me get this resolved. Implementing `canImport` on top of this patch is incredibly simple, so I’m hoping we can get the hard part out of the way to start staging things in sooner rather than later.
Thanks,
~Robert Widmann