Skip to content

Implement type imports and exports #7330

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
Validation fixes
  • Loading branch information
vouillon committed Feb 27, 2025
commit fce76ffd505816434f6df9df13bdf2e9a1b45b2a
11 changes: 4 additions & 7 deletions src/parser/parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ Result<std::vector<Name>> inlineExports(Lexer&);
template<typename Ctx> Result<> comptype(Ctx&);
template<typename Ctx> Result<> sharecomptype(Ctx&);
template<typename Ctx> Result<> subtype(Ctx&);
template<typename Ctx> MaybeResult<> typedef_(Ctx&, bool inRecGroup);
template<typename Ctx> MaybeResult<> typedef_(Ctx&);
template<typename Ctx> MaybeResult<> rectype(Ctx&);
template<typename Ctx> MaybeResult<typename Ctx::LocalsT> locals(Ctx&);
template<typename Ctx> MaybeResult<> import_(Ctx&);
Expand Down Expand Up @@ -2983,7 +2983,7 @@ template<typename Ctx> Result<> subtype(Ctx& ctx) {
// typedef ::= '(' 'type' id? ('(' 'export' name ')')* subtype ')'
// | '(' 'type' id? ('(' 'export' name ')')*
// '(' 'import' mod:name nm:name ')' typetype ')'
template<typename Ctx> MaybeResult<> typedef_(Ctx& ctx, bool inRecGroup) {
template<typename Ctx> MaybeResult<> typedef_(Ctx& ctx) {
auto pos = ctx.in.getPos();

if (!ctx.in.takeSExprStart("type"sv)) {
Expand All @@ -3002,9 +3002,6 @@ template<typename Ctx> MaybeResult<> typedef_(Ctx& ctx, bool inRecGroup) {
CHECK_ERR(import);

if (import) {
if (inRecGroup) {
return ctx.in.err("type import not allowed in recursive group");
}
auto typePos = ctx.in.getPos();
auto type = typetype(ctx);
CHECK_ERR(type);
Expand All @@ -3030,15 +3027,15 @@ template<typename Ctx> MaybeResult<> rectype(Ctx& ctx) {
if (ctx.in.takeSExprStart("rec"sv)) {
size_t startIndex = ctx.getRecGroupStartIndex();
size_t groupLen = 0;
while (auto type = typedef_(ctx, true)) {
while (auto type = typedef_(ctx)) {
CHECK_ERR(type);
++groupLen;
}
if (!ctx.in.takeRParen()) {
return ctx.in.err("expected type definition or end of recursion group");
}
ctx.addRecGroup(startIndex, groupLen);
} else if (auto type = typedef_(ctx, false)) {
} else if (auto type = typedef_(ctx)) {
CHECK_ERR(type);
} else {
return {};
Expand Down
2 changes: 2 additions & 0 deletions src/wasm-type.h
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,8 @@ struct TypeBuilder {
InvalidFuncType,
// A type import has an invalid bound.
InvalidBoundType,
// Type import in recursive group
ImportInRecGroup,
// A non-shared field of a shared heap type.
InvalidUnsharedField,
};
Expand Down
26 changes: 21 additions & 5 deletions src/wasm/wasm-type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,8 @@ std::ostream& operator<<(std::ostream& os, TypeBuilder::ErrorReason reason) {
return os << "Continuation has invalid function type";
case TypeBuilder::ErrorReason::InvalidBoundType:
return os << "Type import has invalid bound type";
case TypeBuilder::ErrorReason::ImportInRecGroup:
return os << "Type import in recursive group";
case TypeBuilder::ErrorReason::InvalidUnsharedField:
return os << "Heap type has an invalid unshared field";
}
Expand Down Expand Up @@ -2316,14 +2318,17 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) {
case HeapTypeKind::Array:
return typer.isSubType(sub.array, super.array);
case HeapTypeKind::Import:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should return false here. It seems possible for someone to accidentally set a supertype on an import entry in a TypeBuilder. We should also check whether the supertype is an import here.

return false;
case HeapTypeKind::Basic:
break;
}
WASM_UNREACHABLE("unknown kind");
}

std::optional<TypeBuilder::ErrorReason>
validateType(HeapTypeInfo& info, std::unordered_set<HeapType>& seenTypes) {
validateType(HeapTypeInfo& info,
std::unordered_set<HeapType>& seenTypes,
bool inRecGroup) {
if (auto* super = info.supertype) {
// The supertype must be canonical (i.e. defined in a previous rec group)
// or have already been defined in this rec group.
Expand All @@ -2340,6 +2345,14 @@ validateType(HeapTypeInfo& info, std::unordered_set<HeapType>& seenTypes) {
return TypeBuilder::ErrorReason::InvalidFuncType;
}
}
if (info.isImport()) {
if (!info.import.bound.isBasic()) {
return TypeBuilder::ErrorReason::InvalidBoundType;
}
if (inRecGroup) {
return TypeBuilder::ErrorReason::ImportInRecGroup;
}
}
if (info.share == Shared) {
switch (info.kind) {
case HeapTypeKind::Func:
Expand All @@ -2365,9 +2378,8 @@ validateType(HeapTypeInfo& info, std::unordered_set<HeapType>& seenTypes) {
break;
}
case HeapTypeKind::Import:
if (!info.import.bound.isShared()) {
return TypeBuilder::ErrorReason::InvalidBoundType;
}
// The sharedness is inherited (see buildRecGroup below).
assert(info.import.bound.isShared());
break;
case HeapTypeKind::Basic:
WASM_UNREACHABLE("unexpected kind");
Expand Down Expand Up @@ -2440,7 +2452,11 @@ buildRecGroup(std::unique_ptr<RecGroupInfo>&& groupInfo,
std::unordered_set<HeapType> seenTypes;
for (size_t i = 0; i < typeInfos.size(); ++i) {
auto& info = typeInfos[i];
if (auto err = validateType(*info, seenTypes)) {
if (info->isImport()) {
// Sharedness is inherited from the bound
info->share = info->import.bound.getShared();
}
if (auto err = validateType(*info, seenTypes, typeInfos.size() > 1)) {
return {TypeBuilder::Error{i, *err}};
}
seenTypes.insert(asHeapType(info));
Expand Down