pretyping: reject vector shifts with conflicting sign prefix and suffix - #1532
Open
MavenRain wants to merge 1 commit into
Open
pretyping: reject vector shifts with conflicting sign prefix and suffix#1532MavenRain wants to merge 1 commit into
MavenRain wants to merge 1 commit into
Conversation
The scalar path rejects `>>s32u` with "invalid operator >>s 32u". The vector path accepted `>>s4u32` and silently used the suffix sign, emitting a logical shift where the prefix asks for an arithmetic one. Apply the same consistency check in the CVS arm of op_info. Agreeing signs (e.g. `>>s8s32`) stay accepted, matching the scalar rule. Fixes jasmin-lang#1441 Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
MavenRain
force-pushed
the
1441-vector-shift-sign-consistency
branch
from
August 8, 2026 15:19
d808bc2 to
e7fd960
Compare
vbgl
reviewed
Aug 18, 2026
vbgl
left a comment
vbgl
left a comment
Member
There was a problem hiding this comment.
Thanks for your contribution. I’ve left a minor comment about your implementation.
| | CVS(vs,sg,ve) -> | ||
| let s1 = tt_sign sg in | ||
| let s = | ||
| Option.map_default |
Member
There was a problem hiding this comment.
Using map here looks strange. Using Option.may might be clearer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The scalar shift
>>s32uis rejected withinvalid operator >>s 32u. The vector form>>s4u32was accepted. The compiler ignored thesprefix and used only the sign in the suffix. It emitted a logical (unsigned) shift for an operator that asks for an arithmetic (signed) shift.This change adds the same consistency check to the vector path. The
CVSarm ofop_infoincompiler/src/pretyping.mlnow compares the prefix sign with the sign in the vector suffix. If they conflict, the type checker rejects the program with the sameinvalid operatorerror as the scalar path. If they agree, or if there is no prefix, the behavior does not change. Only>>(ShR) is affected. All other operators either take no sign prefix or reject vector suffixes before this point.Notes on scope:
>>s4s32and>>u4u64(agreeing signs) stay accepted. This matches the scalar rule, where>>s32sis accepted. Two such lines are added to the positive testcompiler/tests/success/x86-64/vector_expression.jazzto pin this.Tests: a new negative test
compiler/tests/fail/typing/x86-64/vector_shift_sign_mismatch.jazzchecks that>>s4u32is rejected, with its expected diagnostic incompiler/tests/negative.expected.Fixes #1441
Checklist
changesif the PR is a user-visible changecompiler/testsif it makes sense, especially if it is a bug fixAUTHORSI used Claude to help prepare this change. I reviewed and tested it.