BugFix Challenge

Started by Zlatko Vid, April 02, 2026, 12:00:10 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Zlatko Vid

WOW Theo
this looks really well from what i can figure  ;)
but i like this:
QuoteNo structs, no pointers, no preprocessor, no floating-point codegen, and no optimizer — just a straight path from sour
  ce → tokens → AST → IR → x64 machine code → COFF object → PE .exe
.

ps.Does this Kimi can fix tiny interpreter ?
I will give you code if is not problem
Z.ai stuck on functions and code is in OxygenBasic

all best
  •  

Zlatko Vid

also QWEN 3.5 failed at function too
  •  

Theo Gottwald

#2
@Zlatko Vid
Yes, we can try that.
Send me the code in a zip-file and send me a textfile with instructions what you want then i can fix that for you.
If its in Oxygen Basic, i need additionally a textfile with the syntax of all used commands.
Without this current AI's may struggle to fix the problem.
You have my mail?

Zlatko Vid

Hi Theo
there is no need for special things
in attachment is zipped SAI3

Just throw to AI this code it is very simple
inside code is test program
  •  

Zlatko Vid

Ok tell to AI to fix function execution.
  •  

Theo Gottwald

#5
i told you i need a textfile with the exact description of the problem, or what should be changed and a textfile with the Oxygen commands, as you did not provide any of these i can not guarantee success.
So lets just wait and see ...

I'll search the web for Oxygen Basic syntax, focusing on variable declarations, line continuation, select/case statements, array usage, type definitions, and how to handle pointer selection with strings.
I've confirmed the archive contains a single source file, SAI3.o2bas, and the next pass is splitting definite Oxygen Basic compile/syntax faults from interpreter logic defects. Early hot spots are function-header parsing, keyword/string dispatch, loop bookkeeping and re-entry/block skipping, token/index advancement, and debug print traces that would corrupt interpreter output.
I've narrowed the main compile-risk cluster to the tokenizer dispatch and declaration forms: select identPtr paired with string case labels, inline declarations inside case branches, PowerBASIC-style one-line statement forms, and one suspicious missing end if.

The broader pass shows most of the unusual Oxygen syntax is actually legal, including the declaration/type style and _ continuations. The real breakpoints are function-definition parsing, lost statement boundaries from discarded CR/LF, IF/ELSE/ENDIF flow and token re-entry after calls, plus a likely IF ... THEN parser-compatibility issue.
I've confirmed a key Oxygen-specific language issue: the token dispatcher uses a pointer-based select, which in O2 is only dependable with very short literals. That can misrecognize longer keywords like print, while, endif, function, and return, causing broad interpreter failure.

I've confirmed the first concrete Oxygen-specific problem: the tokenizer's SELECT/CASE string dispatch is invalid for full string matching, and any strptr-based select only safely distinguishes the first four bytes unless followed by a real string comparison.

I've confirmed a key Oxygen-specific language issue: the token dispatcher uses a pointer-based select, which in O2 is only dependable with very short literals. That can misrecognize longer keywords like print, while, endif, function, and return, causing broad interpreter failure.

Theo Gottwald

MASTER,

I reviewed `SAI3.o2bas`. The sample `add(1,2)` path cannot work as written. The core breakage is a combination of **bad function-header parsing**, **bad call/return token positioning**, and **non-local variable binding**.

## Confirmed faults in the source

### 1) Lines **372–385** — `FUNCTION` header parsing is off by one

**Problem:** after reading the function name, the code does one `GetToken()` too few for the parameter list and then stores `StartPos` before the body is positioned correctly.

**Effect:**
`function add(a,b)` ends up with `ParamCount = 0`, and `StartPos` lands inside or just after the header instead of at the first token of the body.

**Correction:**
After the function name:

* consume `(`,
* consume the first parameter token,
* parse variables until `)`,
* store the body start **after** the closing `)`,
* then skip to `ENDFUNC`.

---

### 2) Lines **263–295** and **459–492** — function-call return position is wrong

**Problem:** both call paths read **past** `)` and then save `ReturnPos`.

**Effect:**
After returning from a function, the interpreter skips the token immediately following the call.
Example: `x = add(1,2) + 3` loses the `+`.

**Correction:**
Save `ReturnPos` while the current token is still `)` and `IJ.Pos` is already positioned **after** it. Then, after return, restore `IJ.Pos = ReturnPos` and call `GetToken()` once.

---

### 3) Lines **106–117**, **279–280**, **476–477** — local/function scope is broken

**Problem:** `SetVar()` searches the whole symbol table and updates the first matching name from the top.

**Effect:**
Function parameters or assignments inside a function can overwrite globals instead of creating/updating locals.

**Correction:**
Use frame-local assignment:

