Repository: bytecodealliance/wasm-tools
Pull request #2076 (2025-02-26 15:24:04)

fix mismatch in stability attributes error in wit-parser
fixes: #1995

When a new interface is marked as `@unstable (feature = somefeaturegate)` and it uses a stable type from another package via `use package:interface/type.{name}` the resulting package should be able to be imported into other packages that use it. The use case for this is adding a unstable interface such as `wasi-tls` to `wasi-cli`.  Once we get to `resolve_include`'s processing we've already verified that everything checks out from a feature/version perspective for those features. i.e. we can't be trying to consolidate an interface type with a feature that hasn't already been resolved.

alexcrichton: Hm ok the CI failure is kind of worrisome since I think this looks like we're getting nondeterministic output? I forget what affects the order things are fed in to the resolve (maybe filesystem ordering?) but it may mean that we still want some sort of "unifying" here but with a defined ordering rather than that `update_stability` method
jsturtevant: I've been looking into it and found that it is because I am not preserving the versioning that was previously in `update_stability`, specifically:

```
// Otherwise if `into` is unknown then inherit the stability listed in
    // `from`.
    if into.is_unknown() {
        *into = from.clone();
        return Ok(());
    }
```

but I found that there might be a bug prior to this due to ordering in https://github.com/jsturtevant/wasm-tools/tree/fix-mismatch-attributes-on-include/crates/wit-component/tests/merge/success/merge.

For instance the final merge preserves the `@since` for the `into` but not the `from`:

```
world shared-world-with-versions-and-include {
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}
```

And it does the opposite with the `since` when I merge `into` onto `from` (so ordering matters here):

```
world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  import shared-version-on-into;
}
```

Where I would expect to see:

```
world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}
```

My thought is replace the `update_stability` with something that copies those appropriately, but might need to sort through some changes with `usntable` attribute.
alexcrichton: > My thought is replace the update_stability with something that copies those appropriately, but might need to sort through some changes with unstable attribute which isn't covered by that set of tests currently.

That sounds reasonable to me, sort of a "define an `Ord` for stability and preserve the max" strategy of sorts
jsturtevant: > For instance the final merge preserves the `@since` for the `into` but not the `from`:

This ended up being a separate issue.  I'd be happy to split this out into a separate change and do a little refactoring to cleanup the change which I sorta shove into the world merge section. 

> That sounds reasonable to me, sort of a "define an Ord for stability and preserve the max" strategy of sorts

I added `derive ord` to the enum, the order ends up being the order in which it is in the enum so `stable` is less than `unstable` which I find a little awkward.  I am not sure if it's OK that way or if we should reorder the enum (would that be a breaking change?)

I'll clean up some of the comments tomorrow too
alexcrichton: > This ended up being a separate issue. I'd be happy to split this out into a separate change and do a little refactoring to cleanup the change which I sorta shove into the world merge section.

Ah ok good catch! I'm happy to leave that in here, too, but if you'd be up for refactoring to deduplicate a bit I think that'd be good 👍 

>  I am not sure if it's OK that way or if we should reorder the enum (would that be a breaking change?)

Definitely ok to reorder an enum, so how about a swap of `Unknown` and `Stable` so that way `>` should work? Also, mind leaving a comment on the enum that the `derive(Ord)` ordering is significant?

