Admit ternary property-read operands (#3023)

Author: rogeralsing

docs/unified-bytecode-expansion-contract.md

@@ -378,7 +378,7 @@ the final post-compile production subset check before VM entry.
 |---|---|---|---|---|
 | `prototype-guard:Binary` | `Binary` opcodes currently admit the selector-owned production subset: arithmetic (`+`, `-`, `*`, `/`, `%`, `**`), equality/comparison (`==`, `!=`, `===`, `!==`, `<`, `<=`, `>`, `>=`), bitwise/shift (`&`, `|`, `^`, `<<`, `>>`, `>>>`), and relational/object tests (`in`, `instanceof`); binary operators outside that subset (for example `&&`, `\|\|`, `??`) decline as `UnsupportedPlanShape` | Existing expression route | Binary operator widening lane | `rtk dotnet test tests/Asynkron.JsEngine.Tests --filter "FullyQualifiedName~UnifiedBytecodeProductionEligibilityTests&FullyQualifiedName~Evaluate_PropertyReadAdjacentFamilies_DeclineWithExplicitCodes"` |
 | `prototype-guard:Jump` | `Jump` and `JumpWithDriverCleanup` are admitted only for compiler-owned branch, loop, and driver cleanup shapes | Existing control-flow route if an unowned jump shape is introduced | Control-flow lane | `rtk dotnet test tests/Asynkron.JsEngine.Tests --filter "FullyQualifiedName~UnifiedBytecodeProductionEligibilityTests&FullyQualifiedName~Evaluate_LabeledBreakCrossingDriverLoop_DeclinesWithLabelControlFlow"` |
-| `prototype-guard:JumpIfFalse` | `JumpIfFalse` and short-circuit conditional jump opcodes are admitted only for proven compiler-owned expression and statement control flow | Existing control-flow route if an unowned conditional shape is introduced | Control-flow lane | `rtk dotnet test tests/Asynkron.JsEngine.Tests --filter "FullyQualifiedName~UnifiedBytecodeProductionEligibilityTests&FullyQualifiedName~Evaluate_ConditionalExpression_ThisPropertyConditionAndArms_DeclinesWithPropertyReadBoundaryOutOfScope"` |
+| `prototype-guard:JumpIfFalse` | `JumpIfFalse` and short-circuit conditional jump opcodes are admitted only for proven compiler-owned expression and statement control flow | Existing control-flow route if an unowned conditional shape is introduced | Control-flow lane | `rtk dotnet test tests/Asynkron.JsEngine.Tests --filter "FullyQualifiedName~UnifiedBytecodeProductionEligibilityTests&FullyQualifiedName~Evaluate_ConditionalExpression_ThisPropertyConditionAndArms_Accepts"` |
 | `prototype-guard:DefaultUnsupportedOpcode` | Any future `UnifiedBytecodeOpCode` not explicitly listed in the switch declines as `UnsupportedPlanShape` until the selector, compiler, VM, and proof pack are updated together | Existing execution-plan route | Opcode ownership lane | `rtk dotnet test tests/Asynkron.JsEngine.Tests --filter "FullyQualifiedName~ExpressionProgramCoverageMapTests&FullyQualifiedName~UnifiedBytecodeExpansionContract_ListsRequiredHeadingsAndCurrentEnums"` |
 
 ### Statement Diagnostics Supported Kinds (current)

src/Asynkron.JsEngine/Execution/UnifiedBytecode/UnifiedBytecodeProductionEligibility.cs

@@ -1063,6 +1063,15 @@ private static bool TryFindExpressionDecline(
                         break;
                     }
 
+                    if (TryIsConditionalExpressionActivationResolvedNamedPropertyReadOperand(
+                            program,
+                            operationIndex,
+                            identifierConstants,
+                            activationSlots))
+                    {
+                        break;
+                    }
+
                     if (TryIsFirstBoundaryOptionalNamedChainCandidate(program, identifierConstants, activationSlots))
                     {
                         break;
@@ -1893,6 +1902,53 @@ private static bool TryIsFirstBoundaryBinaryNamedPropertyReadCandidate(
         return true;
     }
 
+    private static bool TryIsConditionalExpressionActivationResolvedNamedPropertyReadOperand(
+        ExpressionProgram program,
+        int operationIndex,
+        ReadOnlySpan<IdentifierOperand> identifierConstants,
+        ActivationSlotShape activationSlots)
+    {
+        if (FindFirstOperation(program, ExpressionOpKind.JumpIfConditionalFalse) < 0)
+        {
+            return false;
+        }
+
+        var stringConstants = program.StringConstants.AsSpan();
+        var operation = program.GetOperation(operationIndex);
+        if (operation.Kind != ExpressionOpKind.GetNamedProperty ||
+            operation.IsOptional ||
+            operation.ShortCircuitOnNullishTarget ||
+            operation.GetString(stringConstants).IsPrivateName())
+        {
+            return false;
+        }
+
+        var chainStartIndex = operationIndex;
+        while (chainStartIndex > 0)
+        {
+            var previous = program.GetOperation(chainStartIndex - 1);
+            if (previous.Kind != ExpressionOpKind.GetNamedProperty)
+            {
+                break;
+            }
+
+            if (previous.IsOptional ||
+                previous.ShortCircuitOnNullishTarget ||
+                previous.GetString(stringConstants).IsPrivateName())
+            {
+                return false;
+            }
+
+            chainStartIndex--;
+        }
+
+        return chainStartIndex > 0 &&
+               TryGetActivationResolvedValue(
+                   program.GetOperation(chainStartIndex - 1),
+                   identifierConstants,
+                   activationSlots);
+    }
+
     /// <summary>
     /// Returns <c>true</c> when the <see cref="ExpressionOpKind.GetNamedProperty"/> operation at
     /// <paramref name="operationIndex"/> is the last op before a logical short-circuit jump

tests/Asynkron.JsEngine.Tests/UnifiedBytecodeProductionEligibilityTests.cs

@@ -6289,17 +6289,8 @@ function classify(c1, c2, a, b, d) {
     }
 
     [Fact]
-    public void Evaluate_ConditionalExpression_ThisPropertyConditionAndArms_DeclinesWithPropertyReadBoundaryOutOfScope()
-    {
-        // AC-5 deviation (intentional): AC-5 originally required an accept test for this.flag ? a : b.
-        // TryIsFirstBoundaryPropertyReadShortCircuitExpressionCandidate handles only logical short-circuit
-        // opcodes (JumpIfFalse/JumpIfTrue/JumpIfNotNullish) — not JumpIfConditionalFalse, which is the
-        // ternary condition opcode. The ternary condition is not a "short-circuit property read" candidate;
-        // the property value is consumed as a boolean condition, not returned directly. Extending the
-        // method to admit JumpIfConditionalFalse would be architecturally incorrect. This decline test
-        // documents the boundary as the intentional AC-5 resolution.
-        // this.flag ? this.a : other has GetNamedProperty in ternary condition position,
-        // which is outside the admitted property-read boundary shapes for ternary.
+    public void Evaluate_ConditionalExpression_ThisPropertyConditionAndArms_Accepts()
+    {
         var plan = GetClassMethodPlan("""
             class Toggle {
                 select(other) {
@@ -6314,8 +6305,12 @@ class Toggle {
             plan,
             new UnifiedBytecodeProductionActivationDescriptor());
 
-        Assert.False(result.IsEligible);
-        Assert.Equal(UnifiedBytecodeProductionDeclineCode.PropertyReadBoundaryOutOfScope, result.Code);
+        Assert.True(result.IsEligible, result.Reason);
+        Assert.Equal(UnifiedBytecodeProductionDeclineCode.None, result.Code);
+        Assert.Contains(result.Program.Instructions, instruction =>
+            instruction.OpCode == UnifiedBytecodeOpCode.JumpIfFalse);
+        Assert.Contains(result.Program.Instructions, instruction =>
+            instruction.OpCode == UnifiedBytecodeOpCode.GetNamedProperty);
     }
 
     [Fact]

tests/Asynkron.JsEngine.Tests/UnifiedBytecodeProductionInvocationTests.cs

@@ -7838,6 +7838,32 @@ static record => record.Message.Contains(
                 StringComparison.Ordinal));
     }
 
+    [Fact(Timeout = 5000)]
+    public async Task Ternary_ThisPropertyConditionAndArm_UsesProductionFastPath()
+    {
+        await using var engine = CreateEngine();
+        var result = await engine.Evaluate("""
+            class Toggle {
+                constructor(flag, value) {
+                    this.flag = flag;
+                    this.value = value;
+                }
+
+                select(other) {
+                    return this.flag ? this.value : other;
+                }
+            }
+
+            [new Toggle(true, 42).select(10), new Toggle(false, 42).select(10)].join(",");
+            """);
+
+        Assert.Equal("42,10", result?.ToString());
+        Assert.Contains(CurrentLogger!.Collector.Snapshot(),
+            static record => record.Message.Contains(
+                "unified-bytecode-production-fast-path func=<anonymous>",
+                StringComparison.Ordinal));
+    }
+
     [Fact(Timeout = 5000)]
     public async Task Ternary_ConditionIsConsumedNotPeeked_BothBranchesCorrect()
     {