* when entering a function, **push** parameter variables as new entries,
* inside a function, search/update only the current frame first,
* if not found in the current frame, create a new local instead of overwriting a global.

---

### 4) Lines **387–390** — `IF` true branch skips the first statement

**Problem:** after `Expression()` finishes, `IJ.CurrToken` is already at the first token of the true branch, but the code does another `GetToken()`.

**Effect:**
The first statement inside a true `IF` block is skipped.

**Correction:**
Remove that extra `GetToken()` from the true branch.

---

### 5) Lines **391–393** plus missing cases in `Statement()` — `ELSE` / `ENDIF` logic is incomplete

**Problem:** false `IF` calls `SkipBlock(KW_IF, KW_ENDIF)`, which jumps to `ENDIF`, not to `ELSE`. The code also has no proper runtime handling for `ELSE` and `ENDIF`.

**Effect:**

* false branch can never execute `ELSE`,
* true branch falls into `ELSE`,
* `ELSE` / `ENDIF` can leave the parser stalled.

**Correction:**
Add dedicated helpers:

* `SkipToElseOrEndif()`
* `SkipToMatchingEndif()`

And add explicit `case KW_ELSE` / `case KW_ENDIF` handlers.

---

### 6) `Statement()` has no safe fallback for several block keywords

**Problem:** `KW_RETURN`, `KW_ENDFUNC`, `KW_ELSE`, `KW_ENDIF` are not all consumed safely in the general statement dispatcher.

**Effect:**
On some paths the interpreter can sit on the same token forever.

**Correction:**
Add cases that at least consume those tokens, or route them through proper block logic.

---

### 7) Lines **323–347** — comparison parsing is wrong on the right-hand side

**Problem:** comparisons use `r = Term()`.

**Effect:**
`1 < 2 + 3` parses incorrectly because the RHS stops at `2` and leaves `+ 3` behind.

**Correction:**
Split expression parsing into:

* `Factor`
* `Term`
* `Additive`
* `Expression` (comparisons over additive expressions)

So the RHS of `<`, `>`, `=` is an **additive expression**, not just a term.

---

### 8) Lines **240–308** — unary `+` / `-` is incomplete

**Problem:** there is no real unary stage in the grammar.

**Effect:**
`2 * -3`, `4 / -2`, similar forms parse incorrectly.

**Correction:**
Handle unary operators in `Factor()` or add a dedicated `Unary()` stage before `Factor()`.

---

### 9) Lines **194–204**, **240–308**, **368–370** — strings are tokenized but not actually supported

**Problem:** tokenizer recognizes `TK_STRING`, but `Factor()` does not evaluate strings, and `PRINT` always calls `Expression()` and formats numerically.

**Effect:**
`print "hello"` does not behave correctly.

**Correction:**
At minimum:

* in `KW_PRINT`, detect `TK_STRING` and print it directly,
* or add proper string-expression support.

---

### 10) Lines **275–277** and **472–474** — call frames do not preserve loop stacks

**Problem:** call frames save `OldVarCount`, but not `ForPtr` / `WhilePtr`.

**Effect:**
Loops inside functions can corrupt caller loop state.

**Correction:**
Extend `CallStackInfo` with `OldForPtr` and `OldWhilePtr`, then restore them on return.

---

### 11) `LastReturnValue` is global and not reset per call

**Problem:** return value storage is reused across calls.

**Effect:**
A function without an explicit `RETURN` can leak the previous function's return value.

**Correction:**
Reset `IJ.LastReturnValue = 0` before each function invocation, or store a per-frame return value.

---

### 12) Unknown function calls are not consumed safely

**Problem:** when the parser sees `name(...)` and `name` is not found in `Funcs[]`, it does not cleanly consume the call syntax.

**Effect:**
The parser leaves `(` and/or argument tokens in the stream and drifts out of sync.

**Correction:**
If the function is unknown:

* either raise an error,
* or explicitly skip tokens until the matching `)`.

---

## Secondary defects

### 13) No bounds checks on symbol/function/loop/call stacks

`VarCount`, `FuncCount`, `ForPtr`, `WhilePtr`, `CStackPtr`, arg count, and param count can all write past their arrays.

**Correction:**
Guard every increment before storing.

---

### 14) Debug output is still embedded in the parser

Examples:

* line **250**: `print "VAR-NAME: " + name + crlf`
* line **253**: `print "FN-NAME: " + name + crlf`
* line **264**: `print "ARGS:" + crlf`
* line **373**: `print "FUNCTION:" + crlf`

**Effect:**
Interpreter output is polluted even when logic is otherwise correct.

**Correction:**
Remove them or wrap them in a debug flag.

---

## Things I am **not** flagging as errors

Line **5** (`$ filename "SAI3.exe"`) is valid Oxygen Basic usage, and your 1-based array walks are not inherently wrong because Oxygen's default array index base is `1`. ([Oxygen Basic][1])

