Guard conditional switch discriminants in optimized Ok()

compiler/back_end/cpp/generated_code_templates

@@ -411,6 +411,26 @@ ${switch_cases}
     }
 
 
+// ** ok_method_switch_block_guarded ** ////////////////////////////////////////
+    // Check fields that share a conditionally-present discriminant.  Every arm
+    // carries a residual, so when the discriminant is out of bounds the message
+    // is still valid for any arm whose residual is statically false.  Dispatch
+    // only when the discriminant can be read; otherwise fall back to per-field
+    // existence checks.  (An Unknown discriminant can never make has_X()
+    // Known-true, so only the Known() test is needed in the fallback.)
+    {
+      const auto emboss_reserved_switch_discrim = ${discriminant};
+      if (emboss_reserved_switch_discrim.Known()) {
+        switch (emboss_reserved_switch_discrim.ValueOrDefault()) {
+${switch_cases}
+
+          default: break;
+        }
+      } else {
+${fallback_checks}      }
+    }
+
+
 // ** ok_method_switch_arm ** //////////////////////////////////////////////////
 ${case_labels}${case_body}          break;
 

compiler/back_end/cpp/header_generator.py

@@ -1801,18 +1801,56 @@ def _emit_switch_block(group):
             )
         )
 
-    if group.get("known_check_required", True):
-        known_check = (
-            "      if (!emboss_reserved_switch_discrim.Known()) return false;\n"
+    if not group.get("known_check_required", True):
+        # Provably-present discriminant: the per-field validation loop has
+        # already proven the discriminant readable, so dispatch is always safe.
+        return code_template.format_template(
+            _TEMPLATES.ok_method_switch_block,
+            discriminant=group["discrim_rendered"],
+            known_check="",
+            switch_cases="".join(rendered_arms),
         )
-    else:
-        known_check = ""
 
+    # A "bare" arm is one whose existence condition is exactly `discrim == K`,
+    # with no residual conjuncts.  When the group has at least one bare arm, an
+    # Unknown discriminant leaves that field's has_X() Unknown, so the simple
+    # `if (!discrim.Known()) return false;` guard faithfully reproduces the
+    # unoptimized Ok() (which bails for the same reason).  This covers ordinary
+    # tagged unions and always-present-but-virtual discriminants (e.g. a `let`
+    # alias of a parameter).
+    has_bare_arm = any(
+        not has_residual
+        for case_entry in group["cases_by_label"].values()
+        for (_field, has_residual) in case_entry["entries"]
+    )
+    if has_bare_arm:
+        return code_template.format_template(
+            _TEMPLATES.ok_method_switch_block,
+            discriminant=group["discrim_rendered"],
+            known_check=(
+                "      if (!emboss_reserved_switch_discrim.Known()) return false;\n"
+            ),
+            switch_cases="".join(rendered_arms),
+        )
+
+    # Every arm carries a residual and the discriminant is not provably present.
+    # The `Known()` guard would be unsound here: a valid message can have an
+    # out-of-bounds (Unknown) discriminant while every arm's residual is
+    # statically false, making every has_X() Known-false (absent).  Dispatch
+    # only when the discriminant is in bounds; otherwise fall back to per-field
+    # existence checks.  (With the discriminant Unknown, has_X() can never be
+    # Known-true, so the fallback needs only the Known() test.)
+    fallback_checks = "".join(
+        "        if (!has_{}().Known()) return false;\n".format(
+            _cpp_field_name(field.name.name.text)
+        )
+        for field in group["encounter_order"]
+    )
     return code_template.format_template(
-        _TEMPLATES.ok_method_switch_block,
+        _TEMPLATES.ok_method_switch_block_guarded,
         discriminant=group["discrim_rendered"],
-        known_check=known_check,
         switch_cases="".join(rendered_arms),
+        fallback_checks=fallback_checks,
     )
 
 

compiler/back_end/cpp/testcode/condition_test.cc

@@ -300,6 +300,217 @@ TEST(Conditional, StructWithNestedConditionIsOkWhenOuterConditionExists) {
   EXPECT_EQ(2U, writer.SizeInBytes());
 }
 
+// Regression tests for the optimized Ok() switch when the discriminant (`tag`)
+// is itself a conditionally-present field.  In ResidualConditionalDiscriminant
+// every arm carries a residual (`outer == 1`), so when `outer != 1` the message
+// is valid even though `tag` is absent -- and the optimized Ok() must not
+// reject it just because the (out-of-bounds) discriminant reads as Unknown.
+TEST(Conditional,
+     ResidualConditionalDiscriminantOkWhenDiscriminantFieldIsAbsent) {
+  // outer == 0, so `tag` is absent and `a`/`b` cannot exist.  The 1-byte
+  // buffer is too short to read `tag`, but the message is still valid.
+  ::std::uint8_t buffer[1] = {0};
+  auto writer = ResidualConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_tag().Known());
+  EXPECT_FALSE(writer.has_tag().Value());
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_FALSE(writer.has_a().Value());
+  EXPECT_TRUE(writer.has_b().Known());
+  EXPECT_FALSE(writer.has_b().Value());
+}
+
+TEST(Conditional,
+     ResidualConditionalDiscriminantNotOkWhenDiscriminantPresentButTruncated) {
+  // outer == 1, so `tag` is present, but the buffer is too short to read it:
+  // whether `a`/`b` exist is indeterminate, so the message is invalid.
+  ::std::uint8_t buffer[1] = {1};
+  auto writer = ResidualConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_FALSE(writer.Ok());
+  EXPECT_TRUE(writer.has_tag().Known());
+  EXPECT_TRUE(writer.has_tag().Value());
+  EXPECT_FALSE(writer.has_a().Known());
+}
+
+TEST(Conditional, ResidualConditionalDiscriminantOkWhenGatedFieldIsPresent) {
+  // outer == 1, tag == 0, so `a` exists and is in bounds; `b` is absent.
+  ::std::uint8_t buffer[3] = {1, 0, 7};
+  auto writer = ResidualConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_TRUE(writer.has_b().Known());
+  EXPECT_FALSE(writer.has_b().Value());
+  EXPECT_EQ(7, writer.a().Read());
+}
+
+TEST(Conditional, ResidualConditionalDiscriminantNotOkWhenGatedFieldTruncated) {
+  // outer == 1, tag == 0, so `a` exists, but the buffer is too short for it.
+  ::std::uint8_t buffer[2] = {1, 0};
+  auto writer = ResidualConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_FALSE(writer.a().Ok());
+  EXPECT_FALSE(writer.Ok());
+}
+
+TEST(Conditional, ResidualConditionalDiscriminantOkWhenDiscriminantMatchesNoArm) {
+  // outer == 1, tag == 5: neither `a` nor `b` exists, so the message is valid.
+  ::std::uint8_t buffer[2] = {1, 5};
+  auto writer = ResidualConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_FALSE(writer.has_a().Value());
+  EXPECT_FALSE(writer.has_b().Value());
+}
+
+// BareConditionalDiscriminant keeps the simple discriminant Known() guard:
+// the arms are bare (`if tag == K:`), so an Unknown `tag` leaves has_a()/
+// has_b() Unknown and Ok() must report the message as invalid.
+TEST(Conditional, BareConditionalDiscriminantNotOkWhenDiscriminantTruncated) {
+  // 1-byte buffer: `tag` (offset 1) is out of bounds, so has_a()/has_b() are
+  // Unknown and the message's validity cannot be determined.
+  ::std::uint8_t buffer[1] = {0};
+  auto writer = BareConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_FALSE(writer.Ok());
+  EXPECT_FALSE(writer.has_a().Known());
+  EXPECT_FALSE(writer.has_b().Known());
+}
+
+TEST(Conditional, BareConditionalDiscriminantOkWhenGatedFieldIsPresent) {
+  // outer == 1 (so `tag` is present) and tag == 0, so `a` exists and is in
+  // bounds.
+  ::std::uint8_t buffer[3] = {1, 0, 7};
+  auto writer = BareConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_EQ(7, writer.a().Read());
+}
+
+TEST(Conditional, BareConditionalDiscriminantNotOkWhenGatedFieldTruncated) {
+  // outer == 1, tag == 0, so `a` exists, but the buffer is too short for it.
+  ::std::uint8_t buffer[2] = {1, 0};
+  auto writer = BareConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_FALSE(writer.Ok());
+}
+
+// Distinguishing test: a bare arm on a conditional discriminant whose field is
+// size-dominated by an always-present field.  IsComplete() stays true, so only
+// the discriminant Known() guard can reject the message.  This is the case
+// where a `if (has_tag().ValueOrDefault())` wrapper diverges (it would accept).
+TEST(Conditional, DominatedBareDiscriminantNotOkWhenDiscriminantAbsent) {
+  ::std::uint8_t buffer[4] = {0, 0, 0, 9};
+  auto writer = DominatedBareDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.IsComplete());
+  EXPECT_FALSE(writer.has_a().Known());
+  EXPECT_FALSE(writer.has_b().Known());
+  EXPECT_FALSE(writer.Ok());
+}
+
+TEST(Conditional, DominatedBareDiscriminantOkWhenDiscriminantPresent) {
+  // outer == 1, tag == 0: `a` present and readable; `tail` present.
+  ::std::uint8_t buffer[4] = {1, 0, 5, 9};
+  auto writer = DominatedBareDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_EQ(5, writer.a().Read());
+  EXPECT_EQ(9, writer.tail().Read());
+}
+
+// Disjunction arms (#256 matcher) on a conditional discriminant, all carrying a
+// residual -> the residual-only guarded switch with coalesced case labels.
+TEST(Conditional, DisjunctionConditionalDiscriminantOkWhenDiscriminantAbsent) {
+  ::std::uint8_t buffer[1] = {0};
+  auto writer = DisjunctionConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_FALSE(writer.has_a().Value());
+  EXPECT_FALSE(writer.has_b().Value());
+}
+
+TEST(Conditional, DisjunctionConditionalDiscriminantBothDisjunctsSelectField) {
+  // tag == 0 and tag == 1 both make `a` present.
+  ::std::uint8_t buffer0[3] = {1, 0, 7};
+  auto w0 = DisjunctionConditionalDiscriminantWriter(buffer0, sizeof buffer0);
+  EXPECT_TRUE(w0.Ok());
+  EXPECT_TRUE(w0.has_a().Value());
+  EXPECT_EQ(7, w0.a().Read());
+
+  ::std::uint8_t buffer1[3] = {1, 1, 8};
+  auto w1 = DisjunctionConditionalDiscriminantWriter(buffer1, sizeof buffer1);
+  EXPECT_TRUE(w1.Ok());
+  EXPECT_TRUE(w1.has_a().Value());
+  EXPECT_EQ(8, w1.a().Read());
+}
+
+TEST(Conditional, DisjunctionConditionalDiscriminantSecondArmAndNoMatch) {
+  // tag == 2 selects `b` (which overlaps `a` at offset 2); tag == 9 selects
+  // neither (still valid).
+  ::std::uint8_t buffer2[3] = {1, 2, 6};
+  auto w2 = DisjunctionConditionalDiscriminantWriter(buffer2, sizeof buffer2);
+  EXPECT_TRUE(w2.Ok());
+  EXPECT_TRUE(w2.has_b().Value());
+  EXPECT_EQ(6, w2.b().Read());
+
+  ::std::uint8_t buffer9[2] = {1, 9};
+  auto w9 = DisjunctionConditionalDiscriminantWriter(buffer9, sizeof buffer9);
+  EXPECT_TRUE(w9.Ok());
+  EXPECT_FALSE(w9.has_a().Value());
+  EXPECT_FALSE(w9.has_b().Value());
+}
+
+// Single-entry group on a conditional discriminant: demoted to a has_X() check
+// (#256), so no discriminant guard is emitted and the bug cannot arise.
+TEST(Conditional, SingleEntryConditionalDiscriminantOkWhenDiscriminantAbsent) {
+  ::std::uint8_t buffer[1] = {0};
+  auto writer = SingleEntryConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Known());
+  EXPECT_FALSE(writer.has_a().Value());
+}
+
+TEST(Conditional, SingleEntryConditionalDiscriminantOkWhenPresent) {
+  ::std::uint8_t buffer[3] = {1, 0, 4};
+  auto writer = SingleEntryConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_EQ(4, writer.a().Read());
+}
+
+TEST(Conditional,
+     SingleEntryConditionalDiscriminantNotOkWhenDiscriminantTruncated) {
+  // outer == 1 but the buffer is too short for tag: a's existence is Unknown.
+  ::std::uint8_t buffer[1] = {1};
+  auto writer = SingleEntryConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_FALSE(writer.has_a().Known());
+  EXPECT_FALSE(writer.Ok());
+}
+
+// Enum-typed conditional discriminant with residual arms (static_cast case
+// labels in the guarded switch).
+TEST(Conditional, EnumConditionalDiscriminantOkWhenDiscriminantAbsent) {
+  ::std::uint8_t buffer[1] = {0};  // outer == OFF
+  auto writer = EnumConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_FALSE(writer.has_a().Value());
+  EXPECT_FALSE(writer.has_b().Value());
+}
+
+TEST(Conditional, EnumConditionalDiscriminantOkWhenPresent) {
+  ::std::uint8_t buffer[3] = {1, 0, 7};  // outer == ON, tag == OFF -> a present
+  auto writer = EnumConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_TRUE(writer.Ok());
+  EXPECT_TRUE(writer.has_a().Value());
+  EXPECT_EQ(7, writer.a().Read());
+}
+
+TEST(Conditional, EnumConditionalDiscriminantNotOkWhenDiscriminantTruncated) {
+  ::std::uint8_t buffer[1] = {1};  // outer == ON, tag truncated
+  auto writer = EnumConditionalDiscriminantWriter(buffer, sizeof buffer);
+  EXPECT_FALSE(writer.Ok());
+}
+
 TEST(Conditional, AlwaysMissingFieldDoesNotContributeToStaticSize) {
   EXPECT_EQ(0U, OnlyAlwaysFalseConditionWriter::SizeInBytes());
   EXPECT_EQ(1U, AlwaysFalseConditionWriter::SizeInBytes());