From e401ce075d4f08fd550c74e696a29bfece0dc3ff Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jul 2026 17:09:37 -0700 Subject: [PATCH] [NFC] Track grammar of definitions in parser When e.g. a function is declared in the text format, it can either be with a normal `func` production (which may or may not have an inline import declaration) or it can be an `import` production. Previously the parser would just store the location of the declaration but not the kind of production it used. In later parser phases where the declaration has to be parsed again, we would just try both productions to see which one worked. This is rather hacky and brittle and will not play nicely with the text format for compact imports, so refactor to explicitly track how the declaration should be parsed. --- src/parser/context-decls.cpp | 31 ++++++++------ src/parser/context-defs.cpp | 6 ++- src/parser/contexts.h | 71 ++++++++++++++++++++++---------- src/parser/parse-5-defs.cpp | 8 ++-- src/parser/parsers.h | 50 ++++++++++++++++------ src/parser/wat-parser-internal.h | 8 ++-- 6 files changed, 119 insertions(+), 55 deletions(-) diff --git a/src/parser/context-decls.cpp b/src/parser/context-decls.cpp index 0298f1cee9a..2174b1bad83 100644 --- a/src/parser/context-decls.cpp +++ b/src/parser/context-decls.cpp @@ -71,13 +71,14 @@ Result<> ParseDeclsCtx::addFunc(Name name, Exactness exact, std::optional, std::vector&& annotations, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto f = addFuncDecl(pos, name, import); CHECK_ERR(f); CHECK_ERR(addExports(in, wasm, *f, exports, ExternalKind::Function)); funcDefs.push_back( - {name, pos, Index(funcDefs.size()), std::move(annotations)}); + {name, pos, Index(funcDefs.size()), std::move(annotations), kind}); return Ok{}; } @@ -110,13 +111,14 @@ Result<> ParseDeclsCtx::addTable(Name name, ImportNames* import, TableType type, std::optional, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto t = addTableDecl(pos, name, import, type); CHECK_ERR(t); CHECK_ERR(addExports(in, wasm, *t, exports, ExternalKind::Table)); // TODO: table annotations - tableDefs.push_back({name, pos, Index(tableDefs.size()), {}}); + tableDefs.push_back({name, pos, Index(tableDefs.size()), {}, kind}); return Ok{}; } @@ -167,13 +169,14 @@ Result<> ParseDeclsCtx::addMemory(Name name, const std::vector& exports, ImportNames* import, MemType type, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto m = addMemoryDecl(pos, name, import, type); CHECK_ERR(m); CHECK_ERR(addExports(in, wasm, *m, exports, ExternalKind::Memory)); // TODO: memory annotations - memoryDefs.push_back({name, pos, Index(memoryDefs.size()), {}}); + memoryDefs.push_back({name, pos, Index(memoryDefs.size()), {}, kind}); return Ok{}; } @@ -213,13 +216,14 @@ Result<> ParseDeclsCtx::addGlobal(Name name, ImportNames* import, GlobalTypeT, std::optional, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto g = addGlobalDecl(pos, name, import); CHECK_ERR(g); CHECK_ERR(addExports(in, wasm, *g, exports, ExternalKind::Global)); // TODO: global annotations - globalDefs.push_back({name, pos, Index(globalDefs.size()), {}}); + globalDefs.push_back({name, pos, Index(globalDefs.size()), {}, kind}); return Ok{}; } @@ -239,7 +243,8 @@ Result<> ParseDeclsCtx::addElem( e->name = name; } // TODO: element segment annotations - elemDefs.push_back({name, pos, Index(wasm.elementSegments.size()), {}}); + elemDefs.push_back( + {name, pos, Index(wasm.elementSegments.size()), {}, DefKind::Definition}); wasm.addElementSegment(std::move(e)); return Ok{}; } @@ -264,7 +269,8 @@ Result<> ParseDeclsCtx::addData(Name name, } d->data = std::move(data); // TODO: data segment annotations - dataDefs.push_back({name, pos, Index(wasm.dataSegments.size()), {}}); + dataDefs.push_back( + {name, pos, Index(wasm.dataSegments.size()), {}, DefKind::Definition}); wasm.addDataSegment(std::move(d)); return Ok{}; } @@ -292,13 +298,14 @@ Result<> ParseDeclsCtx::addTag(Name name, const std::vector& exports, ImportNames* import, TypeUseT type, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto t = addTagDecl(pos, name, import); CHECK_ERR(t); CHECK_ERR(addExports(in, wasm, *t, exports, ExternalKind::Tag)); // TODO: tag annotations - tagDefs.push_back({name, pos, Index(tagDefs.size()), {}}); + tagDefs.push_back({name, pos, Index(tagDefs.size()), {}, kind}); return Ok{}; } diff --git a/src/parser/context-defs.cpp b/src/parser/context-defs.cpp index d0d170ed2c5..f191ea638a9 100644 --- a/src/parser/context-defs.cpp +++ b/src/parser/context-defs.cpp @@ -55,7 +55,8 @@ Result<> ParseDefsCtx::addGlobal(Name, ImportNames*, GlobalTypeT, std::optional exp, - Index) { + Index, + DefKind) { if (exp) { wasm.globals[index]->init = *exp; } @@ -67,7 +68,8 @@ Result<> ParseDefsCtx::addTable(Name, ImportNames*, TableTypeT, std::optional init, - Index) { + Index, + DefKind) { if (init) { wasm.tables[index]->init = *init; } diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 31a3aa253e3..0a403cf1073 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -63,6 +63,11 @@ struct TableType { Limits limits; }; +enum class DefKind { + ImportDesc, + Definition, +}; + // The location, possible name, and index in the respective module index space // of a module-level definition in the input. struct DefPos { @@ -70,6 +75,7 @@ struct DefPos { Index pos; Index index; std::vector annotations; + DefKind kind; }; struct GlobalType { @@ -1074,13 +1080,15 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { void setSupertype(HeapTypeT) {} void finishTypeDef(Name name, Index pos) { // TODO: type annotations - typeDefs.push_back({name, pos, Index(typeDefs.size()), {}}); + typeDefs.push_back( + {name, pos, Index(typeDefs.size()), {}, DefKind::Definition}); } size_t getRecGroupStartIndex() { return 0; } void addRecGroup(Index, size_t) {} void finishRectype(Index pos) { // TODO: type annotations - recTypeDefs.push_back({{}, pos, Index(recTypeDefs.size()), {}}); + recTypeDefs.push_back( + {{}, pos, Index(recTypeDefs.size()), {}, DefKind::Definition}); } bool skipFunctionBody(); @@ -1135,7 +1143,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { Exactness exact, std::optional, std::vector&&, - Index pos); + Index pos, + DefKind kind); Result addTableDecl(Index pos, Name name, @@ -1146,7 +1155,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { ImportNames*, TableType, std::optional, - Index); + Index, + DefKind kind); // TODO: Record index of implicit elem for use when parsing types and instrs. Result<> addImplicitElems(TypeT, ElemListT&& elems); @@ -1158,7 +1168,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { const std::vector& exports, ImportNames* import, MemType type, - Index pos); + Index pos, + DefKind kind); Result<> addImplicitData(DataStringT&& data); @@ -1169,14 +1180,15 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { ImportNames* import, GlobalTypeT, std::optional, - Index pos); + Index pos, + DefKind kind); Result<> addStart(FuncIdxT, Index pos) { if (!startDefs.empty()) { return Err{"unexpected extra 'start' function"}; } // TODO: start function annotations. - startDefs.push_back({{}, pos, 0, {}}); + startDefs.push_back({{}, pos, 0, {}, DefKind::Definition}); return Ok{}; } @@ -1196,7 +1208,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { const std::vector& exports, ImportNames* import, TypeUseT type, - Index pos); + Index pos, + DefKind kind); Result<> addExport(Index pos, Ok, Name, ExternalKind) { exportDefs.push_back(pos); @@ -1526,7 +1539,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, Exactness exact, std::optional locals, std::vector&& annotations, - Index pos) { + Index pos, + DefKind kind) { auto& f = wasm.functions[index]; if (!type.type.isSignature()) { return in.err(pos, "expected signature type"); @@ -1556,7 +1570,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, ImportNames*, Type ttype, std::optional init, - Index pos) { + Index pos, + DefKind kind) { auto& t = wasm.tables[index]; if (!ttype.isRef()) { return in.err(pos, "expected reference type"); @@ -1572,8 +1587,12 @@ struct ParseModuleTypesCtx : TypeParserCtx, return Ok{}; } - Result<> - addMemory(Name, const std::vector&, ImportNames*, MemTypeT, Index) { + Result<> addMemory(Name, + const std::vector&, + ImportNames*, + MemTypeT, + Index, + DefKind kind) { return Ok{}; } @@ -1584,7 +1603,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, ImportNames*, GlobalType type, std::optional, - Index) { + Index, + DefKind kind) { auto& g = wasm.globals[index]; g->mutable_ = type.mutability; g->type = type.type; @@ -1600,8 +1620,12 @@ struct ParseModuleTypesCtx : TypeParserCtx, Result<> addDeclareElem(Name, ElemListT&&, Index) { return Ok{}; } - Result<> - addTag(Name, const std::vector&, ImportNames*, TypeUse use, Index pos) { + Result<> addTag(Name, + const std::vector&, + ImportNames*, + TypeUse use, + Index pos, + DefKind kind) { auto& t = wasm.tags[index]; if (!use.type.isSignature()) { return in.err(pos, "tag type must be a signature"); @@ -1916,7 +1940,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Exactness, std::optional, std::vector&&, - Index) { + Index, + DefKind) { return Ok{}; } @@ -1925,10 +1950,11 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { ImportNames*, TableTypeT, std::optional, - Index); + Index, + DefKind); - Result<> - addMemory(Name, const std::vector&, ImportNames*, TableTypeT, Index) { + Result<> addMemory( + Name, const std::vector&, ImportNames*, TableTypeT, Index, DefKind) { return Ok{}; } @@ -1937,7 +1963,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { ImportNames*, GlobalTypeT, std::optional exp, - Index); + Index, + DefKind); Result<> addStart(Name name, Index pos) { wasm.start = name; @@ -1961,8 +1988,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Result<> addData(Name, Name* mem, std::optional offset, DataStringT, Index pos); - Result<> - addTag(Name, const std::vector, ImportNames*, TypeUseT, Index) { + Result<> addTag( + Name, const std::vector, ImportNames*, TypeUseT, Index, DefKind) { return Ok{}; } diff --git a/src/parser/parse-5-defs.cpp b/src/parser/parse-5-defs.cpp index 82909ea1a16..14ad9ffcdd5 100644 --- a/src/parser/parse-5-defs.cpp +++ b/src/parser/parse-5-defs.cpp @@ -49,12 +49,14 @@ Result<> parseDefinitions( if (!f->imported()) { CHECK_ERR(ctx.visitFunctionStart(f)); } - if (auto parsed = func(ctx)) { - CHECK_ERR(parsed); - } else { + if (decls.funcDefs[i].kind == DefKind::ImportDesc) { auto im = import_(ctx); assert(im); CHECK_ERR(im); + } else { + auto parsed = func(ctx); + assert(parsed); + CHECK_ERR(parsed); } if (!f->imported()) { auto end = ctx.irBuilder.visitEnd(); diff --git a/src/parser/parsers.h b/src/parser/parsers.h index d6f2297ff57..8c43fb3e30c 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -3454,30 +3454,49 @@ template MaybeResult<> import_(Ctx& ctx) { CHECK_ERR(use); auto [type, exact] = *use; // TODO: function import annotations - CHECK_ERR(ctx.addFunc( - name ? *name : Name{}, {}, &names, type, exact, std::nullopt, {}, pos)); + CHECK_ERR(ctx.addFunc(name ? *name : Name{}, + {}, + &names, + type, + exact, + std::nullopt, + {}, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("table"sv)) { auto name = ctx.in.takeID(); auto type = tabletype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTable( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addTable(name ? *name : Name{}, + {}, + &names, + *type, + std::nullopt, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("memory"sv)) { auto name = ctx.in.takeID(); auto type = memtype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addMemory(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addMemory( + name ? *name : Name{}, {}, &names, *type, pos, DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("global"sv)) { auto name = ctx.in.takeID(); auto type = globaltype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addGlobal( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addGlobal(name ? *name : Name{}, + {}, + &names, + *type, + std::nullopt, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("tag"sv)) { auto name = ctx.in.takeID(); auto type = typeuse(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTag(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addTag( + name ? *name : Name{}, {}, &names, *type, pos, DefKind::ImportDesc)); } else { return ctx.in.err("expected import description"); } @@ -3551,7 +3570,8 @@ template MaybeResult<> func(Ctx& ctx) { exact, localVars, std::move(annotations), - pos)); + pos, + DefKind::Definition)); return Ok{}; } @@ -3640,7 +3660,8 @@ template MaybeResult<> table(Ctx& ctx) { return ctx.in.err("expected end of table declaration"); } - CHECK_ERR(ctx.addTable(name, *exports, import.getPtr(), *ttype, init, pos)); + CHECK_ERR(ctx.addTable( + name, *exports, import.getPtr(), *ttype, init, pos, DefKind::Definition)); if (elems) { CHECK_ERR(ctx.addImplicitElems(*type, std::move(*elems))); @@ -3711,7 +3732,8 @@ template MaybeResult<> memory(Ctx& ctx) { return ctx.in.err("expected end of memory declaration"); } - CHECK_ERR(ctx.addMemory(name, *exports, import.getPtr(), *mtype, pos)); + CHECK_ERR(ctx.addMemory( + name, *exports, import.getPtr(), *mtype, pos, DefKind::Definition)); if (data) { CHECK_ERR(ctx.addImplicitData(std::move(*data))); @@ -3754,7 +3776,8 @@ template MaybeResult<> global(Ctx& ctx) { return ctx.in.err("expected end of global"); } - CHECK_ERR(ctx.addGlobal(name, *exports, import.getPtr(), *type, exp, pos)); + CHECK_ERR(ctx.addGlobal( + name, *exports, import.getPtr(), *type, exp, pos, DefKind::Definition)); return Ok{}; } @@ -4027,7 +4050,8 @@ template MaybeResult<> tag(Ctx& ctx) { return ctx.in.err("expected end of tag"); } - CHECK_ERR(ctx.addTag(name, *exports, import.getPtr(), *type, pos)); + CHECK_ERR(ctx.addTag( + name, *exports, import.getPtr(), *type, pos, DefKind::Definition)); return Ok{}; } diff --git a/src/parser/wat-parser-internal.h b/src/parser/wat-parser-internal.h index 2f4ecc7c3e1..9219b62012c 100644 --- a/src/parser/wat-parser-internal.h +++ b/src/parser/wat-parser-internal.h @@ -81,12 +81,14 @@ Result<> parseDefs(Ctx& ctx, ctx.index = def.index; WithPosition with(ctx, def.pos); ctx.in.setAnnotations(def.annotations); - if (auto parsed = parser(ctx)) { - CHECK_ERR(parsed); - } else { + if (def.kind == DefKind::ImportDesc) { auto im = import_(ctx); assert(im); CHECK_ERR(im); + } else { + auto parsed = parser(ctx); + assert(parsed); + CHECK_ERR(parsed); } } return Ok{};