Line **178** (`INT identPtr AT STRPTR ident`) is also a valid O2-style aliasing/cast idiom. Using `select` over aliased bytes/pointers is a known pattern. The important caveat is that direct string `select` is **not** supported, and the integer/pointer trick compares only the **first four characters**. With your current keyword set that is acceptable, but it is prefix-based, not full-string matching. ([Oxygen Basic][2])

## Corrected source

I prepared a best-effort patched version that addresses the major failures above:

[SAI3_fixed.o2bas](sandbox:/mnt/data/SAI3_fixed.o2bas)

[1]: https://o2.allbasic.info/forum/index.php?topic=1351.0 "Different approach for Commandline arguments"
[2]: https://o2.allbasic.info/forum/index.php?topic=1753.15 "Documentation"

Theo Gottwald

**MASTER,** I checked `SAI3.o2bas` against Oxygen Basic usage and syntax references. The declaration surface is mostly **valid Oxygen**: `DIM ... AS`, `TYPE/END TYPE`, bracketed arrays, and `_` line continuation all match working O2 examples, and O2 arrays are **1-based by default** unless `indexbase 0` is set. So the file's real failures are mostly **interpreter defects**, not basic declaration syntax. ([Oxygen Basic][1])

The one syntax area that **is** wrong in principle is your keyword dispatch strategy. Oxygen `SELECT/CASE` does **not** do full string comparison. The documented workaround is pointer-based selection such as `int *p = strptr func`, but that only tests the **first 4 characters**, so it is unsafe for full keywords like `function`, `return`, `endif`, etc. Also, `at strptr(...)` is overlay syntax in Oxygen, not the same thing as the pointer-select idiom. For keyword recognition, `if / elseif / endif` is the correct tool here. ([Oxygen Basic][2])

## ✅ What is *not* an Oxygen syntax error

These parts are fine and should **not** be treated as compile faults:

* your `TYPE ... END TYPE` blocks
* `DIM x AS ...` declarations
* fixed-size arrays like `Vars[100]`
* the multiline source string using `_`
* 1-based loops over your arrays and stacks

So your earlier suspicion about declarations, line continuation, arrays, and type definitions was mostly a false lead. ([Oxygen Basic][1])

---

# 🔧 Definite faults in `SAI3.o2bas`

## 1) Keyword dispatch is broken

**Lines 178–192**

Current code:

```o2bas
INT identPtr at strptr ident
select identPtr
    case "print": ...
    ...
end select
```

### Why it is wrong

* `SELECT/CASE` is not reliable for full string matching in Oxygen.
* The pointer trick only compares the first 4 characters.
* `function`, `endif`, `return`, `while`, `print` etc. are therefore unsafe in this form.

### Correction

Replace the whole block with full-string comparisons:

```o2bas
IJ.CurrToken.tkType = TK_VARIABLE
IJ.CurrToken.SubType = 0

if ident = "print" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_PRINT
elseif ident = "if" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_IF
elseif ident = "else" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_ELSE
elseif ident = "endif" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_ENDIF
elseif ident = "while" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_WHILE
elseif ident = "wend" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_WEND
elseif ident = "for" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_FOR
elseif ident = "to" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_TO
elseif ident = "next" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_NEXT
elseif ident = "function" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_FUNCTION
elseif ident = "endfunc" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_ENDFUNC
elseif ident = "return" then
    IJ.CurrToken.tkType = TK_KEYWORD : IJ.CurrToken.SubType = KW_RETURN
end if
```

Also remove the stray trailing `:` at line **191**.

---

## 2) Function-definition parsing is wrong

**Lines 372–385**

This block never parses parameter names correctly.

### What happens now

After `function add(a,b)`:

* `GetToken()` reads the function name ✅
* next `GetToken()` reads `(` ✅
* then your `while IJ.CurrToken.tkType = TK_VARIABLE` never runs, because current token is `(` ❌
* then `GetToken()` moves to `a`
* then `StartPos = IJ.Pos` is set too early, while still inside the header ❌

### Effect

* `ParamCount` stays zero
* parameters are never recorded
* `StartPos` points inside the header instead of the body
* a function call re-enters at the wrong location

### Correction

```o2bas
case KW_FUNCTION
    IJ.FuncCount += 1
    dim f as integer
    f = IJ.FuncCount

    GetToken()                          ' function name
    IJ.Funcs[f].Name = IJ.CurrToken.Text

    GetToken()                          ' should be "("
    if IJ.CurrToken.tkType = TK_LPAREN then GetToken()

    IJ.Funcs[f].ParamCount = 0
    while IJ.CurrToken.tkType = TK_VARIABLE
        IJ.Funcs[f].ParamCount += 1
        IJ.Funcs[f].ParamNames[IJ.Funcs[f].ParamCount] = IJ.CurrToken.Text
        GetToken()
        if IJ.CurrToken.tkType = TK_COMMA then GetToken()
    wend

    if IJ.CurrToken.tkType = TK_RPAREN then
        IJ.Funcs[f].StartPos = IJ.Pos   ' position after ')'
    end if

    SkipBlock(KW_FUNCTION, KW_ENDFUNC)
    GetToken()
```

