Skip to content

ir: Add end locations in IR plan statements - #9007

Merged
charlieegan3 merged 3 commits into
open-policy-agent:mainfrom
charlieegan3:end-locations-ir-plans
Aug 13, 2026
Merged

ir: Add end locations in IR plan statements#9007
charlieegan3 merged 3 commits into
open-policy-agent:mainfrom
charlieegan3:end-locations-ir-plans

Conversation

@charlieegan3

Copy link
Copy Markdown
Contributor

This will allow sub line coverage reporting in IR evaluators.

Follows/related to #8752 to allow feature parity with topdown reporting.

Helps with addressing issues like this:

Screenshot

Example before

            "stmts": [
              {
                "type": "ReturnLocalStmt",
                "stmt": {
                  "source": 2,
                  "file": 0,
                  "col": 1,
                  "row": 5
                }
              }
            ]

Example after

 "stmts": [
              {
                "type": "ReturnLocalStmt",
                "stmt": {
                  "source": 2,
                  "file": 0,
                  "col": 1,
                  "row": 5,
                  "end_col": 14, // new and improved!
                  "end_row": 5 // new and improved!
                }
              }
            ]

Comment thread v1/ir/ir.go

// SetLocation sets the Location for a given Stmt.
func (l *Location) SetLocation(index, row, col int, file, text string) {
func (l *Location) SetLocation(index, row, col int, file string, text []byte) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a breaking change, but it seemed acceptable given:

  • IR plan integration point is intended to be not go.
  • text []byte is a simple change for any callers
  • text in location is []byte

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Switching text to []byte changes what Pretty prints, pretty.go dumps the struct with %+v, and Go renders a []byte as numbers rather than text.

A String() method on ir.Location would fix it without giving up the []byte change, since %+v picks it up automatically.

@charlieegan3 charlieegan3 Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, interesting... I have added 26ade847c to address. it's not ideal but adding String() on the location doesn't work since it's embedded into Stmt and so String set there risks only showing the Location string representation.

@charlieegan3
charlieegan3 force-pushed the end-locations-ir-plans branch 2 times, most recently from 07eb8f2 to a03c2fa Compare August 12, 2026 14:38
Comment thread cmd/build_jsonv2_test.go
got := strings.ReplaceAll(string(planBytes), root, "TEMPDIR")

expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3}}]}],"path":["g0","test","p"]}]}}`
expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"end_col":9,"end_row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]}],"path":["g0","test","p"]}]}}`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These just add the new fields.

This will allow sub line coverage reporting in IR evaluators.

Follows/related to open-policy-agent#8752 to
allow feature parity with topdown reporting.

Signed-off-by: Charlie Egan <charlie_egan@apple.com>
@charlieegan3
charlieegan3 force-pushed the end-locations-ir-plans branch from a03c2fa to 53578e1 Compare August 12, 2026 15:09
Comment thread v1/ir/ir.go Outdated
Comment thread v1/ir/ir.go

// SetLocation sets the Location for a given Stmt.
func (l *Location) SetLocation(index, row, col int, file, text string) {
func (l *Location) SetLocation(index, row, col int, file string, text []byte) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Switching text to []byte changes what Pretty prints, pretty.go dumps the struct with %+v, and Go renders a []byte as numbers rather than text.

A String() method on ir.Location would fix it without giving up the []byte change, since %+v picks it up automatically.

This mean End() will work after unmarshalling too.

Signed-off-by: Charlie Egan <charlie_egan@apple.com>
@charlieegan3
charlieegan3 force-pushed the end-locations-ir-plans branch 3 times, most recently from 1e437da to c06efb8 Compare August 13, 2026 10:29
This is tricky as the type is embedded and so we need to have a custom
type to handle the printing of text rather than registering String on
Location itself.

Signed-off-by: Charlie Egan <charlie_egan@apple.com>
@charlieegan3
charlieegan3 force-pushed the end-locations-ir-plans branch from c06efb8 to fe0cd8c Compare August 13, 2026 13:34
@charlieegan3
charlieegan3 merged commit 48bb309 into open-policy-agent:main Aug 13, 2026
43 checks passed
@charlieegan3

Copy link
Copy Markdown
Contributor Author

Thanks!

@charlieegan3
charlieegan3 deleted the end-locations-ir-plans branch August 13, 2026 14:38
@github-actions

Copy link
Copy Markdown
Benchmark Comparison (`d00b87c6372bb1b165ac60be2466e15db39eb832` vs `48bb309719d039dd725d0c4ba2614e784ec28596`)
benchmark delta
BuildNakedRefIndex/100 +4.73%
ObjectGet/lookup_in_empty_object +12.50%
ObjectGet/existing_float_number_key_as_int +4.26%
ObjectGet/existing_int_number_key_as_float +3.75%
ObjectFind/5_5 -1.33%
ObjectInsert/new_key -1.46%
LazyObjectFind/500_500 +0.24%
SetIntersection/50 -2.77%
SetIntersection/500 -3.08%
SetIntersectionDifferentSize/5000 -1.35%
SetUnion/5 -1.39%
SetUnion/50 -1.29%
SetMembership/5 +2.12%
ObjectString/50/String() -3.78%
ObjectString/50/json.Marshal -1.56%
ObjectString/500/String() -4.31%
ObjectString/500/json.Marshal -0.67%
ObjectStringInterfaces/50/String() -3.62%
ObjectStringInterfaces/500/String() -2.97%
ObjectStringInterfaces/5000/String() -2.07%
ObjectConstruction/shuffled_keys/5 -5.91%
ObjectConstruction/shuffled_keys/50 -6.81%
ObjectConstruction/shuffled_keys/500 -6.91%
ObjectConstruction/shuffled_keys/5000 -5.47%
ObjectConstruction/shuffled_keys/50000 -5.70%
ObjectConstruction/shuffled_keys/500000 -10.29%
ObjectConstruction/increasing_keys/5000 -4.18%
ObjectConstruction/increasing_keys/50000 -3.73%
IsVarCompatibleString/__really_long_variable_name_1234567890 -21.65%
IsVarCompatibleString/ello -17.45%
IsVarCompatibleString/h_llo -11.05%
IsVarCompatibleString/hello -20.97%
IsVarCompatibleString/incompatible_last_char! -36.36%
RefString/dot_builtin +6.77%
EscapeTemplateStringStringPart/_*_100 +4.06%
EscapeTemplateStringStringPart/{_*_100 +4.55%
CountUnescapedLeftCurly/_*_100 +0.44%
CountUnescapedLeftCurly/{_*_100 +2.89%
InterningAccessValue/object_value +2.77%
FromBuiltinNames/three_parts +6.98%
NoNodeTypeAllocatesOnAppend/head -3.72%
NoNodeTypeAllocatesOnAppend/head_assign -5.34%
NoNodeTypeAllocatesOnAppend/head_with_key -5.30%
NoNodeTypeAllocatesOnAppend/ref_head_with_value -3.50%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/string -7.90%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/string_with_newlines -8.61%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/array_comprehension_nested_infix_operators -1.87%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/array_comprehension_nested_infix_with_function_call -2.78%

This comment was automatically generated by the benchmarks workflow.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants