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.

Sign up for GitHub

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

Match names more precisely in update_lit_checks.py #6190

Merged
merged 2 commits into from
Jan 2, 2024
Merged

Match names more precisely in update_lit_checks.py #6190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ba1696
Match names more precisely in update_lit_checks.py
tlively Dec 20, 2023
b5a1ed8
fix string prefix
tlively Jan 2, 2024
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
20 changes: 14 additions & 6 deletions scripts/update_lit_checks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@
CHECK_PREFIX_RE = re.compile(r'.*--check-prefix[= ](\S+).*')
MODULE_RE = re.compile(r'^\(module.*$', re.MULTILINE)

ALL_ITEMS = '|'.join(['type', 'import', 'global', 'memory', 'data', 'table',
'elem', 'tag', 'export', 'start', 'func'])
DECL_ITEMS = '|'.join(['type', 'global', 'memory', 'data', 'table',
'elem', 'tag', 'start', 'func'])
IMPORT_ITEM = r'import\s*"[^"]*"\s*"[^"]*"\s*\((?:' + DECL_ITEMS + ')'
EXPORT_ITEM = r'export\s*"[^"]*"\s*\((?:' + DECL_ITEMS + ')'
ALL_ITEMS = DECL_ITEMS + '|' + IMPORT_ITEM + '|' + EXPORT_ITEM

# Regular names as well as the "declare" in (elem declare ... to get declarative
# segments included in the output.
ITEM_NAME = r'\$[^\s()]*|"[^\s()]*"|declare'
ITEM_NAME = r'\$[^\s()]*|\$"[^"]*"|declare'

# FIXME: This does not handle nested string contents. For example,
# (data (i32.const 10) "hello(")
Expand All @@ -55,6 +58,11 @@
FUZZ_EXEC_FUNC = re.compile(r'^\[fuzz-exec\] calling (?P<name>\S*)$')


def indentKindName(match):
# Return the indent, kind, and name from an ITEM_RE match
return (match[1], match[2].split()[0], match[3])


def warn(msg):
print(f'warning: {msg}', file=sys.stderr)

Expand Down Expand Up @@ -141,7 +149,7 @@ def parse_output_modules(text):
for module in split_modules(text):
items = []
for match in ITEM_RE.finditer(module):
kind, name = match[2], match[3]
_, kind, name = indentKindName(match)
end = find_end(module, match.end(1))
lines = module[match.start():end].split('\n')
items.append(((kind, name), lines))
Expand Down Expand Up @@ -247,7 +255,7 @@ def update_test(args, test, lines, tmp):
for line in lines:
match = ITEM_RE.match(line)
if match:
kind, name = match[2], match[3]
_, kind, name = indentKindName(match)
named_items.append((kind, name))

script = script_name
Expand Down Expand Up @@ -286,7 +294,7 @@ def pad(line):
output_lines.append(line)
continue

indent, kind, name = match.groups()
indent, kind, name = indentKindName(match)

for prefix, items in output.items():
# If the output for this prefix contains an item with this
Expand Down
9 changes: 6 additions & 3 deletions test/lit/basic/empty_imported_table.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
;; RUN: cat %t.bin.nodebug.wast | filecheck %s --check-prefix=CHECK-BIN-NODEBUG

(module
;; CHECK-TEXT: (import "env" "table" (table $timport$0 0 0 funcref))
;; CHECK-BIN: (import "env" "table" (table $timport$0 0 0 funcref))
;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 0 0 funcref))
(import "env" "table" (table 0 0 funcref))
;; CHECK-TEXT: (import "env" "table" (table $timport$0 0 0 funcref))

;; CHECK-TEXT: (memory $0 0)
;; CHECK-BIN: (import "env" "table" (table $timport$0 0 0 funcref))

;; CHECK-BIN: (memory $0 0)
;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 0 0 funcref))

;; CHECK-BIN-NODEBUG: (memory $0 0)
(memory $0 0)
)
19 changes: 12 additions & 7 deletions test/lit/basic/export-import.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,26 @@
;; CHECK-BIN: (type $v (func))
(type $v (func))
;; CHECK-TEXT: (import "env" "test2" (global $test2 i32))
;; CHECK-BIN: (import "env" "test2" (global $test2 i32))
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (import "env" "test2" (global $gimport$0 i32))
(import "env" "test1" (func $test1))
;; CHECK-TEXT: (import "env" "test1" (func $test1 (type $v)))
;; CHECK-BIN: (import "env" "test2" (global $test2 i32))