---

## 3) Function calls skip the token after `)`

**Two places**

* **Factor(): lines 268–295**
* **Statement(): lines 444–492**

This is one of the worst bugs in the file.

### What happens now

After the argument loop, `CurrToken` is already `)` and `IJ.Pos` is already positioned **after** it.

Then you do another `GetToken()`:

```o2bas
GetToken() ' consume ')'
```

That does **not** consume `)` — it consumes the **next token after `)`**.

Then `ReturnPos` is saved too late.
When the function returns and you do `GetToken()` again, the interpreter skips one more token.

### Effect

Anything after a function call is misparsed or skipped.

### Correction

Delete that extra `GetToken()` before saving `ReturnPos`.

Correct pattern:

```o2bas
' after arg parsing loop:
' CurrToken = TK_RPAREN
' IJ.Pos is already after ")"

IJ.CStackPtr += 1
IJ.CStack[IJ.CStackPtr].ReturnPos = IJ.Pos
IJ.CStack[IJ.CStackPtr].OldVarCount = IJ.VarCount

' jump into function
IJ.Pos = IJ.Funcs[fIdx].StartPos
GetToken()

' ...
' on return:
IJ.VarCount = IJ.CStack[IJ.CStackPtr].OldVarCount
IJ.Pos = IJ.CStack[IJ.CStackPtr].ReturnPos
IJ.CStackPtr -= 1
GetToken()
```

So:

* remove line **274**
* remove line **469**

---

## 4) `IF / ELSE / ENDIF` flow is broken

**Lines 387–393**, plus missing handlers later

### Current problems

#### a) True branch skips its first statement

You do:

```o2bas
GetToken()
if Expression() > 0
    GetToken()
else
    ...
end if
```

But `Expression()` already leaves `CurrToken` at the first token of the body.
That extra `GetToken()` skips the first statement.

#### b) False branch skips all the way to `ENDIF`

You use:

```o2bas
SkipBlock(KW_IF, KW_ENDIF)
```

That ignores `ELSE`, so the else-part can never execute correctly.

#### c) `ELSE` and `ENDIF` are not handled in `Statement()`

There are no `case KW_ELSE` / `case KW_ENDIF` handlers, so execution can stall or drift when those tokens are reached.

### Correction

At minimum:

* remove the extra `GetToken()` in the true branch
* implement a `SkipIfFalse()` that stops at `ELSE` **or** `ENDIF` at the right nesting depth
* add handlers for `KW_ELSE` and `KW_ENDIF`

Skeleton:

```o2bas
case KW_IF
    GetToken()
    if Expression() > 0 then
        ' do nothing here
        ' CurrToken is already the first token of the true branch
    else
        SkipIfFalse()
    end if

case KW_ELSE
    SkipBlock(KW_IF, KW_ENDIF)
    GetToken()

case KW_ENDIF
    GetToken()
```

---

## 5) Debug `print` traces corrupt interpreter output

**Lines 250, 253, 264, 373**

These are not diagnostics anymore; they are output corruption:

```o2bas
print "VAR-NAME: " + name + crlf
print "FN-NAME: " + name + crlf
print "ARGS:" + crlf
print "FUNCTION:" + crlf
```

### Effect

Your interpreter prints internal debug traces into the program's visible output stream.

### Correction

Remove them, or guard them behind a debug flag:

```o2bas
const DEBUGMODE = 0
if DEBUGMODE then print ...
```

---

## 6) `FOR` parsing/bookkeeping is fragile and semantically wrong

**Lines 395–417**

### Problems

* line **396** ends with a stray `:`
* no validation for variable / `=` / `TO`
* no zero-iteration handling when `startVal > endVal`
* no `STEP` support
* loop body start relies on token-position side effects

### Concrete bug

For:

```basic
for i = 5 to 3
```

your code still enters the loop once.

### Correction

Parse the header explicitly and skip the block immediately when the initial range is empty:

```o2bas
case KW_FOR
    GetToken()
    dim v as string
    v = IJ.CurrToken.Text

    GetToken()
    if IJ.CurrToken.tkType <> TK_EQUAL then exit sub
    GetToken()

    dim startVal as float
    startVal = Expression()

    if IJ.CurrToken.tkType <> TK_KEYWORD or IJ.CurrToken.SubType <> KW_TO then exit sub
    GetToken()

    dim endVal as float
    endVal = Expression()

    SetVar(v, startVal)

    if startVal > endVal then
        SkipBlock(KW_FOR, KW_NEXT)
        GetToken()
    else
        IJ.ForPtr += 1
        IJ.ForStack[IJ.ForPtr].StartPos = IJ.Pos - len(IJ.CurrToken.Text)
        IJ.ForStack[IJ.ForPtr].VarName = v
        IJ.ForStack[IJ.ForPtr].EndLimit = endVal
    end if
```

---

## 7) Comparison parsing is wrong

**Lines 336–347**

Current pattern:

```o2bas
if IJ.CurrToken.tkType = TK_LESS or ...
    int op = IJ.CurrToken.tkType
    GetToken()
    float r = Term()
```

### Bug

The right-hand side of a comparison only parses a `Term()`, not a full additive expression.

So:

```basic
1 < 2 + 3
```

is parsed incorrectly.

### Correction

Split expression parsing into two layers:

* arithmetic expression (`+` / `-`)
* comparison expression on top of that

Do **not** parse comparison RHS with `Term()` only.

---

## 8) String literals are tokenized but not evaluated

**Tokenizer:** lines **194–204**
**Factor():** lines **303–307**

You create `TK_STRING`, but `Factor()` has no `case TK_STRING`.

### Effect

This breaks statements like:

```basic
print "hello"
```

### Correction

Add:

```o2bas
case TK_STRING
    Factor = 0
    ' or redesign value system to support strings separately
    GetToken()
```

Realistically, your evaluator is currently **numeric-only**, so `print` needs either:

* a separate string-print path, or
* a variant/string-capable value model.

---

## 9) `GetToken()` should reset token state more aggressively

Several branches set only part of `CurrToken`.

### Risk

`SubType` or previous token metadata can leak into the next token.

### Correction

At the top of `GetToken()` reset all token fields:

```o2bas
IJ.CurrToken.tkType = TK_EOF
IJ.CurrToken.SubType = 0
IJ.CurrToken.Value = 0
IJ.CurrToken.Text = ""
```

Then fill what applies.

---

## 10) `InitInterpreter()` is never called

You define:

```o2bas
sub InitInterpreter()
```

but `main()` never calls it.

### Effect

You are relying on zero-initialized globals by accident.

### Correction

At the top of `main()`:

```o2bas
InitInterpreter()
```

---

## 11) Variable scoping around function calls is unsafe

`SetVar()` searches backward and updates an existing variable name if found.

### Hidden bug

If a function parameter or local has the same name as a global, assignment can overwrite the global instead of creating an isolated local binding.

### Effect

Function-local state can leak into globals.

### Correction

You need call-frame-local variables, not just `OldVarCount`.
At minimum, when binding function parameters, **append** them as new variables for the current frame instead of updating pre-existing names.

---

# ⚠️ Syntax-risk items I would clean immediately

These are not the main failures, but I would still remove them:

* **line 191**: trailing `:` after `KW_RETURN`
* **line 396**: trailing `:` after chained `GetToken()` calls

They are pointless and parser-hostile.

---

# Priority order to repair

1. **Replace keyword `select` with `if/elseif`**
2. **Fix function header parsing**
3. **Fix function-call return position handling**
4. **Repair `IF/ELSE/ENDIF` skip logic**
5. **Remove debug prints**
6. **Fix `FOR` empty-range behavior**
7. **Add proper string handling**
8. **Refactor expression parser into arithmetic vs comparison**

---

# Bottom line

`SAI3.o2bas` is **mostly legal Oxygen Basic**, but it is **not** a correct interpreter yet. The declaration/type/array/continuation syntax is largely fine. The real defects are:

* wrong keyword dispatch mechanism
* broken function header parsing
* off-by-one token advancement after function calls
* broken `IF/ELSE/ENDIF` control flow
* debug output contaminating program output
* incomplete `FOR` semantics
* missing string evaluation path

That is the precise split between **compile/syntax risk** and **interpreter logic defects**.

[1]: https://o2.allbasic.info/forum/index.php?topic=1309.0 "Building DLL's"
[2]: https://o2.allbasic.info/forum/index.php?topic=1499.0 "Why SELECT not respond on string variable?"

Theo Gottwald

MASTER,

I unpacked `SAI3.o2bas` and did a docs-checked static audit. ✅

## Hard split: Oxygen syntax vs. interpreter defects

I do **not** see a clear Oxygen-host-language syntax failure in the areas you asked me to verify. The declaration forms, multiline declarations, array forms, `type ... end type`, overlays via `at strptr(...)`, `$ filename "..."`, `_` line continuation, `if/elseif/else/end if`, `while/wend`, and numeric `select/case` are all legitimate Oxygen constructs. ([GitHub][1])

