Remove dead LoadVariable method from AsyncArrowMoveNextEmitter#706
Merged
Conversation
`LoadVariable(string)` in AsyncArrowMoveNextEmitter.Variables.cs had no callers. The only live variable-load paths in this emitter are the `EmitVariable` override and `LoadVariableForCapture` (used by ArrowFunctions.cs); `LoadVariable` was a near-duplicate of `EmitVariable` left behind. It was also a latent copy of the bug fixed in #648 / PR #671: like the pre-fix `EmitVariable`, its `Ldnull` fallback never consulted the JS global constants (NaN/Infinity/undefined), so had it ever been wired up a bare `NaN`/`Infinity` would again compile to a null load. Delete the dead method and update the two comments that referenced it by name so they point at the live `EmitVariable` override / capture-population loader instead. No behavioral change. Build clean; AsyncNaNStrictEqualityTests 20/20 green.
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
1 participant
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.
What
Deletes the unused
LoadVariable(string)method fromCompilation/AsyncArrowMoveNextEmitter.Variables.csand fixes the twocomments that referenced it by name.
Why
A repo-wide search confirms
LoadVariablehad no callers. The only livevariable-load paths in this emitter are:
EmitVariableoverride (general expression loads), andLoadVariableForCapture(capture population, called fromAsyncArrowMoveNextEmitter.ArrowFunctions.cs).LoadVariablewas a leftover near-duplicate ofEmitVariable. It was also alatent copy of the bug fixed in #648 / PR #671: like the pre-fix
EmitVariable, itsLdnullfallback never consulted the JS global constants(
NaN/Infinity/undefined). Had it ever been wired up, a bareNaN/Infinitywould again have compiled to a null load (e.g.NaN === NaNdegrading to
null === null→true). Removing it eliminates that trap ratherthan leaving a second code path that would need the same fix.
Changes
LoadVariablemethod (~82 lines).live
EmitVariableoverride / capture-population loader instead.Reviewer notes
dead-code removal. Diff is net
+2 / -85, confined to two files.EmitVariableoverride on currentmainalready carries theCompiled: NaN === NaN returns true inside an async arrow function (state-machine NaN gap) #648/Fix #648: NaN/Infinity globals compiled to null inside a compiled async arrow #671 fix (
TryEmitJsGlobalConstant), so no functional gap is introduced.dotnet build SharpTS.csproj --no-incrementalclean (0 warnings,0 errors);
dotnet test --filter FullyQualifiedName~AsyncNaNStrictEqualityTests→ 20/20 passing.