;; CHECK-BIN: (import "env" "test1" (func $test1 (type $v)))
;; CHECK-BIN-NODEBUG: (import "env" "test1" (func $fimport$0 (type $0)))
(import "env" "test1" (func $test1))
(import "env" "test2" (global $test2 i32))
;; CHECK-TEXT: (export "test1" (func $test1))
;; CHECK-BIN: (export "test1" (func $test1))
;; CHECK-BIN-NODEBUG: (export "test1" (func $fimport$0))
(export "test1" (func $test1))
;; CHECK-TEXT: (export "test2" (global $test2))
;; CHECK-BIN: (export "test2" (global $test2))
;; CHECK-BIN-NODEBUG: (export "test2" (global $gimport$0))
(export "test2" (global $test2))
)
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (import "env" "test2" (global $gimport$0 i32))

;; CHECK-BIN-NODEBUG: (import "env" "test1" (func $fimport$0 (type $0)))

;; CHECK-BIN-NODEBUG: (export "test1" (func $fimport$0))

;; CHECK-BIN-NODEBUG: (export "test2" (global $gimport$0))
Copy link
Member

Choose a reason for hiding this comment

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

Looks less readable here, as it emits it out past the end of the module for some reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is because round-tripping through the binary does not preserve the names we are now correctly matching against. That's not a new problem with this change, although it now correctly shows up in this case.

Copy link
Member

@kripken kripken Dec 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

But before it looks like it was just in the right place? I mean, before we had this:

 ;; CHECK-BIN: (import "env" "test1" (func $test1 (type $v)))
 ;; CHECK-BIN-NODEBUG:      (import "env" "test1" (func $fimport$0 (type $0)))

So yeah, we didn't round-trip the name $test1 but still the imports were perfectly aligned?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was in a nice place for the wrong reasons. If the input had more imports from "env", then they would all be bunched up together in the output rather than actually matched up with their corresponding input.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I actually think grouping all imports that are identical in module and name makes sense - do you disagree? In the quote from my last comment, env and test1 are identical in both lines, so it's nice to keep them together.

I can see how a more strict approach can justify moving them away, but I think it's a loss for readability. Could we perhaps both match names more precisely and also secondarily keep imports/exports with identical external names close together? Then there would not be a regression here from my perspective.

But if that's very difficult to do I won't insist.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are other cases (such as in the next PR) where this improves readability by correctly matching imports with their inputs where they would not have otherwise been grouped together.

The update script never reorders the input or the output, so in that sense all the output imports will always be grouped together, but now the way they are interleaved with the input is more consistent and the result is more readable in some cases, but not all cases.

Doing better than making the interleaving consistent would require reordering things, and that's not possible or desirable right now.

3 changes: 2 additions & 1 deletion test/lit/basic/hello_world.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
(memory $0 256 256)
;; CHECK-TEXT: (export "add" (func $add))
;; CHECK-BIN: (export "add" (func $add))
;; CHECK-BIN-NODEBUG: (export "add" (func $0))
(export "add" (func $add))
;; CHECK-TEXT: (func $add (type $i32_i32_=>_i32) (param $x i32) (param $y i32) (result i32)
;; CHECK-TEXT-NEXT: (i32.add
Expand All @@ -42,6 +41,8 @@
)
)
)
;; CHECK-BIN-NODEBUG: (export "add" (func $0))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (param $0 i32) (param $1 i32) (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (i32.add
;; CHECK-BIN-NODEBUG-NEXT: (local.get $0)
Expand Down
11 changes: 7 additions & 4 deletions test/lit/basic/imported_memory.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
(module
;; CHECK-TEXT: (import "env" "memory" (memory $0 256 256))
;; CHECK-BIN: (import "env" "memory" (memory $0 256 256))
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 256 256))
(import "env" "memory" (memory $0 256 256))
;; CHECK-TEXT: (import "env" "table" (table $timport$0 256 256 funcref))
;; CHECK-BIN: (import "env" "table" (table $timport$0 256 256 funcref))
;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 256 256 funcref))
(import "env" "table" (table 256 256 funcref))
)
;; CHECK-TEXT: (import "env" "table" (table $timport$0 256 256 funcref))

;; CHECK-BIN: (import "env" "table" (table $timport$0 256 256 funcref))

;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 256 256))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 256 256 funcref))
11 changes: 7 additions & 4 deletions test/lit/basic/imported_memory_growth.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
(module
;; CHECK-TEXT: (import "env" "memory" (memory $0 256))
;; CHECK-BIN: (import "env" "memory" (memory $0 256))
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 256))
(import "env" "memory" (memory $0 256))
;; CHECK-TEXT: (import "env" "table" (table $timport$0 256 funcref))
;; CHECK-BIN: (import "env" "table" (table $timport$0 256 funcref))
;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 256 funcref))
(import "env" "table" (table 256 funcref))
)
;; CHECK-TEXT: (import "env" "table" (table $timport$0 256 funcref))

