Restore drop-in compatibility: make 'Release' a contextual keyword

docs/notes/legacy-release-keyword.md

@@ -0,0 +1,110 @@
+# Working note — `Release` as a contextual keyword (legacy-identifier Phase 2)
+
+Branch: `align/legacy-release-keyword-identifier`
+Type: Feature / Alignment (drop-in source compatibility, INTENT #1)
+
+## Goal (restated)
+
+BlitzForge globally reserves `Release` in the tokenizer, so a classic Blitz3D
+program using `release` as an ordinary variable fails to compile. `Release` is a
+*fork addition* (stock Blitz3D had no such keyword — release was `x = Null` /
+`Delete`), so `release`-as-identifier is a genuine fork-introduced regression
+against INTENT #1. PR #77 made `Test` contextual the same way and explicitly named
+`Release` as the deferred Phase 2; this delivers it.
+
+Scope is `Release` ONLY. `Object` (the other deferred token) is a *base Blitz3D*
+keyword (`Object.Type(handle)`), so `object`-as-identifier would not have compiled
+in stock Blitz3D either — a different, riskier problem class, deferred for separate
+investigation.
+
+## Root cause / current behavior (verified)
+
+- `toker.cpp:81` reserves `Release` → `RELEASE` (unconditional).
+- `parser.cpp:518` statement `case RELEASE` passes through to `parseExpr` (so the
+  keyword statement form `Release.Type x` is handled by the expression parser).
+- `parser.cpp:1032` expression primary `case RELEASE` handles `Release[.]Type
+  operand` → `ReleaseNode` (semant casts operand to BBPointer, calls `__bbRelease`).
+- `parser.cpp:70` `parseIdent` rejects everything but `IDENT`/`TEST`, so `release`
+  can't be a declared name.
+- Repro: `samples/Blitz 2D Samples/spindisc.bb:82` `release=1` →
+  "Expecting identifier near: = Got: =". (Confirmed `release` is its only blocker.)
+
+The keyword form ALWAYS has an identifier immediately after `Release` (a type name,
+with an optional `.`): `Release.Type operand` or `Release Type operand`. So:
+**`Release` is the keyword iff the next token is `.` or IDENT; otherwise it is an
+ordinary identifier** (`release=`, `release\f`, `release(`, `release[`, `If
+release=1`, ...). This is the exact disambiguator, mirroring #77's `Test` lookahead.
+
+## Approach (specific, in order) — mirrors PR #77
+
+1. `parser.cpp:70` (`parseIdent`): accept `RELEASE` as an identifier (add
+   `&& toker->curr()!=RELEASE`), so `release` works as a parameter / Local / Global /
+   Field name and type-instance var. Update the comment.
+2. `parser.cpp:162-163` (statement dispatch): after the `TEST` disambiguator add
+   `if( stmtTok==RELEASE && !(toker->lookAhead(1)=='.' || toker->lookAhead(1)==IDENT) ) stmtTok=IDENT;`
+   — routing `release = ...` / `release(...)` / `release\f` statements through the
+   IDENT statement path (which reads `toker->text()` directly, so it handles the
+   `release` spelling without further change). `Release.Type x` keeps `stmtTok==RELEASE`.
+3. `parser.cpp:1032` expression primary: make the keyword handling conditional and
+   fall through to the IDENT primary when it's an identifier use. Relocate the
+   `case RELEASE:` body to sit immediately above `case TEST: case IDENT:` (line 1135)
+   so the identifier path is a clean fall-through:
+   ```cpp
+   case RELEASE:
+       if( toker->lookAhead(1)=='.' || toker->lookAhead(1)==IDENT ){
+           if( toker->next()=='.' ) toker->next();
+           t=parseIdent();
+           result=parseUniExpr( false );
+           result=d_new ReleaseNode( result,t );
+           break;
+       }
+       // else fall through: 'release' is an ordinary identifier
+   case TEST:
+   case IDENT:
+       ...
+   ```
+4. Remove the old standalone `case RELEASE:` at 1032.
+
+No tokenizer change; `Release.Type` grammar and `ReleaseNode` semantics untouched.
+
+## Files touched
+
+- `src/blitzrc/compiler/parser.cpp` — 3 sites (parseIdent, statement dispatch,
+  expression primary).
+- `tests/LegacyKeywordIdentifierTest.bb` — `release` as identifier in every position
+  + a POSITIVE `Release.Type` keyword test proving the operator still works.
+- `scripts/corpus_compile_allowlist.txt` — remove the `spindisc.bb` line.
+- `docs/notes/legacy-release-keyword.md` — this note.
+
+## Migration / rollback
+
+Additive compatibility — every newly-accepted `release`-as-identifier form was a
+hard compile error before, so no program can depend on the old behavior. Rollback =
+revert the parser hunks + restore the allowlist line.
+
+## Trade-offs considered and rejected
+
+- **Include `Object` too** (the recon majority): rejected this iteration — `Object`
+  is a base-Blitz3D keyword, not a fork addition, so it's a different problem class
+  needing its own investigation. Keeping scope to the clear-cut fork regression.
+- **Tokenizer-level soft keyword / general framework**: rejected — over-engineering;
+  the per-site `.`/IDENT lookahead is the proven #77 pattern.
+
+## Acceptance criteria
+
+- `printf 'release=1\nIf release=1 Then release=0\nEnd\n' | blitzcc -c +q` exits 0.
+- `release` works as Local, assignment target, expression operand, field-typed var,
+  and function parameter (pinned by `tests/LegacyKeywordIdentifierTest.bb`).
+- The `Release.Type obj` GC operator still parses AND works (positive test:
+  RefCount drops after release) — keyword not broken.
+- `spindisc.bb` compiles under `blitzcc -c +q`; its allowlist line is removed and
+  `scripts/sweep_corpus.sh` stays green (one fewer known-fail, 0 new-fail).
+- Full `blitzcc -t tests/*.bb` green.
+
+## Deferred follow-ups
+
+- `Object` contextual-identifier investigation (base-keyword problem class).
+- `Recast`/`Reference` are also expression-position keywords with the same grammar
+  shape — no corpus collision, but the same pattern now applies if ever needed.
+- Modern-features docs as a CI-gated executable contract + BBList docs (strong
+  next-iteration candidate; best anti-pile-on pick with compounding value).

scripts/corpus_compile_allowlist.txt

@@ -20,10 +20,11 @@
 # that classic Blitz3D allowed as identifiers. The HIGH-priority contextual-keyword
 # fix would re-enable these. (`object` is a legacy Blitz3D keyword, not a fork
 # addition, but the program still uses it as an identifier.)
-# NOTE: the four `test`-collision files (fileio/global/mirror/deathisland) were
-# removed once `Test` became a contextual keyword (PR: align/legacy-test-keyword-identifier);
-# they now compile. `release` and `object` remain (see below).
-samples/Blitz 2D Samples/spindisc.bb      # 'release=1' — Release is still a hard keyword (Phase 2: expression-position contextual)
+# NOTE: the `test`-collision files (fileio/global/mirror/deathisland) were removed
+# once `Test` became a contextual keyword (PR: align/legacy-test-keyword-identifier),
+# and `spindisc.bb` once `Release` became contextual (PR: align/legacy-release-keyword-identifier).
+# `object` remains: it is a *base* Blitz3D keyword (Object.Type(handle)), not a fork
+# addition, so it is a different problem class -- deferred for separate investigation.
 samples/Blitz 3D Samples/RobHutchinson/CraftFlare/CraftFlare.bb  # 'object' identifier (legacy Blitz3D keyword, not a fork addition)
 
 # --- Class B: disabled / unregistered builtin --------------------------------

src/blitzrc/compiler/parser.cpp

@@ -63,11 +63,12 @@ void Parser::exp( const string &s ){
 }
 
 string Parser::parseIdent(){
-	// 'Test' is a contextual keyword: it is only the test-block opener at
-	// statement start (handled in parseStmtSeq). In every identifier position --
-	// parameter names, Local/Global/Field declarations, type tags, labels -- it is
-	// an ordinary identifier, so legacy programs using `test` as a name compile.
-	if( toker->curr()!=IDENT && toker->curr()!=TEST ) exp( "identifier near: " + toker->text() );
+	// 'Test' and 'Release' are contextual keywords: 'Test' is only the test-block
+	// opener at statement start, and 'Release' is only the GC release operator
+	// (`Release.Type <obj>`). In every identifier position -- parameter names,
+	// Local/Global/Field declarations, type tags, labels -- both are ordinary
+	// identifiers, so legacy programs using `test`/`release` as names compile.
+	if( toker->curr()!=IDENT && toker->curr()!=TEST && toker->curr()!=RELEASE ) exp( "identifier near: " + toker->text() );
 	string t=toker->text();
 	if (t.find(":") != std::string::npos) ex( "Identifiers cannot include the : character");
 	toker->next();
@@ -161,6 +162,11 @@ void Parser::parseStmtSeq( StmtSeqNode *stmts,int scope ){
 		// source compatibility). Route those uses through the IDENT statement path.
 		int stmtTok = toker->curr();
 		if( stmtTok==TEST && !(scope==STMTS_PROG && toker->lookAhead(1)==IDENT) ) stmtTok=IDENT;
+		// 'Release' is contextual the same way: the GC release operator is always
+		// `Release.Type ...` or `Release Type ...` (a '.' or identifier follows).
+		// A statement like `release = ...`, `release(...)`, or `release\field` is an
+		// ordinary identifier use -- route it through the IDENT statement path.
+		if( stmtTok==RELEASE && !(toker->lookAhead(1)=='.' || toker->lookAhead(1)==IDENT) ) stmtTok=IDENT;
 
 		switch( stmtTok ){
 		case INCLUDE:
@@ -1029,12 +1035,6 @@ ExprNode *Parser::parseUniExpr( bool opt ){
 		result=parseUniExpr( false );
 		result=d_new RecastNode( result,t );
 		break;
-	case RELEASE:
-		if( toker->next()=='.' ) toker->next();
-		t=parseIdent();
-		result=parseUniExpr( false );
-		result=d_new ReleaseNode( result,t );
-		break;
 	case REFERENCE:
 		toker->next();
 		result=parseUniExpr( false );
@@ -1132,6 +1132,19 @@ ExprNode *Parser::parsePrimary( bool opt ){
 	case BBFALSE:
 		result=d_new IntConstNode( 0 );
 		toker->next();break;
+	case RELEASE:
+		// 'Release' is a contextual keyword: `Release[.]Type <operand>` is the GC
+		// release operator, but a bare `release` (followed by '=', '\', '(', '[', an
+		// operator, ...) is an ordinary identifier. Disambiguate on the next token:
+		// the operator form always has a '.' or a type identifier immediately after.
+		if( toker->lookAhead(1)=='.' || toker->lookAhead(1)==IDENT ){
+			if( toker->next()=='.' ) toker->next();
+			t=parseIdent();
+			result=parseUniExpr( false );
+			result=d_new ReleaseNode( result,t );
+			break;
+		}
+		// else fall through: treat 'release' as an ordinary identifier
 	case TEST:	// contextual keyword: usable as an identifier in expressions
 	case IDENT:
 		ident=toker->text();

tests/LegacyKeywordIdentifierTest.bb

@@ -15,6 +15,25 @@ Function withTestParam( test )
 	Return test + 1
 End Function
 
+; --- 'Release' as a legacy identifier ---
+; 'Release' is BlitzForge's GC release operator (`Release.Type <obj>`), but legacy
+; Blitz3D programs use `release` as an ordinary variable/parameter name (e.g. the
+; spindisc.bb `If release = 1` case). It must work as an identifier everywhere it
+; is not the release operator, while `Release.Type <obj>` still parses and works.
+
+Type ReleaseHolder
+	Field value
+End Type
+
+Type ReleaseTarget
+	Field var
+End Type
+
+; 'release' as a function parameter name, used as an expression operand in the body.
+Function withReleaseParam( release )
+	Return release + 1
+End Function
+
 Test legacyTestAsLocalAndAssignment()
 	; Local declaration named `test`, statement-start assignment, and use as an
 	; expression operand -- all positions that previously tokenized as the TEST
@@ -34,3 +53,43 @@ Test legacyTestFieldAccess()
 	test\value = 7
 	Assert( test\value = 7 )
 End Test
+
+Test legacyReleaseAsLocalAndAssignment()
+	; Local declaration named `release`, statement-start assignment, and use as an
+	; expression operand -- all positions that previously tokenized as the RELEASE
+	; keyword and failed.
+	Local release = 41
+	release = release + 1
+	Assert( release = 42 )
+End Test
+
+Test legacyReleaseInIfCondition()
+	; The spindisc.bb real-world case: `release` as an operand in an If condition.
+	Local release = 1
+	Assert( release = 1 )
+	If release = 1
+		release = release + 1
+	EndIf
+	Assert( release = 2 )
+End Test
+
+Test legacyReleaseAsParameter()
+	Assert( withReleaseParam( 41 ) = 42 )
+End Test
+
+Test legacyReleaseFieldAccess()
+	; `release` as a type-instance variable plus field access on it.
+	Local release.ReleaseHolder = New ReleaseHolder()
+	release\value = 7
+	Assert( release\value = 7 )
+End Test
+
+Test releaseOperatorStillWorks()
+	; Positive: the GC operator `Release.Type <obj>` must still parse and run.
+	; Mirrors GarbageCollectionTest's testRef/testRel: New gives RefCount 1, and
+	; after Release the object is gone so RefCount(First ReleaseTarget) = 0.
+	Local obj.ReleaseTarget = New ReleaseTarget()
+	Assert( RefCount(First ReleaseTarget) = 1 )
+	Release.ReleaseTarget obj
+	Assert( RefCount(First ReleaseTarget) = 0 )
+End Test