From cde8acb2be47fca00202e50fa1e7417de4fc5825 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 27 Sep 2024 13:00:28 +0200 Subject: [PATCH] Fix when input redirection becomes visible. With `&parse-at/from` we were updating the internal state on our current position immediately, meaning they were visible already when evaluating other attributes on the same field afterwards, which is unexpected. Closes #1842. (cherry picked from commit 2eeffa6442a7382e07af698e27d0bd2a7a299b7c) --- doc/autogen/types/unit.rst | 5 ++-- spicy/toolchain/src/ast/operators/unit.cc | 6 ++-- .../src/compiler/codegen/parser-builder.cc | 3 +- .../spicy.types.unit.size-with-offset/output | 5 ++++ .../spicy/types/unit/offset-with-parse.spicy | 18 +++++++++++ tests/spicy/types/unit/size-with-offset.spicy | 30 +++++++++++++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 tests/Baseline/spicy.types.unit.size-with-offset/output create mode 100644 tests/spicy/types/unit/offset-with-parse.spicy create mode 100644 tests/spicy/types/unit/size-with-offset.spicy diff --git a/doc/autogen/types/unit.rst b/doc/autogen/types/unit.rst index a4a72cc031..ca0bd9b128 100644 --- a/doc/autogen/types/unit.rst +++ b/doc/autogen/types/unit.rst @@ -57,14 +57,13 @@ Returns the offset of the current location in the input stream relative to the unit's start. If executed from inside a field hook, - the offset will represent the first byte that the field has been - parsed from. + the offset will represent the beginning of the field, not the end. .. spicy:method:: unit::position unit position False iterator () Returns an iterator to the current position in the unit's input stream. If executed from inside a field hook, the position will - represent the first byte that the field has been parsed from. + represent the beginning of the field, not the end. .. spicy:method:: unit::set_input unit set_input False void (i: iterator) diff --git a/spicy/toolchain/src/ast/operators/unit.cc b/spicy/toolchain/src/ast/operators/unit.cc index f1118d013b..37da54aebe 100644 --- a/spicy/toolchain/src/ast/operators/unit.cc +++ b/spicy/toolchain/src/ast/operators/unit.cc @@ -191,7 +191,7 @@ class Offset : public hilti::BuiltInMemberCall { .doc = R"( Returns the offset of the current location in the input stream relative to the unit's start. If executed from inside a field hook, the offset will represent -the first byte that the field has been parsed from. +the beginning of the field, not the end. )", }; } @@ -212,8 +212,8 @@ class Position : public hilti::BuiltInMemberCall { .ns = "unit", .doc = R"( Returns an iterator to the current position in the unit's input stream. If -executed from inside a field hook, the position will represent the first byte -that the field has been parsed from. +executed from inside a field hook, the position will represent the beginning of +the field, not the end. )", }; } diff --git a/spicy/toolchain/src/compiler/codegen/parser-builder.cc b/spicy/toolchain/src/compiler/codegen/parser-builder.cc index 951a87dce9..18f03bde34 100644 --- a/spicy/toolchain/src/compiler/codegen/parser-builder.cc +++ b/spicy/toolchain/src/compiler/codegen/parser-builder.cc @@ -549,6 +549,7 @@ struct ProductionVisitor : public production::Visitor { auto etype = c->parseType()->type()->elementType(); auto container_element = builder()->addTmp("elem", etype); pushDestination(container_element); + pb->saveParsePosition(); // need to update position for container elements in case input is redirected } else if ( ! meta.isFieldProduction() ) @@ -1364,7 +1365,6 @@ struct ProductionVisitor : public production::Visitor { pstate.cur = builder()->addTmp("parse_cur", builder()->typeStreamView(), builder()->deref(tmp)); pstate.ncur = {}; pushState(std::move(pstate)); - pb->saveParsePosition(); } // Redirects input to be read from given stream position next. @@ -1380,7 +1380,6 @@ struct ProductionVisitor : public production::Visitor { pstate.cur = builder()->addTmp("parse_cur", cur); pstate.ncur = {}; pushState(std::move(pstate)); - pb->saveParsePosition(); } // Start sync and trial mode. diff --git a/tests/Baseline/spicy.types.unit.size-with-offset/output b/tests/Baseline/spicy.types.unit.size-with-offset/output new file mode 100644 index 0000000000..a60e44c0a4 --- /dev/null +++ b/tests/Baseline/spicy.types.unit.size-with-offset/output @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +x, 3 +x, \x01\x02\x03 +z, 4 +z, 1234 diff --git a/tests/spicy/types/unit/offset-with-parse.spicy b/tests/spicy/types/unit/offset-with-parse.spicy new file mode 100644 index 0000000000..a85595eca5 --- /dev/null +++ b/tests/spicy/types/unit/offset-with-parse.spicy @@ -0,0 +1,18 @@ +# @TEST-EXEC: printf 'a' | spicy-driver -d %INPUT +# +# @TEST-DOC: + +module Test; + +public type X = unit { + a: bytes &size=1; + + y1: Y &parse-at=self.input(); + y2: Y &parse-from=self.a; # Behaves identical. +}; + +type Y = unit { + a: bytes &size=1 { + assert self.offset() == 0 : "First element should always be at offset 0"; + } +}; diff --git a/tests/spicy/types/unit/size-with-offset.spicy b/tests/spicy/types/unit/size-with-offset.spicy new file mode 100644 index 0000000000..a61cf980d5 --- /dev/null +++ b/tests/spicy/types/unit/size-with-offset.spicy @@ -0,0 +1,30 @@ +# @TEST-EXEC: ${SCRIPTS}/printf '\x01\x02\x03\x04\0x05' | spicy-driver %INPUT >output +# @TEST-EXEC: btest-diff output +# +# @TEST-EXED: Check that changing the input with `&parse-at` isn't visible to other attributes on same field; regression test for #1842. + +module Test; + +import spicy; + +public type X = unit { + a: uint8; + b: uint8; + c: uint8; + + x: bytes &size=self.offset() # yields 3 + &parse-at=self.input() { + print "x", self.offset(); # yields still 3 + print "x", $$; + } + + y: uint8; + + z: bytes &size=self.offset() # yields 4 + &parse-from=b"12345" { + print "z", self.offset(); # yields still 4 + print "z", $$; + } + + d: uint8; +};