inst defintion and its optional stack_effect
#133412
Comments
|
I think it's easier to change the docs than to change the parser. And it's better, IMO, to keep an explicit stack effect as it helps readability and avoids having special cases during parsing. |
|
@picnixz I agree. Clearly defining the stack effect and enforcing it at the parser level will help ensure consistency in the |
|
The @contextual
def inst_header(self) -> InstHeader | None:
# annotation* inst(NAME, (inputs -- outputs))
# | annotation* op(NAME, (inputs -- outputs))
annotations = []
while anno := self.expect(lx.ANNOTATION):
if anno.text == "replicate":
self.require(lx.LPAREN)
times = self.require(lx.NUMBER)
self.require(lx.RPAREN)
annotations.append(f"replicate({times.text})")
else:
annotations.append(anno.text)
tkn = self.expect(lx.INST)
if not tkn:
tkn = self.expect(lx.OP)
if tkn:
kind = cast(Literal["inst", "op"], tkn.text)
if self.expect(lx.LPAREN) and (tkn := self.expect(lx.IDENTIFIER)):
name = tkn.text
if self.expect(lx.COMMA):
inp, outp = self.io_effect()
if self.expect(lx.RPAREN):
if (tkn := self.peek()) and tkn.kind == lx.LBRACE:
return InstHeader(annotations, kind, name, inp, outp)
return None |
|
I think it should be enough to update the internal docs. I don't think there's any urgent need to support the optional syntax (imo, it's better to be explicit) |
|
I haven't merged your PR yet :). I'm only merging PRs when I'm on my Linux session and before 8PM (Paris time) |
The `stack_effect` is incorrectly documented as being allowed to be optional.
The `stack_effect` is incorrectly documented as being allowed to be optional. (cherry picked from commit f77dac6) Co-authored-by: Nybblista <170842536+nybblista@users.noreply.github.com>
The `stack_effect` is incorrectly documented as being allowed to be optional. (cherry picked from commit f77dac6) Co-authored-by: Nybblista <170842536+nybblista@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
The DSL specification syntax definition of the
instshows that thestack_effectis optional,including its parentheses:
But if we defined a new instruction, let's say,
MY_INSTRUCTIONas:We will get the following error:
However, if we defined
MY_INSTRUCTIONwith an emptystack-effect, it will work!Linked PRs
instdefinition (GH-133708) #133729instdefinition (GH-133708) #133730The text was updated successfully, but these errors were encountered: