feat(semantic): reject variables that shadow a type name (#102)
A variable declaration sharing its identifier with any visible type name compiled without complaint — e.g. an Iface interface and an iface variable in the same program (Pascal is case-insensitive, so these are one identifier). The variable silently shadows the type, which is confusing and almost always a mistake. Blaise already rejected var-vs-var, var-vs-const and type-vs-type clashes; var-vs-type slipped through because type decls are registered in the block's enclosing scope (so they outlive the block scope) while var decls are registered one scope deeper, hiding the type from FTable.Define's duplicate check. AnalyseVarDecls now looks up the name in the type namespace (FTable.FindType, which honours uses-chain visibility) and rejects any match. The rule is stricter than FPC mode objfpc, which permits shadowing a built-in or outer-scope type (var Integer: Int64 compiles in FPC). Blaise rejects the whole class — same-block, outer-scope, imported, or built-in — to eliminate the confusion rather than carry FPC's footgun. Recorded in docs/language-rationale.adoc with the alternatives considered. FIXPOINT_OK + NATIVE_FIXPOINT_OK; full suite OK (3190 tests, 5 new).
This commit is contained in:
parent
bf63d7419f
commit
4e90165b73
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue