Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix overly-aggressive pruning of types based on features #1958

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 77 additions & 10 deletions crates/wit-parser/src/ast/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,8 +1045,8 @@ impl<'a> Resolver<'a> {
) -> Result<Function> {
let docs = self.docs(docs);
let stability = self.stability(attrs)?;
let params = self.resolve_params(&func.params, &kind, func.span, &stability)?;
let results = self.resolve_results(&func.results, &kind, func.span, &stability)?;
let params = self.resolve_params(&func.params, &kind, func.span)?;
let results = self.resolve_results(&func.results, &kind, func.span)?;
Ok(Function {
docs,
stability,
Expand Down Expand Up @@ -1293,6 +1293,68 @@ impl<'a> Resolver<'a> {
}
}

/// If `stability` is `Stability::Unknown`, recursively inspect the
/// specified `kind` until we either bottom out or find a type which has a
/// stability that's _not_ unknown. If we find such a type, return a clone
/// of its stability; otherwise return `Stability::Unknown`.
///
/// The idea here is that e.g. `option<T>` should inherit `T`'s stability.
/// This gets a little ambiguous in the case of e.g. `tuple<T, U, V>`; for
/// now, we just pick the first one has a known stability, if any.
fn find_stability(&self, kind: &TypeDefKind, stability: &Stability) -> Stability {
fn find_in_type(types: &Arena<TypeDef>, ty: Type) -> Option<&Stability> {
if let Type::Id(id) = ty {
let ty = &types[id];
if !matches!(&ty.stability, Stability::Unknown) {
Some(&ty.stability)
} else {
find_in_kind(types, &ty.kind)
}
} else {
None
}
}

fn find_in_kind<'a>(
types: &'a Arena<TypeDef>,
kind: &TypeDefKind,
) -> Option<&'a Stability> {
match kind {
TypeDefKind::Type(ty) => find_in_type(types, *ty),
TypeDefKind::Handle(Handle::Borrow(id) | Handle::Own(id)) => {
find_in_type(types, Type::Id(*id))
}
TypeDefKind::Tuple(t) => t.types.iter().find_map(|ty| find_in_type(types, *ty)),
TypeDefKind::List(ty) | TypeDefKind::Stream(ty) | TypeDefKind::Option(ty) => {
find_in_type(types, *ty)
}
TypeDefKind::Future(ty) => ty.as_ref().and_then(|ty| find_in_type(types, *ty)),
TypeDefKind::Result(r) => {
r.ok.as_ref()
.and_then(|ty| find_in_type(types, *ty))
.or_else(|| r.err.as_ref().and_then(|ty| find_in_type(types, *ty)))
}
// Assume these are named types which will be annotated with an
// explicit stability if applicable:
TypeDefKind::ErrorContext
| TypeDefKind::Resource
| TypeDefKind::Variant(_)
| TypeDefKind::Record(_)
| TypeDefKind::Flags(_)
| TypeDefKind::Enum(_)
| TypeDefKind::Unknown => None,
}
}

if let Stability::Unknown = stability {
find_in_kind(&self.types, kind)
.cloned()
.unwrap_or(Stability::Unknown)
} else {
stability.clone()
}
}

fn resolve_type(&mut self, ty: &super::Type<'_>, stability: &Stability) -> Result<Type> {
// Resources must be declared at the top level to have their methods
// processed appropriately, but resources also shouldn't show up
Expand All @@ -1302,12 +1364,13 @@ impl<'a> Resolver<'a> {
_ => {}
}
let kind = self.resolve_type_def(ty, stability)?;
let stability = self.find_stability(&kind, stability);
Ok(self.anon_type_def(
TypeDef {
kind,
name: None,
docs: Docs::default(),
stability: stability.clone(),
stability,
owner: TypeOwner::None,
},
ty.span(),
Expand Down Expand Up @@ -1455,7 +1518,6 @@ impl<'a> Resolver<'a> {
params: &ParamList<'_>,
kind: &FunctionKind,
span: Span,
stability: &Stability,
) -> Result<Params> {
let mut ret = IndexMap::new();
match *kind {
Expand All @@ -1467,11 +1529,13 @@ impl<'a> Resolver<'a> {
// Methods automatically get a `self` initial argument so insert
// that here before processing the normal parameters.
FunctionKind::Method(id) => {
let kind = TypeDefKind::Handle(Handle::Borrow(id));
let stability = self.find_stability(&kind, &Stability::Unknown);
let shared = self.anon_type_def(
TypeDef {
docs: Docs::default(),
stability: stability.clone(),
kind: TypeDefKind::Handle(Handle::Borrow(id)),
stability,
kind,
name: None,
owner: TypeOwner::None,
},
Expand All @@ -1481,7 +1545,10 @@ impl<'a> Resolver<'a> {
}
}
for (name, ty) in params {
let prev = ret.insert(name.name.to_string(), self.resolve_type(ty, stability)?);
let prev = ret.insert(
name.name.to_string(),
self.resolve_type(ty, &Stability::Unknown)?,
);
if prev.is_some() {
bail!(Error::new(
name.span,
Expand All @@ -1497,7 +1564,6 @@ impl<'a> Resolver<'a> {
results: &ResultList<'_>,
kind: &FunctionKind,
span: Span,
stability: &Stability,
) -> Result<Results> {
match *kind {
// These kinds of methods don't have any adjustments to the return
Expand All @@ -1508,9 +1574,10 @@ impl<'a> Resolver<'a> {
rs,
&FunctionKind::Freestanding,
span,
stability,
)?)),
ResultList::Anon(ty) => Ok(Results::Anon(self.resolve_type(ty, stability)?)),
ResultList::Anon(ty) => {
Ok(Results::Anon(self.resolve_type(ty, &Stability::Unknown)?))
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions crates/wit-parser/tests/ui/feature-types.wit
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package a:b;

interface foo {
variant bar {
x,
y
}

@unstable(feature = my-feature)
my-unstable: func(p: bar) -> result<_, bar>;

my-stable: func(p: bar) -> result<_, bar>;
}
70 changes: 70 additions & 0 deletions crates/wit-parser/tests/ui/feature-types.wit.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{
"worlds": [],
"interfaces": [
{
"name": "foo",
"types": {
"bar": 0
},
"functions": {
"my-stable": {
"name": "my-stable",
"kind": "freestanding",
"params": [
{
"name": "p",
"type": 0
}
],
"results": [
{
"type": 1
}
]
}
},
"package": 0
}
],
"types": [
{
"name": "bar",
"kind": {
"variant": {
"cases": [
{
"name": "x",
"type": null
},
{
"name": "y",
"type": null
}
]
}
},
"owner": {
"interface": 0
}
},
{
"name": null,
"kind": {
"result": {
"ok": null,
"err": 0
}
},
"owner": null
}
],
"packages": [
{
"name": "a:b",
"interfaces": {
"foo": 0
},
"worlds": {}
}
]
}
Loading