I have questions/suggestions WRT Jonas' SVN r42342:
https://github.com/graemeg/freepascal/commit/40d31e34116efffa239b6dc94373fcfccedfa646
1) Why was `typesym.owner` used instead of `owner`? Beside efficiency, this
hurts understandability: is it possible for a typedef to reside in a symtable
separate from typesym?
2) Why exactly was `get_top_level_symtable` replaced with `get_unit_symtable`? Despite
different implementations, they appear to be functionally equivalent. Conceptually, the
replacement does make sense; but the commit claims to "fix tobjectdef.vmt_def to
search in the correct symtable", yet does not mention a bug report / test case.
3) Streamline the logic: invoke `Find` only once.
4) Drop `tdef.get_top_level_symtable`. It was only used by tobjectdef.vmt_def
before r42342.
Attached are the patches "Amend" (for 1-3) and "Drop" (for 4).
Two currently commented out ICE checks in "Amend" need to be dropped. I had
them tested against the FPC/RTL/test-suite/LCL/LazIDE, and those two were not triggered.
Could (2) be specific to the LLVM back-end?
(Also, I have dropped the comment about the tabstractrecordsymtable typecast; I
reckon it is rather obvious, especially now, having it reside right under the
'[ObjectSymtable,recordsymtable]' condition.)
--
βþ
# HG changeset patch
# User Blaise.ru
TODO: WTF r42342
diff -r 0b5141d52cf0 -r b7add2e1e427 symdef.pas
--- a/symdef.pas
+++ b/symdef.pas
@@ -7815,14 +7815,15 @@
function tobjectdef.vmt_def: trecorddef;
var
+ where: tsymtable;
vmttypesym: tsymentry;
begin
- if not(typesym.owner.symtabletype in [ObjectSymtable,recordsymtable])
then
- vmttypesym:=typesym.owner.Find('vmtdef$'+mangledparaname)
- else
- { Use common parent of trecordsymtable and tobjectsymtable
- to avoid invalid typecast error when compiled with -CR option }
-
vmttypesym:=tabstractrecordsymtable(typesym.owner).get_unit_symtable.Find('vmtdef$'+mangledparaname);
+ where:=owner;
+ //IF WHERE <> typesym.owner THEN INTERNALERROR(2020);
+ if where.symtabletype in [ObjectSymtable,recordsymtable] then
+ where:=tabstractrecordsymtable(where).get_unit_symtable;
+ //IF WHERE <> get_top_level_symtable THEN INTERNALERROR(2022);
+ vmttypesym:=where.Find('vmtdef$'+mangledparaname);
if not assigned(vmttypesym) or
(vmttypesym.typ<>symconst.typesym) or
(ttypesym(vmttypesym).typedef.typ<>recorddef) then
# HG changeset patch
# User Blaise.ru
DROP tdef.get_top_level_symtable
diff -r b7add2e1e427 -r 16be4eecb14e symtype.pas
--- a/symtype.pas
+++ b/symtype.pas
@@ -97,7 +97,6 @@
procedure ChangeOwner(st:TSymtable);
function getreusablesymtab: tsymtable;
procedure register_created_object_type;virtual;
- function get_top_level_symtable: tsymtable;
{ only valid for registered defs and defs for which a unique id string
has been requested; otherwise, first call register_def }
function deflist_index: longint;
@@ -442,15 +441,6 @@
end;
- function tdef.get_top_level_symtable: tsymtable;
- begin
- result:=owner;
- while assigned(result) and
- assigned(result.defowner) do
- result:=tdef(result.defowner).owner;
- end;
-
-
function tdef.deflist_index: longint;
begin
if defid<defid_not_registered then
_______________________________________________
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
