Skip to content

Commit

Permalink
Fix parsing for typed function reference types
Browse files Browse the repository at this point in the history
Previously an indexed ref type was parsed as a `Var`,
assuming that regular types would be represented as
index vars and named ref types would be named vars.

Unfortunately, it's also possible for a ref type to
be an indexed var that overlaps with a type (e.g.,
the type "any" which is 0x0 and the index 0).

This commit restructures the code so that both a `Type`
and `Var` are returned.

Closes #1889
  • Loading branch information
takikawa authored and keithw committed Aug 15, 2022
1 parent 8c50fd5 commit 5765b1c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 32 deletions.
59 changes: 28 additions & 31 deletions src/wast-parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ bool WastParser::ParseElemExprVarListOpt(ExprListVector* out_list) {
return !out_list->empty();
}

Result WastParser::ParseValueType(Var* out_type) {
Result WastParser::ParseValueType(Type* out_type, Var* out_var) {
WABT_TRACE(ParseValueType);

const bool is_ref_type = PeekMatchRefType();
Expand All @@ -911,8 +911,17 @@ Result WastParser::ParseValueType(Var* out_type) {
if (is_ref_type) {
EXPECT(Lpar);
EXPECT(Ref);
CHECK_RESULT(ParseVar(out_type));
Var name;
CHECK_RESULT(ParseVar(&name));
if (out_var) {
*out_var = name;
}
EXPECT(Rpar);
if (name.is_index()) {
*out_type = Type(Type::Reference, name.index());
} else {
*out_type = Type(Type::Reference, kInvalidIndex);
}
return Result::Ok;
}

Expand All @@ -937,7 +946,7 @@ Result WastParser::ParseValueType(Var* out_type) {
return Result::Error;
}

*out_type = Var(type, GetLocation());
*out_type = type;
return Result::Ok;
}

Expand All @@ -950,17 +959,15 @@ Result WastParser::ParseValueTypeList(
break;
}

Var type;
CHECK_RESULT(ParseValueType(&type));
Var index;
Type type;
CHECK_RESULT(ParseValueType(&type, &index));

if (type.is_index()) {
out_type_list->push_back(Type(type.index()));
} else {
assert(type.is_name());
if (index.is_name()) {
assert(options_->features.function_references_enabled());
type_names->emplace(out_type_list->size(), type.name());
out_type_list->push_back(Type(Type::Reference, kInvalidIndex));
type_names->emplace(out_type_list->size(), index.name());
}
out_type_list->push_back(type);
}

return Result::Ok;
Expand Down Expand Up @@ -1478,15 +1485,11 @@ Result WastParser::ParseField(Field* field) {
// TODO: Share with ParseGlobalType?
if (MatchLpar(TokenType::Mut)) {
field->mutable_ = true;
Var type;
CHECK_RESULT(ParseValueType(&type));
field->type = Type(type.index());
CHECK_RESULT(ParseValueType(&field->type));
EXPECT(Rpar);
} else {
field->mutable_ = false;
Var type;
CHECK_RESULT(ParseValueType(&type));
field->type = Type(type.index());
CHECK_RESULT(ParseValueType(&field->type));
}
return Result::Ok;
};
Expand Down Expand Up @@ -1844,20 +1847,18 @@ Result WastParser::ParseBoundValueTypeList(
while (MatchLpar(token)) {
if (PeekMatch(TokenType::Var)) {
std::string name;
Var type;
Var index;
Type type;
Location loc = GetLocation();
ParseBindVarOpt(&name);
CHECK_RESULT(ParseValueType(&type));
CHECK_RESULT(ParseValueType(&type, &index));
bindings->emplace(name,
Binding(loc, binding_index_offset + types->size()));
if (type.is_index()) {
types->push_back(Type(type.index()));
} else {
assert(type.is_name());
if (index.is_name()) {
assert(options_->features.function_references_enabled());
type_names->emplace(binding_index_offset + types->size(), type.name());
types->push_back(Type(Type::Reference, kInvalidIndex));
type_names->emplace(binding_index_offset + types->size(), index.name());
}
types->push_back(type);
} else {
CHECK_RESULT(ParseValueTypeList(types, type_names));
}
Expand Down Expand Up @@ -3147,15 +3148,11 @@ Result WastParser::ParseGlobalType(Global* global) {
WABT_TRACE(ParseGlobalType);
if (MatchLpar(TokenType::Mut)) {
global->mutable_ = true;
Var type;
CHECK_RESULT(ParseValueType(&type));
global->type = Type(type.index());
CHECK_RESULT(ParseValueType(&global->type));
CHECK_RESULT(ErrorIfLpar({"i32", "i64", "f32", "f64"}));
EXPECT(Rpar);
} else {
Var type;
CHECK_RESULT(ParseValueType(&type));
global->type = Type(type.index());
CHECK_RESULT(ParseValueType(&global->type));
}

return Result::Ok;
Expand Down
2 changes: 1 addition & 1 deletion src/wast-parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class WastParser {
bool ParseElemExprOpt(ExprList* out_elem_expr);
bool ParseElemExprListOpt(ExprListVector* out_list);
bool ParseElemExprVarListOpt(ExprListVector* out_list);
Result ParseValueType(Var* out_type);
Result ParseValueType(Type* out_type, Var* out_var = nullptr);
Result ParseValueTypeList(
TypeVector* out_type_list,
std::unordered_map<uint32_t, std::string>* type_names);
Expand Down
24 changes: 24 additions & 0 deletions test/typecheck/funcref-equality.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
;;; TOOL: wat2wasm
;;; ARGS: --enable-function-references
(module
(type $f32-f32-1 (func (param f32) (result f32)))
(type $f32-f32-2 (func (param f32) (result f32)))

(func $foo (param $f (ref $f32-f32-1)) (result f32)
(call_ref (f32.const 42.0) (local.get $f))
)

(func $bar (type $f32-f32-2)
(f32.const 1.0)
)

(func (export "main") (result f32)
;; $f32-f32-1 and $f32-f32-2 should be equal
(call $foo (ref.func $bar))
)

(elem declare funcref (ref.func $bar))
)

(;; STDERR ;;;
;;; STDERR ;;)

0 comments on commit 5765b1c

Please sign in to comment.