Patch:
diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs
index ed8580cb..97673d4e 100644
--- a/crates/wit-parser/src/lib.rs
+++ b/crates/wit-parser/src/lib.rs
@@ -1176,18 +1176,21 @@ fn find_futures_and_streams(resolve: &Resolve, ty: Type, results: &mut Vec<TypeI
 ///
 /// This is added for WebAssembly/component-model#332 where @since and @unstable
 /// annotations were added to WIT.
-#[derive(Debug, Clone, PartialEq, Eq)]
+///
+/// The order of the of enum values is significant since it is used with Ord and PartialOrd
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 #[cfg_attr(feature = "serde", derive(serde_derive::Deserialize, Serialize))]
 #[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))]
 pub enum Stability {
-    /// `@since(version = 1.2.3)`
+    /// This item does not have either `@since` or `@unstable`.
+    Unknown,
+
+    /// `@unstable(feature = foo)`
     ///
-    /// This item is explicitly tagged with `@since` as stable since the
-    /// specified version.  This may optionally have a feature listed as well.
-    Stable {
-        #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_version"))]
-        #[cfg_attr(feature = "serde", serde(deserialize_with = "deserialize_version"))]
-        since: Version,
+    /// This item is explicitly tagged `@unstable`. A feature name is listed and
+    /// this item is excluded by default in `Resolve` unless explicitly enabled.
+    Unstable {
+        feature: String,
         #[cfg_attr(
             feature = "serde",
             serde(
@@ -1200,12 +1203,14 @@ pub enum Stability {
         deprecated: Option<Version>,
     },
 
-    /// `@unstable(feature = foo)`
+    /// `@since(version = 1.2.3)`
     ///
-    /// This item is explicitly tagged `@unstable`. A feature name is listed and
-    /// this item is excluded by default in `Resolve` unless explicitly enabled.
-    Unstable {
-        feature: String,
+    /// This item is explicitly tagged with `@since` as stable since the
+    /// specified version.  This may optionally have a feature listed as well.
+    Stable {
+        #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_version"))]
+        #[cfg_attr(feature = "serde", serde(deserialize_with = "deserialize_version"))]
+        since: Version,
         #[cfg_attr(
             feature = "serde",
             serde(
@@ -1217,9 +1222,6 @@ pub enum Stability {
         )]
         deprecated: Option<Version>,
     },
-
-    /// This item does not have either `@since` or `@unstable`.
-    Unknown,
 }
 
 impl Stability {
diff --git a/crates/wit-parser/src/resolve.rs b/crates/wit-parser/src/resolve.rs
index 2c461529..f579d67e 100644
--- a/crates/wit-parser/src/resolve.rs
+++ b/crates/wit-parser/src/resolve.rs
@@ -3411,7 +3411,7 @@ impl Remap {
                         },
                     ) => {
                         assert_eq!(*aid, *bid);
-                        update_stability(astability, bstability)?;
+                        merge_stability(astability, bstability)?;
                     }
                     (WorldItem::Interface { .. }, _) => unreachable!(),
                     (WorldItem::Function(_), _) => unreachable!(),
@@ -3837,7 +3837,28 @@ fn update_stability(from: &Stability, into: &mut Stability) -> Result<()> {
 
     // Failing all that this means that the two attributes are different so
     // generate an error.
-    bail!("mismatch in stability attributes")
+    bail!("mismatch in stability from '{:?}' to '{:?}'", from, into)
+}
+
+/// Compares the two attributes and if the `from` is more stable than the `into` then
+/// it will elevate the `into` to the same stability.  
+/// This should be used after its already been confirmed that the types are the same and
+/// should be apart of the component because versions/features are enabled.
+fn merge_stability(from: &Stability, into: &mut Stability) -> Result<()> {
+    // If the two stability annotations are equal then
+    // there's nothing to do here.
+    if from == into {
+        return Ok(());
+    }
+
+    // if the from is more stable elevate stability of into
+    if from > into {
+        *into = from.clone();
+        return Ok(());
+    }
+
+    // otherwise `into`` already has higher stability
+    return Ok(());
 }
 
 /// An error that can be returned during "world elaboration" during various


Test patch:
diff --git a/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit b/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit
new file mode 100644
index 00000000..159ffb39
--- /dev/null
+++ b/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit
@@ -0,0 +1,75 @@
+package wasmtime:test;
+
+world test{
+    @unstable(feature = active)
+    include wasi:unstable/imports@0.2.3; 
+    include wasi:foo/imports@0.2.3;
+} 
+
+world test-ordered{
+    include wasi:foo/imports@0.2.3;
+    @unstable(feature = active)
+    include wasi:unstable/imports@0.2.3; 
+} 
+
+package wasi:unstable@0.2.3 {
+    @unstable(feature = active)
+    world imports {
+        @unstable(feature = active)
+        use wasi:dep2/stable@0.2.3.{stable-resource};
+        @unstable(feature = active)
+        use wasi:dep-unversioned/unversioned.{unversioned-resource};
+        @unstable(feature = active)
+        use wasi:dep-unstable/unstable.{unstable-resource};
+    }
+}
+
+package wasi:foo@0.2.3 {
+    @since(version = 0.2.0)
+    world imports {
+        @since(version = 0.2.0)
+        include wasi:dep2/imports@0.2.3;
+        include wasi:dep-unversioned/imports;
+        include wasi:dep-unstable/imports;
+    }
+}
+
+package wasi:dep2@0.2.3 {
+    @since(version = 0.2.0)
+    world imports {
+        @since(version = 0.2.0)
+        import stable;
+    }
+    @since(version = 0.2.1)
+    interface stable {
+        resource stable-resource {
+        }
+    }
+}
+
+package wasi:dep-unversioned{
+    world imports {
+        import unversioned;
+    }
+    interface unversioned {
+        resource unversioned-resource {
+
+        }
+    }
+}
+
+package wasi:dep-unstable{
+    @unstable(feature = active)
+    world imports {
+        @unstable(feature = active)
+        import unstable;
+    }
+    @unstable(feature = active)
+    interface unstable {
+        @unstable(feature = active)
+        resource unstable-resource {
+
+        }
+    }
+}
+
diff --git a/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit.json b/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit.json
new file mode 100644
index 00000000..471d6b4c
--- /dev/null
+++ b/crates/wit-parser/tests/ui/gated-include-use-with-stable.wit.json
@@ -0,0 +1,397 @@
+{
+  "worlds": [
+    {
+      "name": "imports",
+      "imports": {
+        "interface-0": {
+          "interface": {
+            "id": 0,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        }
+      },
+      "exports": {},
+      "package": 0,
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    },
+    {
+      "name": "imports",
+      "imports": {
+        "interface-1": {
+          "interface": {
+            "id": 1
+          }
+        }
+      },
+      "exports": {},
+      "package": 1
+    },
+    {
+      "name": "imports",
+      "imports": {
+        "interface-2": {
+          "interface": {
+            "id": 2,
+            "stability": {
+              "stable": {
+                "since": "0.2.0"
+              }
+            }
+          }
+        }
+      },
+      "exports": {},
+      "package": 2,
+      "stability": {
+        "stable": {
+          "since": "0.2.0"
+        }
+      }
+    },
+    {
+      "name": "imports",
+      "imports": {
+        "interface-2": {
+          "interface": {
+            "id": 2,
+            "stability": {
+              "stable": {
+                "since": "0.2.0"
+              }
+            }
+          }
+        },
+        "interface-1": {
+          "interface": {
+            "id": 1
+          }
+        },
+        "interface-0": {
+          "interface": {
+            "id": 0,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        }
+      },
+      "exports": {},
+      "package": 3,
+      "stability": {
+        "stable": {
+          "since": "0.2.0"
+        }
+      }
+    },
+    {
+      "name": "imports",
+      "imports": {
+        "interface-2": {
+          "interface": {
+            "id": 2,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "stable-resource": {
+          "type": 3
+        },
+        "interface-1": {
+          "interface": {
+            "id": 1,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "unversioned-resource": {
+          "type": 4
+        },
+        "interface-0": {
+          "interface": {
+            "id": 0,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "unstable-resource": {
+          "type": 5
+        }
+      },
+      "exports": {},
+      "package": 4,
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    },
+    {
+      "name": "test",
+      "imports": {
+        "interface-2": {
+          "interface": {
+            "id": 2,
+            "stability": {
+              "stable": {
+                "since": "0.2.0"
+              }
+            }
+          }
+        },
+        "stable-resource": {
+          "type": 3
+        },
+        "interface-1": {
+          "interface": {
+            "id": 1,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "unversioned-resource": {
+          "type": 4
+        },
+        "interface-0": {
+          "interface": {
+            "id": 0,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "unstable-resource": {
+          "type": 5
+        }
+      },
+      "exports": {},
+      "package": 5
+    },
+    {
+      "name": "test-ordered",
+      "imports": {
+        "interface-2": {
+          "interface": {
+            "id": 2,
+            "stability": {
+              "stable": {
+                "since": "0.2.0"
+              }
+            }
+          }
+        },
+        "interface-1": {
+          "interface": {
+            "id": 1,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "interface-0": {
+          "interface": {
+            "id": 0,
+            "stability": {
+              "unstable": {
+                "feature": "active"
+              }
+            }
+          }
+        },
+        "stable-resource": {
+          "type": 3
+        },
+        "unversioned-resource": {
+          "type": 4
+        },
+        "unstable-resource": {
+          "type": 5
+        }
+      },
+      "exports": {},
+      "package": 5
+    }
+  ],
+  "interfaces": [
+    {
+      "name": "unstable",
+      "types": {
+        "unstable-resource": 0
+      },
+      "functions": {},
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      },
+      "package": 0
+    },
+    {
+      "name": "unversioned",
+      "types": {
+        "unversioned-resource": 1
+      },
+      "functions": {},
+      "package": 1
+    },
+    {
+      "name": "stable",
+      "types": {
+        "stable-resource": 2
+      },
+      "functions": {},
+      "stability": {
+        "stable": {
+          "since": "0.2.1"
+        }
+      },
+      "package": 2
+    }
+  ],
+  "types": [
+    {
+      "name": "unstable-resource",
+      "kind": "resource",
+      "owner": {
+        "interface": 0
+      },
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    },
+    {
+      "name": "unversioned-resource",
+      "kind": "resource",
+      "owner": {
+        "interface": 1
+      }
+    },
+    {
+      "name": "stable-resource",
+      "kind": "resource",
+      "owner": {
+        "interface": 2
+      }
+    },
+    {
+      "name": "stable-resource",
+      "kind": {
+        "type": 2
+      },
+      "owner": {
+        "world": 4
+      },
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    },
+    {
+      "name": "unversioned-resource",
+      "kind": {
+        "type": 1
+      },
+      "owner": {
+        "world": 4
+      },
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    },
+    {
+      "name": "unstable-resource",
+      "kind": {
+        "type": 0
+      },
+      "owner": {
+        "world": 4
+      },
+      "stability": {
+        "unstable": {
+          "feature": "active"
+        }
+      }
+    }
+  ],
+  "packages": [
+    {
+      "name": "wasi:dep-unstable",
+      "interfaces": {
+        "unstable": 0
+      },
+      "worlds": {
+        "imports": 0
+      }
+    },
+    {
+      "name": "wasi:dep-unversioned",
+      "interfaces": {
+        "unversioned": 1
+      },
+      "worlds": {
+        "imports": 1
+      }
+    },
+    {
+      "name": "wasi:dep2@0.2.3",
+      "interfaces": {
+        "stable": 2
+      },
+      "worlds": {
+        "imports": 2
+      }
+    },
+    {
+      "name": "wasi:foo@0.2.3",
+      "interfaces": {},
+      "worlds": {
+        "imports": 3
+      }
+    },
+    {
+      "name": "wasi:unstable@0.2.3",
+      "interfaces": {},
+      "worlds": {
+        "imports": 4
+      }
+    },
+    {
+      "name": "wasmtime:test",
+      "interfaces": {},
+      "worlds": {
+        "test": 5,
+        "test-ordered": 6
+      }
+    }
+  ]
+}
\ No newline at end of file