The one syntax-adjacent trap is your keyword scanner. Oxygen `select/case` does **not** directly compare string variables, and the `int *p = strptr func` technique only tests the **first 4 characters**. So `select identPtr` is not an exact keyword matcher; it is a 4-byte prefix filter. ([Oxygen Basic][2])

## Errors found, with corrections

### 1) Keyword dispatch is prefix-based, not exact

**Lines 173–192**

Current code:

```basic
INT identPtr at strptr ident
select identPtr
    case "print"
    ...
    case "function"
```

**Problem:** this can misclassify identifiers by their first 4 bytes. Examples: `printf`, `functionality`, `returnValue`, `whileCount`.
**Correction:** replace the whole `select identPtr` block with exact full-string checks:

```basic
if ident = "print" then
    ...
elseif ident = "if" then
    ...
end if
```

or use `select asc ident` only as a first-letter filter, then do full `if ident = ...` checks inside each branch.

---

### 2) Function header parsing is broken

**Lines 372–385**

Current flow:

```basic
GetToken() : IJ.Funcs[f].Name = IJ.CurrToken.Text
GetToken() ' '('
IJ.Funcs[f].ParamCount = 0
while IJ.CurrToken.tkType = TK_VARIABLE
```

**Problem:** after the second `GetToken()`, `CurrToken` is `(`, not the first parameter. So the parameter loop never starts correctly, `ParamCount` stays wrong, and `StartPos` is captured in the wrong place.

**Correction:** after confirming `(`, call `GetToken()` again to move to the first parameter or `)`, then parse params until `TK_RPAREN`. Save `StartPos` only after the header is finished.

---

### 3) Function call return-position bookkeeping skips the next token

**Lines 268–295** and **463–492**

**Problem:** after parsing arguments, the code does an extra `GetToken()` while sitting on `TK_RPAREN`, then stores `IJ.Pos` as `ReturnPos`. That consumes the token immediately after `)` before the call frame is stored. On return, another `GetToken()` advances again. Net effect: the caller loses the token after the call.

**Correction:** when the arg loop ends on `TK_RPAREN`, **do not** advance before saving return state. Save:

```basic
IJ.CStack[...].ReturnPos = IJ.Pos
```

while `CurrToken` is still `TK_RPAREN`, then restore `IJ.Pos` after the function and call `GetToken()` exactly once.

---

### 4) Unknown function calls leave the parser out of sync

**Lines 297–299** and **494–495**

**Problem:** if `name(` is seen but `name` is not in `IJ.Funcs`, the argument list is not consumed. Parsing state is left hanging on `(` or inside the arguments.

**Correction:** either raise an error, or at minimum consume tokens through the matching `)` before continuing.

---

### 5) Comparison precedence is wrong

**Lines 323–349**

Current comparison RHS:

```basic
r = Term()
```

**Problem:** `1 < 2 + 3` parses incorrectly because the right side is only a `Term()`, not a full additive expression.

**Correction:** split arithmetic into:

* `Factor()`
* `Term()`
* `AddExpr()`
* `Expression()` for comparisons

Then compare `AddExpr()` against `AddExpr()`.

---

### 6) `IF` true branch skips the first statement

**Lines 387–393**

Current code:

```basic
GetToken()
if Expression() > 0
  GetToken()
else
  ...
end if
```

**Problem:** after `Expression()`, `CurrToken` is already positioned at the first token of the THEN block. The extra `GetToken()` skips it.

**Correction:** remove that `GetToken()`.

---

### 7) False `IF` handling cannot execute `ELSE`, and `SkipBlock()` is structurally flawed

**Lines 352–362** and **392**

There are **two** issues here:

1. `SkipBlock(KW_IF, KW_ENDIF)` skips to `ENDIF`, so a false condition never lands on `ELSE`.
2. `SkipBlock()` checks nesting **after** calling `GetToken()`, which means it can miss a nested opener when that opener is the first skipped token.

**Correction:**

* create a dedicated `SkipToElseOrEndif()` helper for false IFs
* rewrite `SkipBlock()` so it inspects the **current** token before advancing

---

### 8) `ELSE`, `ENDIF`, and other unhandled keywords can stall the interpreter

**Lines 365–442**

**Problem:** inside `case TK_KEYWORD`, there is no handler for `KW_ELSE`, `KW_ENDIF`, or a default keyword case. That means a taken IF branch can hit `ELSE` or `ENDIF` and stop making progress, causing an infinite loop.

**Correction:** add:

```basic
case KW_ELSE
    SkipBlock(KW_IF, KW_ENDIF) : GetToken()

case KW_ENDIF
    GetToken()

case else
    GetToken()
```

---

### 9) Function parameters overwrite globals instead of shadowing them

**Lines 279–280** and **476–477**

Current code uses:

```basic
SetVar(IJ.Funcs[fIdx].ParamNames[idx], args[idx])
```

**Problem:** `SetVar()` searches backward and updates an existing variable if the name already exists. So a function parameter named `i` can overwrite a global `i`. Restoring `VarCount` later does **not** restore the old global value.

**Correction:** on function entry, append params as fresh symbol-table slots so they shadow globals naturally; then restore `VarCount` on return.

---

### 10) `PRINT` ignores `TK_STRING` completely

**Lines 194–204** and **368–370**

**Problem:** the lexer produces `TK_STRING`, but `KW_PRINT` always does:

```basic
print str(Expression()) + crlf
```

So `print "hello"` is not handled correctly.

**Correction:** special-case string tokens:

```basic
GetToken()
if IJ.CurrToken.tkType = TK_STRING then
    print IJ.CurrToken.Text + crlf
    GetToken()
else
    print str(Expression()) + crlf
end if
```

---

### 11) Debug trace prints corrupt interpreter output

**Lines 250, 253, 264, 373, 511–520**

Examples:

```basic
print "VAR-NAME: " + name + crlf
print "FN-NAME: " + name + crlf
print "FUNCTION:" + crlf
print "Source Code:" + crlf
```

**Problem:** these mix diagnostics into the interpreted program's output stream.

**Correction:** remove them, or guard them behind a `DEBUG` flag.

---

### 12) Unknown characters are silently converted to EOF

**Lines 228–230**

Current code:

```basic
else
    IJ.CurrToken.tkType = TK_EOF
end if
```

**Problem:** any unsupported character becomes end-of-file, hiding lexer errors and truncating execution.

**Correction:** emit an error token, or at least skip/report the bad character explicitly.

---

### 13) Secondary weaknesses

These are not the core breakages, but they are still defects/limitations:

* `FOR` does not validate the presence of `TO`
* no `STEP` support
* number scanning accepts multiple dots
* unterminated strings are not reported cleanly
* no comment syntax in the interpreted language itself

## What is actually valid Oxygen syntax in your file

These are **not** the source-language errors:

* `$ filename "SAI3.exe"`
* `_` continuation in the `IJ.Source = ...` construction
* `type ... end type`
* array declarations like `string ParamNames[9]`
* `select/case` itself
* `at strptr(...)`-style overlay usage as a language feature

Those forms are documented Oxygen syntax. The bug is **how** your interpreter uses them, especially the pointer-based keyword selection trick. ([GitHub][1])

## Highest-priority fixes

1. Replace the keyword scanner with exact string comparisons.
2. Repair function header parsing.
3. Fix call/return token-position handling.
4. Fix IF/ELSE skipping and add `ELSE` / `ENDIF` handling.
5. Remove debug prints.
6. Bind params as locals, not via `SetVar()`.

## Files

* [Patched best-effort source](sandbox:/mnt/data/SAI3_fixed.o2bas)
* [Audit notes](sandbox:/mnt/data/SAI3_review.md)
* [Unified diff](sandbox:/mnt/data/SAI3_changes.diff)

The patched file already includes the major corrections above: exact keyword matching, function-header repair, proper return-position handling, addi

tive-vs-comparison parsing split, `ELSE`/`ENDIF` handling, local param shadowing, string `PRINT`, unknown-call resync, and removal of trace output.

[1]: https://github.com/Charles-Pegge/OxygenBasic/blob/master/Docs/Variables.md "OxygenBasic/Docs/Variables.md at master · Charles-Pegge/OxygenBasic · GitHub"
[2]: https://o2.allbasic.info/forum/index.php?topic=1499.0 "Why SELECT not respond on string variable?"


Theo Gottwald

#9
@Zlatko Vid Above is a collection of results I got. Try to get something out of this.
The rule "What you put in is what you get out" is still true.
So you did not specify exactly what you want and do get exactly that.

I have one ki still working, will post that below on result.

Theo Gottwald

MASTER,

I reviewed the supplied `SAI3.o2bas` against the Oxygen Basic sources and isolated the actual failure chain. The **main breakage** in your sample is this:

## Core root cause

**`function add(a,b)` is parsed incorrectly, and function calls resume at the wrong place.** ⚙️

### 1) Function definition parser is off by one token

**Original lines:** `372–385`

After reading the function name, the code reads `(` into `CurrToken`, but then immediately starts scanning parameters without consuming it.
That means:

* `ParamCount` stays `0`
* `ParamNames[]` are never filled
* `StartPos` is saved from **inside the parameter list**, not from the function body

So `add(1,2)` executes from the wrong source position and with no bound parameters.

### 2) Function call return position is saved too late

**Original lines:** `268–276` and `463–474`

After parsing the last argument, the code does `GetToken()` before storing `ReturnPos`. Since `GetToken()` already advances `IJ.Pos` past the token it reads, the interpreter resumes **after the next token**.

