ProtoDef-io / ProtoDef-io/node-protodef

compiler: Incorrect code gen for anon switch with default field

Open
#132 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
35
Forks
25
Avg merge
4d 3m
Merged PRs (30d)
2

Description

https://github.com/PrismarineJS/minecraft-data/blob/c6a1e9af1a144d78d7d020ee55029aa1d3922a34/data/bedrock/1.18.0/protocol.json#L6571-L6617

above uses an anonymous switch field with a default condition, but the compiler generates code that repeatedly reads the default condition (minecart_entity_runtime_id):

packet_command_block_update: (buffer, offset) => {
  let { value: is_block, size: is_blockSize } = (ctx.bool)(buffer, offset)
  let { value: position, size: positionSize } = ((buffer, offset) => {
    switch (is_block) {
      case true: return (ctx.BlockCoordinates)(buffer, offset)
      default: return ((buffer, offset) => {
        let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = (ctx.varint64)(buffer, offset) // <==
        return { value: { minecart_entity_runtime_id }, size: minecart_entity_runtime_idSize}
      })(buffer, offset)
    }
  })(buffer, offset + is_blockSize)
  let { value: mode, size: modeSize } = ((buffer, offset) => {
    switch (is_block) {
      case true: return ((buffer, offset) => {
        const { value, size } = (ctx.varint)(buffer, offset)
        return { value: {"0":"impulse","1":"repeat","2":"chain"}[value] || value, size }
      })(buffer, offset)
      default: return ((buffer, offset) => {
        let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = (ctx.varint64)(buffer, offset)
        return { value: { minecart_entity_runtime_id }, size: minecart_entity_runtime_idSize}
      })(buffer, offset)
    }
  })(buffer, offset + is_blockSize + positionSize)
  let { value: needs_redstone, size: needs_redstoneSize } = ((buffer, offset) => {
    switch (is_block) {
      case true: return (ctx.bool)(buffer, offset)
      default: return ((buffer, offset) => {
        let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = (ctx.varint64)(buffer, offset)
        return { value: { minecart_entity_runtime_id }, size: minecart_entity_runtime_idSize}
      })(buffer, offset)
    }
  })(buffer, offset + is_blockSize + positionSize + modeSize)
  let { value: conditional, size: conditionalSize } = ((buffer, offset) => {
    switch (is_block) {
      case true: return (ctx.bool)(buffer, offset)
      default: return ((buffer, offset) => {
        let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = (ctx.varint64)(buffer, offset)
        return { value: { minecart_entity_runtime_id }, size: minecart_entity_runtime_idSize}
      })(buffer, offset)
    }
  })(buffer, offset + is_blockSize + positionSize + modeSize + needs_redstoneSize)
  let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = ((buffer, offset) => {
    switch (is_block) {
      default: return (ctx.varint64)(buffer, offset)
    }
  })(buffer, offset + is_blockSize + positionSize + modeSize + needs_redstoneSize + conditionalSize)

not using an anon field or not using default generates correct code, e.g.

  let { value: data, size: dataSize } = ((buffer, offset) => {
        switch (is_block) {
          case true: return ((buffer, offset) => {
            let { value: position1, size: position1Size } = (ctx.BlockCoordinates)(buffer, offset)
            let { value: mode1, size: mode1Size } = ((buffer, offset) => {
              const { value, size } = (ctx.varint)(buffer, offset)
              return { value: {"0":"impulse","1":"repeat","2":"chain"}[value] || value, size }
            })(buffer, offset + position1Size)
            let { value: needs_redstone, size: needs_redstoneSize } = (ctx.bool)(buffer, offset + position1Size + mode1Size)
            let { value: conditional, size: conditionalSize } = (ctx.bool)(buffer, offset + position1Size + mode1Size + needs_redstoneSize)
            return { value: { position: position1, mode: mode1, needs_redstone, conditional }, size: position1Size + mode1Size + needs_redstoneSize + conditionalSize}
          })(buffer, offset)
          default: return ((buffer, offset) => {
            let { value: minecart_entity_runtime_id, size: minecart_entity_runtime_idSize } = (ctx.varint64)(buffer, offset)
            return { value: { minecart_entity_runtime_id }, size: minecart_entity_runtime_idSize}
          })(buffer, offset)
        }
      })(buffer, offset + is_blockSize)

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the anonymous switch and default condition in data/bedrock/1.18.0/protocol.json at lines 6571-6617, then reproduce the generated packet_command_block_update code shown in the issue. Trace the compiler path that expands anonymous switch fields; done means the default branch is emitted once rather than rereading minecart_entity_runtime_id for each following field, while the non-default examples remain correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.