diff --git a/compiler/src/main/pascal/uSemantic.pas b/compiler/src/main/pascal/uSemantic.pas index 80b62cd..5f8b3e2 100644 --- a/compiler/src/main/pascal/uSemantic.pas +++ b/compiler/src/main/pascal/uSemantic.pas @@ -5202,6 +5202,20 @@ begin SemanticError( Format('Duplicate identifier ''%s''', [VarName]), Decl.Line, Decl.Col); + { A variable may not share a name with any visible type — built-in, + same-block, outer-scope, or imported (issue #102). Pascal is + case-insensitive, so `type Iface` and `var iface` are the same + identifier; allowing both silently shadows the type and is a + common source of confusion. Stricter than FPC mode objfpc, which + permits shadowing built-in/outer types; Blaise rejects the whole + class. (Same-scope type-vs-var is otherwise invisible to + FTable.Define because types live in the enclosing scope while + var decls are registered one scope deeper.) } + if FTable.FindType(VarName) <> nil then + SemanticError( + Format('Duplicate identifier ''%s'' — a type with this name is ' + + 'already visible', [VarName]), + Decl.Line, Decl.Col); if Decl.IsThreadVar and not Decl.IsGlobal then SemanticError('threadvar is only allowed at unit or program scope', Decl.Line, Decl.Col); diff --git a/compiler/src/test/pascal/cp.test.semantic.pas b/compiler/src/test/pascal/cp.test.semantic.pas index 52762d1..bf64c49 100644 --- a/compiler/src/test/pascal/cp.test.semantic.pas +++ b/compiler/src/test/pascal/cp.test.semantic.pas @@ -27,6 +27,15 @@ type procedure TestVarDecl_MultiName_BothRegistered; procedure TestVarDecl_UnknownType_RaisesError; procedure TestVarDecl_Duplicate_RaisesError; + { A variable may not share a name with ANY visible type (issue #102): + same-block, outer-scope, imported, or built-in. Stricter than FPC + mode objfpc (which allows shadowing built-in/outer types); Blaise + rejects the whole class to eliminate the confusion. Case-insensitive. } + procedure TestVarDecl_DuplicatesType_RaisesError; + procedure TestVarDecl_DuplicatesType_DifferentCase_RaisesError; + procedure TestVarDecl_DuplicatesInterfaceType_RaisesError; + procedure TestVarDecl_ShadowsBuiltinType_RaisesError; + procedure TestVarDecl_ShadowsOuterScopeType_RaisesError; { Const then var with same name in same block is a duplicate } procedure TestVarDecl_DuplicatesConst_RaisesError; { Const redeclared with same name in same block is a duplicate } @@ -185,6 +194,46 @@ begin 'program P; var x: Integer; x: string; begin end.'); end; +procedure TSemanticTests.TestVarDecl_DuplicatesType_RaisesError; +begin + AnalyseExpectError( + 'program P; type TFoo = record X: Integer; end; var TFoo: Integer; begin end.'); +end; + +procedure TSemanticTests.TestVarDecl_DuplicatesType_DifferentCase_RaisesError; +begin + { The exact issue #102 reproduction: type Iface, var iface — same + identifier, different case. Pascal is case-insensitive. } + AnalyseExpectError( + 'program P; type Iface = interface procedure Test; end; ' + + 'var iface: Iface; begin end.'); +end; + +procedure TSemanticTests.TestVarDecl_DuplicatesInterfaceType_RaisesError; +begin + AnalyseExpectError( + 'program P; type IThing = interface procedure Go; end; ' + + 'var IThing: Integer; begin end.'); +end; + +procedure TSemanticTests.TestVarDecl_ShadowsBuiltinType_RaisesError; +begin + { Unlike FPC, Blaise rejects a var that shadows a built-in type name — + `var Integer` is almost always a mistake and shadowing it silently + redefines the type for the rest of the scope. } + AnalyseExpectError('program P; var Integer: Int64; begin end.'); +end; + +procedure TSemanticTests.TestVarDecl_ShadowsOuterScopeType_RaisesError; +begin + { A type declared in an outer scope may not be shadowed by a local var — + Blaise rejects the whole var/type name-clash class regardless of scope. } + AnalyseExpectError( + 'program P; type TFoo = record X: Integer; end; ' + + 'procedure Q; var TFoo: Integer; begin end; ' + + 'begin Q(); end.'); +end; + procedure TSemanticTests.TestVarDecl_DuplicatesConst_RaisesError; begin AnalyseExpectError( diff --git a/docs/language-rationale.adoc b/docs/language-rationale.adoc index 19fc853..2630830 100644 --- a/docs/language-rationale.adoc +++ b/docs/language-rationale.adoc @@ -4214,3 +4214,66 @@ homogeneous literal (`[1, 2, 3]`) and the empty literal (`[]`) bind too. array/record.* Rejected — it would make `Format`-style APIs verbose and un-idiomatic, and the construct is compiler-bounded and tagged, so it does not carry the open-ended risks of the omitted three. + +== Variable Names May Not Shadow Type Names + +=== Decision + +A variable declaration may not share its identifier with any type name that is +visible at the point of declaration — whether the type is built-in +(`Integer`, `string`), declared in the same block, declared in an enclosing +scope, or imported from another unit. Pascal is case-insensitive, so the +shadowing is detected regardless of letter case: + +[source,pascal] +---- +type + Iface = interface + procedure Test; + end; +var + iface: Iface; // ERROR: 'iface' and the type 'Iface' are the same identifier +---- + +The diagnostic is `Duplicate identifier 'X' — a type with this name is already +visible`, reported at the variable's declaration site. + +This rule is stricter than FPC `mode objfpc`, which reports a duplicate only +when the variable clashes with a type declared in the *same* scope, and +silently permits a variable to shadow a built-in or outer-scope type +(`var Integer: Int64` compiles in FPC). + +=== Rationale + +A variable that bears the same name as a type is almost always a mistake, and +when it is not, it is a readability trap: every subsequent use of the +identifier in that scope means the variable, so the type becomes unreferenceable +by its own name and later declarations like `x: Iface` resolve in surprising +ways. Issue #102 reported exactly this confusion — a program declaring both an +`Iface` interface and an `iface` variable compiled without complaint on both +backends. + +Blaise already rejects variable-vs-variable, variable-vs-constant, and +type-vs-type clashes in the same scope. The variable-vs-type case slipped +through only because of an implementation detail: type declarations are +registered in the block's enclosing scope so they remain visible after the +block scope is popped, while variable declarations are registered one scope +deeper. The duplicate check at definition time therefore never saw the type. +Closing the gap with an explicit type-name lookup restores the symmetry users +expect: no two declarations in overlapping scopes may claim the same +identifier. + +=== Alternatives considered + +* *Match FPC exactly (same-scope clash only; built-in/outer shadowing + allowed).* Rejected. It fixes the reported case but keeps the underlying + footgun: `var Integer: Int64` would still compile and silently rebind the + type name for the rest of the scope. Blaise favours a clean rule over FPC + bug-for-bug compatibility (cf. the deliberate omission of `Char`, the + uniform 0-based indexing, and the required `overload` keyword). +* *Warn instead of error.* Rejected. The construct has no legitimate use that + a rename would not serve more clearly, and a warning that is routinely + ignored would leave the confusion in place. +* *Restrict the rule to reference types (classes, interfaces) only.* Rejected + as arbitrary — the confusion is identical for records, enums, aliases, and + built-ins, and a name-based rule is simpler to explain than a kind-based one.