That breaks things like:

* `add(1,2)+4` → the `+` gets skipped
* `add(1,2)` followed by another statement → the first token of the next statement gets skipped

---

## Other confirmed bugs/problems

### 3) `IF` true branch skips the first statement

**Original lines:** `387–390`

After `Expression()`, `CurrToken` is already at the first token of the THEN block.
The extra `GetToken()` skips it.

### 4) `ELSE` never executes

**Original lines:** `387–393`

False `IF` uses `SkipBlock(KW_IF, KW_ENDIF)`, which jumps straight to `ENDIF`.
So `ELSE` is effectively dead code.

### 5) Unhandled keywords can freeze the interpreter

**Original region:** `366–442`

There are no handlers for:

* `KW_ELSE`
* `KW_ENDIF`
* `KW_RETURN`
* `KW_ENDFUNC`
* `KW_TO`

and no keyword `case else`.

That means `Statement()` can return without advancing the token stream, which can lock execution.

### 6) Comparison precedence is wrong

**Original lines:** `335–347`

Right side of `=`, `<`, `>` is parsed with `Term()` instead of additive expression parsing.

Example:

* `1 < 2 + 3` is effectively handled as `1 < 2`

### 7) `SkipBlock()` mishandles nested blocks

**Original lines:** `352–361`

It calls `GetToken()` **before** testing the current token.
If the skipped region starts with a nested `IF`, `WHILE`, `FOR`, or `FUNCTION`, that nested opener is missed and depth tracking becomes wrong.

### 8) `FOR` loops execute once even when they should not

**Original lines:** `395–404`

`FOR i = 5 TO 3` still enters the body once.
There is no initial skip when `startVal > endVal`.

### 9) String tokens exist, but `PRINT` cannot print them

**Original lines:** tokenizer `194–204`, print handler `368–370`

`TK_STRING` is recognized, but `PRINT` always forces numeric `Expression()`.
So `print "abc"` produces the wrong result.

### 10) Unary `+` / `-` are missing

**Original parser:** `Factor()`

So `a = -1` and `print -(1+2)` do not parse correctly.

### 11) Function-local scope is leaky

**Original lines:** `106–117`, `279–280`, `476–477`

Parameters are bound via `SetVar()`, which updates an existing variable of the same name.
If a global has that name, the global gets overwritten instead of shadowed.

### 12) `LastReturnValue` can leak across calls

If a function exits without `return`, the previous function's return value is reused.

### 13) Debug prints corrupt interpreter output

**Original lines:** `250`, `253`, `264`, `373`

These diagnostic prints change the interpreter's visible output.

### 14) Unknown call syntax desynchronizes parsing

If a name is followed by `(` but no function is found, the parser does not consume the call cleanly.

### 15) No bounds checks on fixed-size tables/stacks

Possible overflow points:

* `VarNames[999]`
* `Funcs[99]`
* `ForStack[49]`
* `WhileStack[49]`
* `CStack[99]`
* `ParamNames[9]`

---

## What I prepared

### Full bug report

[SAI3_review.md](sandbox:/mnt/data/SAI3_review.md)

### Corrected source

[SAI3_fixed.o2bas](sandbox:/mnt/data/SAI3_fixed.o2bas)

### Unified diff against your original file

[SAI3_fix.diff](sandbox:/mnt/data/SAI3_fix.diff)

---

## What the patch changes

The fixed source addresses the functional defects above, especially:

* correct function header parsing
* correct call/return token restoration
* working `IF / ELSE / ENDIF`
* safer block skipping
* correct comparison precedence
* unary `+/-`
* string printing
* local parameter shadowing instead of overwriting globals
* reset of `LastReturnValue`
* removal of debug noise

For your supplied test program, the corrected logic makes the `add(1,2)` path resolve properly and the interpreter should print `3` rather than `0`.


Zlatko Vid

Thank you very much Theo on your effort with this one .
Wow i  don't expect such a large elaboration but
OK i read all text..uff
so i will try to compile...

all best
  •  

Zlatko Vid

Theo i don't know if you try to compile
i do and get exactly same output ..which is 0 and not 3
so so-called fixing is not fixing
i don't try download all 3 zip you posted at first then just latest you posted.
but i will download all of them
thanks again and don't worry
all best  ;) 
  •  

Zlatko Vid

just to mention all AI is kind of stupid for OxygenBasic especially on this

`int *p = strptr func`

and should be :

`int p = strptr func`
  •  

Zlatko Vid

#14
QuoteCareful! Only the first 4 characters are tested, as an integer

Heh i am still wondering why just first 4 chars  ::)

btw.
It is very hard to track errors in console programs
i am used to MsgBox or good old PRINT in OH simply because program stop
in execution and you can see current string,int or any other value you need.
  •