;; CHECK-BIN: (import "env" "table" (table $timport$0 256 funcref))

;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 256))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 256 funcref))
3 changes: 2 additions & 1 deletion test/lit/basic/memory-import.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
(type $0 (func (result i32)))
;; CHECK-TEXT: (import "env" "memory" (memory $0 1 1))
;; CHECK-BIN: (import "env" "memory" (memory $0 1 1))
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 1 1))
(import "env" "memory" (memory $0 1 1))

;; CHECK-TEXT: (func $foo (type $0) (result i32)
Expand All @@ -35,6 +34,8 @@
)
)
)
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 1 1))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (i32.load offset=13
;; CHECK-BIN-NODEBUG-NEXT: (i32.const 37)
Expand Down
3 changes: 2 additions & 1 deletion test/lit/basic/memory-import64.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
(type $0 (func (result i32)))
;; CHECK-TEXT: (import "env" "memory" (memory $0 i64 1 1))
;; CHECK-BIN: (import "env" "memory" (memory $0 i64 1 1))
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 i64 1 1))
(import "env" "memory" (memory $0 i64 1 1))

;; CHECK-TEXT: (func $foo (type $0) (result i32)
Expand All @@ -35,6 +34,8 @@
)
)
)
;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 i64 1 1))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (i32.load offset=13
;; CHECK-BIN-NODEBUG-NEXT: (i64.const 37)
Expand Down
3 changes: 2 additions & 1 deletion test/lit/basic/min.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
(memory $0 256 256)
;; CHECK-TEXT: (export "floats" (func $floats))
;; CHECK-BIN: (export "floats" (func $floats))
;; CHECK-BIN-NODEBUG: (export "floats" (func $0))
(export "floats" (func $floats))

;; CHECK-TEXT: (func $floats (type $0) (param $f f32) (result f32)
Expand Down Expand Up @@ -183,6 +182,8 @@
)
)
)
;; CHECK-BIN-NODEBUG: (export "floats" (func $0))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (param $0 f32) (result f32)
;; CHECK-BIN-NODEBUG-NEXT: (local $1 f32)
;; CHECK-BIN-NODEBUG-NEXT: (f32.add
Expand Down
11 changes: 6 additions & 5 deletions test/lit/basic/multi-memories-basics.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
(type $none_=>_i32 (func (result i32)))
;; CHECK-TEXT: (import "env" "memory" (memory $importedMemory 1 1))
;; CHECK-BIN: (import "env" "memory" (memory $importedMemory 1 1))
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (type $1 (func (result i32)))

;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 1 1))
(import "env" "memory" (memory $importedMemory 1 1))
;; CHECK-TEXT: (memory $memory1 1 500)
;; CHECK-BIN: (memory $memory1 1 500)
Expand Down Expand Up @@ -367,6 +362,12 @@
)
)
)
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (type $1 (func (result i32)))

;; CHECK-BIN-NODEBUG: (import "env" "memory" (memory $mimport$0 1 1))

;; CHECK-BIN-NODEBUG: (memory $0 1 500)

;; CHECK-BIN-NODEBUG: (memory $1 1 800)
Expand Down
7 changes: 4 additions & 3 deletions test/lit/basic/multi-table.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@
;; CHECK-BIN: (global $g2 i32 (i32.const 0))
(global $g2 i32 (i32.const 0))

;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (import "a" "b" (table $timport$0 1 10 funcref))
(import "a" "b" (table $t1 1 10 funcref))
;; CHECK-TEXT: (table $t2 3 3 funcref)
;; CHECK-BIN: (table $t2 3 3 funcref)
Expand Down Expand Up @@ -116,6 +113,10 @@
;; CHECK-BIN-NEXT: )
(func $h)
)
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (import "a" "b" (table $timport$0 1 10 funcref))

;; CHECK-BIN-NODEBUG: (global $global$0 (ref null $0) (ref.func $0))

;; CHECK-BIN-NODEBUG: (global $global$1 i32 (i32.const 0))
Expand Down
3 changes: 2 additions & 1 deletion test/lit/basic/mutable-global.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
(type $0 (func))
;; CHECK-TEXT: (import "env" "global-mut" (global $global-mut (mut i32)))
;; CHECK-BIN: (import "env" "global-mut" (global $global-mut (mut i32)))
;; CHECK-BIN-NODEBUG: (import "env" "global-mut" (global $gimport$0 (mut i32)))
(import "env" "global-mut" (global $global-mut (mut i32)))

