Skip to content

isthmus: FieldSelectionConverter converts the container operand before the ROW and MAP branches decline #1409

Open

Description

@nielspardon

FieldSelectionConverter.convert converts the container operand (topLevelConverter.apply(toDereference)) before the ROW and MAP branches decide whether they can handle the index, so a call they decline has already converted, and then thrown away, its whole operand. #1261 moved the decline for unsafe OFFSET/ORDINAL ahead of the conversion, but the other declines still come after it: a ROW subscript with a non-integer literal (r['q'], see #1385), a MAP subscript whose key isn't a fixed-char literal (m[1] on MAP<INTEGER, INTEGER>, see #1386), and containers of any other type.

// m : MAP<INTEGER, INTEGER>
ITEM(m, 1)  // converts m once, then: IllegalArgumentException: Unable to convert call ITEM(map<i32,i32>, i32).

Measured on main at af247e6. With the default converters this is wasted work, and an error from converting the operand surfaces instead of the decline. When a converter added after the built-ins (the ConverterProvider.Builder call-converter list) does claim the call, it converts the operand again, so nested declined subscripts convert their innermost operand 2^depth times. Deciding from the container type and the index literal before calling topLevelConverter.apply fixes all three.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions