Skip to content

Fix compact_script fusing adjacent operators into an invalid token (< - -> <-)#1106

Open
yuvalrakavy wants to merge 2 commits into
rhaiscript:mainfrom
yuvalrakavy:fix-compact-script-operator-adjacency
Open

Fix compact_script fusing adjacent operators into an invalid token (< - -> <-)#1106
yuvalrakavy wants to merge 2 commits into
rhaiscript:mainfrom
yuvalrakavy:fix-compact-script-operator-adjacency

Conversation

@yuvalrakavy

@yuvalrakavy yuvalrakavy commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem

Engine::compact_script's doc comment promises output "semantically identical to the input script, except smaller in size." That breaks for operator adjacency:

let engine = rhai::Engine::new();
let compacted = engine.compact_script("let y = x < -1;").unwrap();
// compacted == "let y=x<-1;"
engine.compile(&compacted).unwrap();
// -> Parse error: '<-' is not a valid symbol. This is not Go! Should it be '<='?

Removing the space in x < -1 yields x<-1; the lexer then reads <- as an (invalid) operator, so the compacted script fails to compile.

Cause

In TokenIterator::next, the compressed-buffer builder inserts a separating space only between two identifier characters (to avoid merging let x into letx). There is no equivalent guard for operator characters, so < followed by - (two separate tokens) is emitted as <-.

Fix

Also insert a separating space when the previous and current characters are both operator-lead characters (= ! < > & | ^ + - * / % ~ . : ?). Brackets and delimiters are not in that set, so ordinary compaction (let x=1;) is unchanged; single tokens such as ?. / ?? / ?[ are emitted whole, so they are unaffected. It also prevents accidentally forming reserved tokens such as ->, ++ and --.

Test

tests/tokens.rs::test_compact_script_operator_adjacency round-trips several operator + negative literal scripts through compact_script and asserts the result recompiles (and that <- is never produced). The test fails on main and passes with this change; the full test suite is green.


Note — a second, unrelated commit is included (Fix no_std_test panic handler for current nightly). Current nightly makes #[lang = "panic_impl"] on a function a hard error, which was failing the NoStdBuild job on this (and any current) PR — not related to the tokenizer change. #[panic_handler] already installs that lang item, so the explicit #[lang = "panic_impl"] (and the now-unused lang_items feature) are removed. Happy to split it into its own PR if you'd prefer to keep this one focused.

`Engine::compact_script`'s whitespace removal only inserted a separating
space between two identifier characters (so `let x` doesn't become `letx`).
It had no equivalent guard for operator characters, so two adjacent operator
tokens could fuse into a different token: `x < -1` compacted to `x<-1`, and
the lexer then reads `<-` as an invalid operator ("This is not Go!"), so the
compacted script no longer compiles — breaking compact_script's documented
"semantically identical to the input" contract.

Also insert a separating space when the previous and current characters are
both operator-lead characters. Brackets and delimiters are unaffected, so
ordinary compaction (`let x=1;`) is unchanged.

Adds a regression test.
@yuvalrakavy yuvalrakavy force-pushed the fix-compact-script-operator-adjacency branch from 7cac2b2 to af59b33 Compare July 13, 2026 15:33
`#[lang = "panic_impl"]` on a function is now a hard error on nightly
("attribute cannot be used on functions / can only be applied to foreign
functions"). `#[panic_handler]` already installs the `panic_impl` lang item,
so the explicit `#[lang = "panic_impl"]` is redundant. Remove it (and the
now-unused `lang_items` feature) so `NoStdBuild` builds on current nightly.
@schungx

schungx commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Would it be better to actually just merge the two tokens and check, via Token::lookup_symbol_from_syntax, whether it is actually a valid token by itself.

This way we don't have to hardcode anything in.

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