refactor[lang]: share conversion rules and bounds by banteg · Pull Request #5126 · vyperlang/vyper

banteg · 2026-06-12T20:41:31Z

What I did

Remove duplicate convert() legality checks from legacy and Venom lowering, and share numeric clamp bounds, including the negative decimal boundary fix for #5110.

Current master already validates type pairs in Convert.infer_arg_types through _convert_rules.validate_convertibility (merged in #5127). This PR uses that implementation instead of adding a second semantic matrix. It does not claim a new change to analysis-only rejection timing.

How I did it

Keep the shared semantic validator as the authority for legal source/target pairs. Remove redundant backend type restrictions, retain value-dependent bounds checks and the runtime length check for unbounded bytes, and share integer/decimal clamp calculations through _convert_bounds.py.

The conversion-matrix test checks the shared validator against the existing test oracle. Direct literal inference and full-compile behavior remain covered.

How to verify it

Conversion syntax and functional suites pass on both pipelines: 11,287 passed per backend.

The Venom unbounded bytes/string/dynamic-array runtime suite also passes: 209 passed. make lint passes.

Commit message

Conversion type-pair legality is already checked during semantic
analysis. Remove duplicate backend restrictions so the shared rules
remain authoritative, while preserving runtime bounds checks for value-
dependent conversions and unbounded bytestring lengths.

Share integer and decimal clamp windows between both lowerers, including
truncating division at the negative decimal boundary.

Description for the changelog

Fix the negative decimal conversion clamp boundary and share conversion bounds between backends.

Cute Animal Picture

the venom-direct codegen for convert() forked the legacy conversion logic and dropped two kinds of knowledge in the port: the per-target input type allowlists (GH 5019, GH 5111) and the truncating (rather than floor) division used to compute the int->decimal clamp bounds (GH 5110). the result was a cluster of accepts-invalid bugs -- programs which legacy rejects with TypeMismatch compiled to bytecode -- and an off-by-one lower clamp which admits one extra negative value whose scaled result lands below the decimal type's lower bound, producing an out-of-range decimal at runtime. this is the third round of such fixes in this file (after GH 4987 and GH 5019); patching individual instances does not converge. instead, extract the conversion matrix and the numeric clamp bounds into a new backend-neutral module, vyper/builtins/_convert_rules.py, used by both pipelines: legacy's @_input_types decorator delegates to it, and venom's lower_convert() validates against it at dispatch before lowering. the checks the venom helpers had accreted piecemeal (GH 4987) are removed in favor of the single dispatch-time validation; value-dependent checks (literal ranges, literal bytestring downcasts) stay in codegen. the module deliberately imports only from vyper.semantics and vyper.utils so that either backend (and potentially the frontend typechecker, in the future) can use it without import cycles. note: the existing conversion matrix tests did not catch these divergences because several output formats (ir_dict, metadata, abi) run the legacy codegen even under --experimental-codegen, so the test harness's multi-format compilation lets legacy validation mask venom bugs. the new regression tests compile bytecode-only.

conversion legality is a language rule, but it was only enforced at codegen time, separately by each pipeline. call the shared validate_convertibility() from Convert.infer_arg_types so illegal conversions are rejected at semantic analysis for every backend. codegen-level validation stays in place as defense-in-depth, so the legacy pipeline is undisturbed. no semantics change -- only where the error is raised. programs that compile today keep compiling; illegal conversions now surface at typechecking (same exception types) instead of codegen, including for analysis-only consumers.

banteg · 2026-06-12T21:08:49Z

note that this doesn't touch the legacy pipeline as per charles ask, so it's redundant and it meant to demonstrate the shape for where things could more logically belong. this pr assumes a buy-in and some further careful cleanup work.

Sporarum

Nice work !

To me the most important change is to move the check actually in the type checker, currently it looks like it was just moved to a shared helper
(I explain in more details in one of the comments)

banteg · 2026-06-15T16:17:10Z

Addressed in 2b27c42e4:

Moved the type-pair legality matrix into the Convert builtin itself (Convert._validate_type_pair()), so this is visibly part of builtin/typechecker validation.
Removed codegen-side conversion-pair revalidation; the shared helper is now only _convert_bounds.py for numeric clamp bounds used by both lowerers.
Tightened test_illegal_conversions_blocked to assert the exact TypeMismatch.message.
Replaced the circular frontend-vs-helper product test with explicit valid/invalid frontend cases and added an exact same-type invalid case.

Validation:

uv run --python 3.12 -m pytest tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py -k "convert_fail_at_typechecking or convert_pass or illegal_conversions_blocked" -q
uv run --python 3.12 ruff check vyper/builtins/_convert_bounds.py vyper/builtins/_convert.py vyper/builtins/functions.py vyper/codegen_venom/builtins/convert.py tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py
uv run --python 3.12 python -m py_compile vyper/builtins/_convert_bounds.py vyper/builtins/_convert.py vyper/builtins/functions.py vyper/codegen_venom/builtins/convert.py tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py

banteg · 2026-06-22T09:04:33Z

Addressed the fresh convert feedback in ba43a56. Changes: added a Convert class docstring explaining the target-to-source direction, moved exact FlagT <-> uint256 handling into the target-keyed source-type lookup, clarified that the Venom codegen path relies on typechecker rejection, and left an explicit comment at the infer_arg_types validation point. Focused validation: uv run --python 3.12 -m pytest tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py -q (1432 passed); git diff --check; uv run --python 3.12 black --check vyper/builtins/functions.py vyper/codegen_venom/builtins/convert.py.

Sporarum · 2026-06-25T09:49:44Z

Please add a test that checks that _validate_type_pair and can_convert in tests/functional/builtins/codegen/test_convert.py agree
(Note that one raises and the other returns a boolean)

banteg · 2026-06-25T13:10:48Z

Addressed in banteg/vyper@287a21415.

Changes:

added a codegen matrix test that checks Convert._validate_type_pair() agrees with can_convert() over the existing BASE_TYPES convert matrix, treating same-type/subtype cases as invalid because convert() raises InvalidType there;
replaced the remaining Bytes[32]-only dynamic-bytes source checks for address/integer/bool/decimal targets with an explicit Bytes[1] through Bytes[32] source set;
added regression coverage for accepted word-sized dynamic bytes sources and rejected Bytes[33] sources.

Validation:

uv run --python 3.12 -m pytest tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py -q (11253 passed)
uv run --python 3.12 black --check vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py
uv run --python 3.12 ruff check vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py
uv run --python 3.12 python -m py_compile vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py tests/functional/syntax/test_convert.py
git diff --check

charles-cooper · 2026-06-25T19:09:42Z

@banteg merge conflict, i'm guessing due to the new flag->bytes32 rule

# Conflicts: # vyper/builtins/_convert.py

banteg · 2026-06-25T22:14:50Z

Resolved in banteg/vyper@93d90c2f0 with a normal merge commit from current upstream master (no force push).

The conflict was the upstream feat[lang]: allow conversions from flag to bytes32 change against this PR's move from _input_types(...) to _validate_inputs in legacy _convert.py. Resolution:

kept _validate_inputs in _convert.py;
retained upstream's is_flag_type codegen branch for to_bytes_m;
added FlagT to Convert._source_types_for_target() for bytes32 targets so frontend validation accepts the new rule.

Validation:

uv run --python 3.12 -m pytest tests/functional/syntax/test_convert.py tests/functional/builtins/codegen/test_convert.py -q (11286 passed)
uv run --python 3.12 black --check vyper/builtins/_convert.py vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py tests/functional/syntax/test_convert.py
uv run --python 3.12 ruff check vyper/builtins/_convert.py vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py tests/functional/syntax/test_convert.py
uv run --python 3.12 python -m py_compile vyper/builtins/_convert.py vyper/builtins/functions.py tests/functional/builtins/codegen/test_convert.py tests/functional/syntax/test_convert.py
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93d90c2f01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

banteg · 2026-07-10T10:56:37Z

handed reins back to the bot

Keep the bool source-type tuple direct and rely on the CI compiler matrix when testing oversized dynamic bytes, avoiding redundant backend parametrization inside the test.

banteg · 2026-07-10T11:12:59Z

Addressed the fresh review feedback in 01189b8 after merging current master normally (a44cdc3): simplified the bool source tuple, restored CI-matrix backend coverage for the oversized-Bytes test, marked and documented the semantic-analysis compatibility break, and linked #5126 from #4572. Validation: focused convert suites 11,287 passed per backend; full functional syntax+builtins 13,737 passed / 16 xfailed per backend; uv run --python 3.12 make lint passed.

banteg · 2026-08-06T09:40:29Z

Remaining review feedback is addressed in 201a239ad:

Simplified the bytesM target source list so unconditional types are declared up front.
Verified the requested full-compile matrix at base bcb3daa53 and head 201a239ad: the typed-constant example raises TypeMismatch in all four master/PR × legacy/Venom lanes.
Updated the title and PR description to state that there is no full-compile behavior break; the observable change is limited to earlier rejection for analysis-only output.
Resolved the previously addressed stale threads.

Validation: 11287 passed in each legacy and Venom conversion suite; Black, isort, flake8, mypy (201 files), and git diff --check pass. Ready for re-review.

banteg · 2026-08-26T14:15:57Z

Addressed the complete August 7 changes-requested review in 87f5371. I confirmed that five source-map entries were always overridden; only BytesT and StringT remained, so the map is removed and those two cases are inline. The Venom clamp variables now use out_lo/out_hi, and bool is the final explicit integer-source branch with an unreachable panic after it. Validation: the full conversion suites passed under legacy and Venom (11,287 each), and make lint plus git diff --check passed. The PR description is refreshed and all four review threads are resolved.

chatgpt-codex-connector · 2026-09-04T20:11:52Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-16T10:50:12.081085Z	`a066e54`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

banteg · 2026-09-04T20:16:01Z

Refreshed against master in 363ae3e. Since #5127 already added _convert_rules.validate_convertibility, I removed this branch’s duplicate semantic matrix and reused that shared validator. The remaining change removes redundant backend legality checks and shares the clamp bounds, including the negative decimal boundary fix. Runtime checks for unbounded bytes are retained. Conversion suites pass on both pipelines (11,287 each), the Venom unbounded bytes/string/dynamic-array suite passes (209), and make lint passes. Updated the description to remove stale claims about master and rejection timing.

The workflows on this exact head, including the test suite, are waiting for maintainer approval (action_required). The test results above are local validation.

Sporarum

LGTM

banteg · 2026-09-16T11:16:06Z

The fuzzing-lane follow-up is now #5270 (draft, with this PR as its merge prerequisite). Enabling Venom exposes the negative integer-to-decimal boundary defect on current master. All 48 affected boundary cases pass with this fix, and all 15,750 reverting conversion cases pass on current master with #5270's harness changes and this PR's source patch applied. The new lane keeps those assertions strict.

banteg added 2 commits June 12, 2026 21:24

Sporarum requested changes Jun 15, 2026

View reviewed changes

refactor[lang]: keep convert validation in builtin

2b27c42

test[lang]: rely on CI codegen matrix for convert rejection

8fb2a80

charles-cooper mentioned this pull request Jun 19, 2026

Simplify Typer Internals #5017

Open

5 tasks

Sporarum reviewed Jun 22, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

refactor[lang]: clarify convert source validation

ba43a56

Sporarum mentioned this pull request Jun 22, 2026

fix[venom]: share convert() validation rules across pipelines #5114

Closed

Sporarum reviewed Jun 23, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

refactor[lang]: fold convert legality into source lookup

a2a8ef7

Sporarum reviewed Jun 24, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

refactor[lang]: make convert source lookup concrete

0ec16ba

fix[lang]: accept word-sized dynamic bytes in convert

287a214

refactor[lang]: use generic bytes convert sources

f0694a4

merge: master into feat/convert-frontend-validation

93d90c2

# Conflicts: # vyper/builtins/_convert.py

chatgpt-codex-connector Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/builtins/functions.py Outdated

Sporarum reviewed Jun 26, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

refactor[lang]: simplify convert integer source checks

0c3807e

Sporarum reviewed Jun 26, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

fix[lang]: bound dynamic bytes scalar conversions

97fbdb0

Sporarum reviewed Jul 10, 2026

View reviewed changes

banteg added 2 commits July 10, 2026 14:59

Merge remote-tracking branch 'origin/master' into feat/pr5126-feedback

a44cdc3

refactor[lang]: apply convert review cleanups

01189b8

Keep the bool source-type tuple direct and rely on the CI compiler matrix when testing oversized dynamic bytes, avoiding redundant backend parametrization inside the test.

banteg changed the title ~~refactor[lang]: validate convert() legality in the typechecker~~ refactor[lang]!: validate convert() legality in the typechecker Jul 10, 2026

banteg mentioned this pull request Jul 10, 2026

v0.5.0 breaking changes #4572

Open

refactor[lang]: simplify bytesM convert sources

201a239

banteg changed the title ~~refactor[lang]!: validate convert() legality in the typechecker~~ refactor[lang]: validate convert() legality in the typechecker Aug 6, 2026

Sporarum requested changes Aug 7, 2026

View reviewed changes

Comment thread vyper/builtins/functions.py Outdated

Comment thread vyper/codegen_venom/builtins/convert.py Outdated

Comment thread vyper/codegen_venom/builtins/convert.py Outdated

Comment thread vyper/codegen_venom/builtins/convert.py Outdated

refactor[lang]: simplify convert source validation

87f5371

Sporarum mentioned this pull request Aug 31, 2026

feat[venom]: wire up dyn allocations #5127

Merged

refactor[lang]: reuse shared conversion rules after master merge

363ae3e

banteg changed the title ~~refactor[lang]: validate convert() legality in the typechecker~~ refactor[lang]: share conversion rules and bounds Sep 4, 2026

Sporarum approved these changes Sep 9, 2026

View reviewed changes

Comment thread vyper/codegen_venom/builtins/convert.py Outdated

refactor[venom]: share bool and flag integer conversion handling

a066e54

This was referenced Sep 16, 2026

chore[ci]: run fuzzing with both codegen backends #5270

Draft

venom test lanes validate against legacy codegen: output formats mask venom bugs, fuzzing lane never runs venom #5116

Open

Uh oh!

Conversation

banteg commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How I did it

How to verify it

Commit message

Description for the changelog

Cute Animal Picture

Uh oh!

banteg commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sporarum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banteg commented Jun 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banteg commented Jun 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sporarum commented Jun 25, 2026

Uh oh!

banteg commented Jun 25, 2026

Uh oh!

charles-cooper commented Jun 25, 2026

Uh oh!

banteg commented Jun 25, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banteg commented Jul 10, 2026

Uh oh!

banteg commented Jul 10, 2026

Uh oh!

banteg commented Aug 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

banteg commented Aug 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

banteg commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sporarum left a comment

Choose a reason for hiding this comment

banteg commented Jun 12, 2026 •

edited

Loading

banteg commented Jun 12, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 4, 2026 •

edited

Loading

banteg commented Sep 4, 2026 •

edited

Loading