feat: extend LiteralDict lowering with runtime support (constant-path tests) by m-akhil-reddy · Pull Request #213 · arxlang/irx

m-akhil-reddy · 2026-03-09T06:23:33Z

Notes

This repository uses an AI bot for reviews. Keep your PR in Draft while
you work. When you’re ready for a review, change the status to Ready for
review to trigger a new review round. If you make additional changes and
don’t want to trigger the bot, switch the PR back to Draft.
AI-bot comments may not always be accurate. Please review them critically and
share your feedback; it helps us improve the tool.
Avoid changing code that is unrelated to your proposal. Keep your PR as short
as possible to increase the chances of a timely review. Large PRs may not be
reviewed and may be closed.
Don’t add unnecessary comments. Your code should be readable and
self-documenting
(guidance).
Don’t change core features without prior discussion with the community. Use
our Discord to discuss ideas, blockers, or issues
(https://discord.gg/Nu4MdGj9jB).
Do not include secrets (API keys, tokens, passwords), credentials, or
sensitive data/PII in code, configs, logs, screenshots, or commit history. If
something leaks, rotate the credentials immediately, invalidate the old key,
and note it in the PR so maintainers can assist.
Do not commit large binaries or generated artifacts. If large datasets are
needed for tests, prefer small fixtures or programmatic downloads declared in
makim.yaml (e.g., a task that fetches data at test time). If a large binary is
unavoidable, discuss first and consider Git LFS.

Pull Request description

#30 #211

How to test these changes

...

Pull Request checklists

This PR is a:

bug-fix
new feature
maintenance

About this PR:

it includes tests.
the tests are executed on CI.
the tests generate log file(s) (path).
pre-commit hooks were executed locally.
this PR requires a project documentation update.

Author's checklist:

I have reviewed the changes and it contains no misspelling.
The code is well commented, especially in the parts that contain more
complexity.
New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

m-akhil-reddy · 2026-03-09T12:45:48Z

Hi @yuvimittal , can you review the PR and suggest any changes if needed .

This PR extends the existing LiteralDict lowering by adding support for the runtime (non-constant) dictionary path.

Approach

The implementation first checks whether all keys and values lower to LLVM Constant values.

If all elements are constants, the dictionary is lowered using the existing constant fast-path to a constant LLVM array [N x {K, V}].

If non-constant values appear, the visitor switches to runtime lowering:

The key/value LLVM types are inferred from the first pair.
Homogeneity of types is enforced for runtime dictionaries.
A struct type {K, V} is created to represent each dictionary entry.
An array [N x {K, V}] is allocated in the function entry block.
Each key/value pair is stored using GEP indexing and store instructions.
The resulting pointer to the allocated array is pushed onto the result stack.

Safety Handling

Runtime lowering requires a valid LLVM function context.
If the visitor is executed outside a function (as in unit tests), the implementation falls back to constant lowering to avoid invalid IR generation.

Tests

Added a test covering the runtime-lowering path behavior.
Since tests run outside a function context, the test verifies the fallback to constant lowering.

Renamed test from 'test_literal_dict_runtime_lowering_fallback' to 'test_literal_dict_const_lowering'.

yuvimittal · 2026-03-10T10:42:45Z

tests/test_literal_dict.py

+
+
+@pytest.mark.parametrize("builder_class", [LLVMLiteIR])
+def test_literal_dict_const_lowering(


i have changed the test name as test never enters runtime lowering.Because
LiteralInt32 -> ir.Constant so this condition is always true: `all_constants = True
meaning all tests actually executes constant fast path , not run time lowering

You're right that this path always hits the constant fast-path since LiteralInt32 lowers to ir.Constant. I updated the test name to test_literal_dict_const_lowering.
thanks.

yuvimittal · 2026-03-10T10:43:51Z

tests/test_literal_dict.py

+    result = visitor.result_stack.pop()
+
+    # When no function context exists, runtime lowering falls back to constant
+    assert isinstance(result, ir.Constant)


better test assert result.type.count == 1
this verifies dictionary layout

I added assert result.type.count == 1 so the test also verifies the dictionary layout.
thank.

yuvimittal · 2026-03-10T10:45:02Z

tests/test_literal_dict.py

+
+
+@pytest.mark.parametrize("builder_class", [LLVMLiteIR])
+def test_literal_dict_const_lowering(


You should ideally test a non-constant key/value. uh for example
x = 1 { x: 2 }
like the code functionality

Thanks for the suggestions, @yuvimittal

Currently the tests run outside a function context, so runtime lowering cannot safely emit IR (no builder block). Because of that, the implementation falls back to constant lowering in tests.

I agree that ideally we should test a true non-constant case (e.g., using a variable like {x: 2} inside a function). I can add a function-based test for that in a follow-up PR's.

@m-akhil-reddy , but the PR title and description doesnt align with existing tests

@yuvimittal ,
thanks for pointing that out.

I'll update the PR title/description to clarify that the implementation includes the runtime lowering path, while the current tests cover the constant fast-path due to running outside a function context.

I have updated the PR title and description to clarify that the implementation includes the runtime lowering path while the current tests cover the constant fast-path due to running outside a function context.

yuvimittal · 2026-03-10T10:46:01Z

src/irx/builders/llvmliteir.py

+        builder.position_at_end(current_bb)
+
+        # i32 type used for GEP indexing
+        i32 = ir.IntType(32)


LLVM actually expects target pointer-sized integers for GEP indices on some targets. so the safer approach would be i32 = self._llvm.INT32_TYPE

I replaced ir.IntType(32) with self._llvm.INT32_TYPE to follow the project's LLVM type definitions.
thanks.

A

m-akhil-reddy · 2026-03-10T14:52:23Z

@yuvimittal , i have made the changes according to your suggestion.

m-akhil-reddy · 2026-03-11T06:00:11Z

This PR extends LiteralDict lowering with runtime construction support when keys/values are non-constant.
Current tests run outside a function context, so they cover the constant fast-path; a function-based runtime test can be added in a follow-up.

yuvimittal · 2026-03-11T07:55:30Z

This PR extends LiteralDict lowering with runtime construction support when keys/values are non-constant. Current tests run outside a function context, so they cover the constant fast-path; a function-based runtime test can be added in a follow-up.

uh i am not sure, how do we verify the current implenmentation

m-akhil-reddy · 2026-03-11T09:33:17Z

Thanks for the feedback.

I updated the tests to better reflect the current implementation. Since LiteralDict requires both keys and values to be Literal nodes, using identifiers like {1: x} is rejected during AST construction. Because of this constraint, the runtime path cannot be directly triggered in a literal-only test.

The test now verifies the fallback behavior when no function context exists (i.e., builder.block is None), ensuring that the implementation safely falls back to constant lowering. All CI checks are passing after the update.

Please let me know if you would prefer a different testing approach.
@yuvimittal

m-akhil-reddy added 9 commits March 9, 2026 11:52

feat: extend LiteralDict lowering with runtime construction

d2a49a0

fix: correct runtime LiteralDict test assertion

32bafd7

fix: adjust runtime LiteralDict test to avoid builder context issue

1b141af

fix: guard runtime LiteralDict lowering without function context

62e342c

fix: fallback to constant lowering when runtime IR context unavailable

da25c60

fix: adjust runtime LiteralDict test for constant fallback

41fd52e

fix: guard runtime lowering when builder block is missing

02da402

fix: avoid alloca in LiteralDict runtime test

e8e0ecd

refactor: improve runtime LiteralDict lowering guard and comments

75894c1

Rename test for literal dict lowering

89f75cc

Renamed test from 'test_literal_dict_runtime_lowering_fallback' to 'test_literal_dict_const_lowering'.

yuvimittal requested changes Mar 10, 2026

View reviewed changes

fix: address review comments for LiteralDict lowering

60683bf

A

m-akhil-reddy requested a review from yuvimittal March 10, 2026 16:54

m-akhil-reddy changed the title ~~feat: extend LiteralDict lowering with runtime construction~~ feat: extend LiteralDict lowering with runtime construction support Mar 11, 2026

m-akhil-reddy changed the title ~~feat: extend LiteralDict lowering with runtime construction support~~ feat: extend LiteralDict lowering with runtime support (constant-path tests) Mar 11, 2026

test: adjust LiteralDict runtime test to validate fallback path

6093d2b



		@pytest.mark.parametrize("builder_class", [LLVMLiteIR])
		def test_literal_dict_const_lowering(

Conversation

m-akhil-reddy commented Mar 9, 2026

Notes

Pull Request description

How to test these changes

Pull Request checklists

Additional information

Reviewer's checklist

Uh oh!

m-akhil-reddy commented Mar 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-akhil-reddy Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-akhil-reddy commented Mar 10, 2026

Uh oh!

m-akhil-reddy commented Mar 11, 2026

Uh oh!

yuvimittal commented Mar 11, 2026

Uh oh!

m-akhil-reddy commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

m-akhil-reddy Mar 10, 2026 •

edited

Loading