refactor(compile): single-source ILEmitter/state-machine Expr.Get static dispatch (drift fix)#1273
Merged
Merged
Conversation
…shared by ILEmitter and the state-machine base ILEmitter.EmitCall re-inlined the module.promises and Class.staticMethod/Promise-subclass-static dispatch that TryEmitGetCalleeViaBaseClass also implements, and the copies were actively drifting: the base gained the #439 await-safe argument spill the inline copy lacked, and the inline copy boxed extra args via EmitBoxIfNeeded while the base used EnsureBoxed. An earlier drift of this exact block shipped a bug (MyP.resolve(1) threw). Extract TryEmitModulePromisesMethodCall and TryEmitClassStaticDispatch into ExpressionEmitterBase.CallHelpers and route both callers through them. The boxing divergence becomes an explicit named seam: EnsureBoxedArg defaults to stack-state EnsureBoxed (state-machine behavior, unchanged) and ILEmitter overrides it with its TypeMap/ literal-aware EmitBoxIfNeeded (unchanged) — both paths keep their exact prior IL. The suspension-spill branch is unreachable from ILEmitter (sync bodies contain no await/yield). Adds StaticDispatchBoxingTests pinning boxing-sensitive static dispatch from sync and async call sites plus the Promise-subclass statics from async code, in both execution modes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 8a: closes the highest-risk item left from the July audits — the actively-diverging duplicate of the Expr.Get static-dispatch blocks.
Problem
ILEmitter.EmitCallre-inlined themodule.promises.method()andClass.staticMethod()/ inherited-Promise-static dispatch thatTryEmitGetCalleeViaBaseClassalso implements. The copies had already drifted twice:MyP.resolve(1)crash (the code carries a comment admitting this);EmitBoxIfNeededvsEnsureBoxed).Change
TryEmitModulePromisesMethodCall+TryEmitClassStaticDispatchintoExpressionEmitterBase.CallHelpers; bothTryEmitGetCalleeViaBaseClassandILEmitter.EmitCallnow route through them.protected virtual EnsureBoxedArg(Expr)defaults to stack-stateEnsureBoxed()(state-machine behavior unchanged); ILEmitter overrides it with its richer TypeMap/literal-awareEmitBoxIfNeeded(its behavior unchanged). Both paths emit their exact prior IL — this is a dedup, not a behavior change.Tests
StaticDispatchBoxingTests(3 theories × interpreter/compiled): boxing-sensitive static args from a sync top-level call site (ILEmitter path), from inside an async function (state-machine path), and Promise-subclass statics from async code.PromiseSubclassTests.ExtendsPromise_TopLevelStaticThenalready pins the historicalMyP.resolve(1)regression.