;; CHECK-TEXT: (func $foo (type $0)
Expand Down Expand Up @@ -44,6 +43,8 @@
)
)
)
;; CHECK-BIN-NODEBUG: (import "env" "global-mut" (global $gimport$0 (mut i32)))

;; CHECK-BIN-NODEBUG: (func $0 (type $0)
;; CHECK-BIN-NODEBUG-NEXT: (global.set $gimport$0
;; CHECK-BIN-NODEBUG-NEXT: (i32.add
Expand Down
25 changes: 14 additions & 11 deletions test/lit/basic/newsyntax.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,13 @@
;; RUN: cat %t.bin.nodebug.wast | filecheck %s --check-prefix=CHECK-BIN-NODEBUG

(module
(import "env" "table" (table 9 9 funcref))

;; CHECK-TEXT: (type $0 (func))

;; CHECK-TEXT: (type $1 (func (param i32 f64) (result i32)))

;; CHECK-TEXT: (import "env" "table" (table $timport$0 9 9 funcref))
;; CHECK-BIN: (type $0 (func))

;; CHECK-BIN: (type $1 (func (param i32 f64) (result i32)))

;; CHECK-BIN: (import "env" "table" (table $timport$0 9 9 funcref))
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (type $1 (func (param i32 f64) (result i32)))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 9 9 funcref))
(import "env" "table" (table 9 9 funcref))

;; CHECK-TEXT: (export "call_indirect" (func $call_indirect))

Expand All @@ -41,6 +32,12 @@
;; CHECK-TEXT-NEXT: (i32.const 1)
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-BIN: (type $0 (func))

;; CHECK-BIN: (type $1 (func (param i32 f64) (result i32)))

;; CHECK-BIN: (import "env" "table" (table $timport$0 9 9 funcref))

;; CHECK-BIN: (export "call_indirect" (func $call_indirect))

;; CHECK-BIN: (func $call_indirect (type $0)
Expand All @@ -62,6 +59,12 @@
(call_indirect (i32.const 1))
)
)
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (type $1 (func (param i32 f64) (result i32)))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 9 9 funcref))

;; CHECK-BIN-NODEBUG: (export "call_indirect" (func $0))

;; CHECK-BIN-NODEBUG: (func $0 (type $0)
Expand Down
33 changes: 18 additions & 15 deletions test/lit/basic/polymorphic_stack.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,13 @@

;; CHECK-BIN: (type $FUNCSIG$ii (func (param i32) (result i32)))
(type $FUNCSIG$ii (func (param i32) (result i32)))
(import "env" "table" (table 9 9 funcref))

;; CHECK-TEXT: (type $2 (func))

;; CHECK-TEXT: (type $3 (func (param i32)))

;; CHECK-TEXT: (import "env" "table" (table $timport$0 9 9 funcref))
;; CHECK-BIN: (type $2 (func))

;; CHECK-BIN: (type $3 (func (param i32)))

;; CHECK-BIN: (import "env" "table" (table $timport$0 9 9 funcref))
;; CHECK-BIN-NODEBUG: (type $0 (func (result i32)))

;; CHECK-BIN-NODEBUG: (type $1 (func (param i32) (result i32)))

;; CHECK-BIN-NODEBUG: (type $2 (func))

;; CHECK-BIN-NODEBUG: (type $3 (func (param i32)))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 9 9 funcref))
(import "env" "table" (table 9 9 funcref))

;; CHECK-TEXT: (func $break-and-binary (type $0) (result i32)
;; CHECK-TEXT-NEXT: (block $x (result i32)
Expand All @@ -53,6 +40,12 @@
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-BIN: (type $2 (func))

;; CHECK-BIN: (type $3 (func (param i32)))

;; CHECK-BIN: (import "env" "table" (table $timport$0 9 9 funcref))

;; CHECK-BIN: (func $break-and-binary (type $0) (result i32)
;; CHECK-BIN-NEXT: (block $label$1 (result i32)
;; CHECK-BIN-NEXT: (unreachable)
Expand Down Expand Up @@ -371,6 +364,16 @@
)
)
)
;; CHECK-BIN-NODEBUG: (type $0 (func (result i32)))

;; CHECK-BIN-NODEBUG: (type $1 (func (param i32) (result i32)))

;; CHECK-BIN-NODEBUG: (type $2 (func))

;; CHECK-BIN-NODEBUG: (type $3 (func (param i32)))

;; CHECK-BIN-NODEBUG: (import "env" "table" (table $timport$0 9 9 funcref))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (block $label$1 (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (unreachable)
Expand Down
Loading