Re: [fpc-devel] Autogenerated names for internal RecordDefs
Am 01.12.2019 um 11:02 schrieb bla...@blaise.ru:
On 30.11.2019 20:47, Sven Barth via fpc-devel wrote:
Am 30.11.2019 um 13:27 schrieb bla...@blaise.ru:
{ This is a resubmission of
https://lists.freepascal.org/pipermail/fpc-devel/2019-August/041915.html
}
trecorddef.create_global_internal generates internal RecordDef names as
'$InternalRec'+tostr(current_module.deflist.count)
However, since such internal RecordDefs are not necessarily
registered afterwards (i.e. deflist.count is not going to be
bumped), such names do not contain actual DefIDs, and thus are not
unique within a module.
1) So, what is the point of that misleading numerical suffix? I
propose that it be dropped.
2) My understanding is that a name should never be autogenerated for
an internal RecordDef that is going to be registered with a DefID. I
propose that this be asserted (two variants are offered).
I've fixed this now in r43616 by using the correct way to generate a
unique ID string.
You are invoking unique_id_str before the inherited constructor sets
defid:=defid_not_registered. Before that, defid is 0, so the
aforementioned test case shows:
TRUE $vmtdef$C
FALSE $rttidef$INIT_$U_$$_C
FALSE $InternalRec
FALSE $InternalRec
FALSE $rttidef$RTTI_$U_$$_C
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
At the very least:
defid:=defid_not_registered;
name:='$InternalRec'+unique_id_str;
which I find hacky.
The LLVM backend makes use of anonymous record defs and I don't know
whether your patch would break that.
Presumably, if such duplicate internal names were breaking LLVM, it
would have been noticed already?
Maybe Jonas could weigh in?
Fixed in r43624 and r43625. I've decided to keep the unique names,
because they are useful when debugging. More useful then say the Self value.
Regards,
Sven
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Re: [fpc-devel] Autogenerated names for internal RecordDefs
On 30.11.2019 20:47, Sven Barth via fpc-devel wrote:
Am 30.11.2019 um 13:27 schrieb bla...@blaise.ru:
{ This is a resubmission of
https://lists.freepascal.org/pipermail/fpc-devel/2019-August/041915.html }
trecorddef.create_global_internal generates internal RecordDef names as
'$InternalRec'+tostr(current_module.deflist.count)
However, since such internal RecordDefs are not necessarily registered
afterwards (i.e. deflist.count is not going to be bumped), such names do not
contain actual DefIDs, and thus are not unique within a module.
1) So, what is the point of that misleading numerical suffix? I propose that it
be dropped.
2) My understanding is that a name should never be autogenerated for an
internal RecordDef that is going to be registered with a DefID. I propose that
this be asserted (two variants are offered).
I've fixed this now in r43616 by using the correct way to generate a unique ID
string.
You are invoking unique_id_str before the inherited constructor sets
defid:=defid_not_registered. Before that, defid is 0, so the aforementioned
test case shows:
TRUE$vmtdef$C
FALSE $rttidef$INIT_$U_$$_C
FALSE $InternalRec
FALSE $InternalRec
FALSE $rttidef$RTTI_$U_$$_C
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
FALSE $InternalRec
At the very least:
defid:=defid_not_registered;
name:='$InternalRec'+unique_id_str;
which I find hacky.
The LLVM backend makes use of anonymous record defs and I don't know whether
your patch would break that.
Presumably, if such duplicate internal names were breaking LLVM, it would have
been noticed already?
Maybe Jonas could weigh in?
--
βþ
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Re: [fpc-devel] Autogenerated names for internal RecordDefs
Am 30.11.2019 um 13:27 schrieb bla...@blaise.ru:
{ This is a resubmission of
https://lists.freepascal.org/pipermail/fpc-devel/2019-August/041915.html
}
trecorddef.create_global_internal generates internal RecordDef names as
'$InternalRec'+tostr(current_module.deflist.count)
However, since such internal RecordDefs are not necessarily registered
afterwards (i.e. deflist.count is not going to be bumped), such names
do not contain actual DefIDs, and thus are not unique within a module.
(See the original email for a test case.)
1) So, what is the point of that misleading numerical suffix? I
propose that it be dropped.
2) My understanding is that a name should never be autogenerated for
an internal RecordDef that is going to be registered with a DefID. I
propose that this be asserted (two variants are offered).
The same draft patch is attached.
I've fixed this now in r43616 by using the correct way to generate a
unique ID string. The LLVM backend makes use of anonymous record defs
and I don't know whether your patch would break that.
Regards,
Sven
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Re: [fpc-devel] Autogenerated names for internal RecordDefs
{ This is a resubmission of
https://lists.freepascal.org/pipermail/fpc-devel/2019-August/041915.html }
trecorddef.create_global_internal generates internal RecordDef names as
'$InternalRec'+tostr(current_module.deflist.count)
However, since such internal RecordDefs are not necessarily registered
afterwards (i.e. deflist.count is not going to be bumped), such names do not
contain actual DefIDs, and thus are not unique within a module.
(See the original email for a test case.)
1) So, what is the point of that misleading numerical suffix? I propose that it
be dropped.
2) My understanding is that a name should never be autogenerated for an
internal RecordDef that is going to be registered with a DefID. I propose that
this be asserted (two variants are offered).
The same draft patch is attached.
--
βþ
# HG changeset patch
# User Blaise.ru
# Date 1565724859 -10800
# 13.08.2019 22:34:19 +0300
# Node ID d56502f486fda6e205e1ce68f172c622a973662d
# Parent 46bd9a9913f31e10286ae4d9320918040c217858
= trecorddef.create_global_internal: non-misleading autogenerated names
diff -r 46bd9a9913f3 -r d56502f486fd symdef.pas
--- a/symdef.pas30.11.2019 14:35:10 +0300
+++ b/symdef.pas13.08.2019 22:34:19 +0300
@@ -4789,8 +4789,8 @@
constructor trecorddef.create_global_internal(const n: string;
packrecords, recordalignmin: shortint);
+ const internaltypeName: string = '$InternalRec';
var
-name : string;
pname : pshortstring;
oldsymtablestack: tsymtablestack;
where : tsymtable;
@@ -4801,8 +4801,9 @@
pname:=@n
else
begin
-name:='$InternalRec'+tostr(current_module.deflist.count);
-pname:=@name;
+if current_module.in_interface then
+ internalerror(2019081301); // do not register a nameless def
+pname:=@internaltypeName;
end;
oldsymtablestack:=symtablestack;
{ do not simply push/pop current_module.localsymtable, because
@@ -4826,7 +4827,10 @@
typed constant data) }
ts.increfcount;
where.insert(ts);
- end;
+ end
+else
+ if defid<>defid_not_registered then
+internalerror(2019081301); // we registered a nameless def
symtablestack:=oldsymtablestack;
{ don't create RTTI for internal types, these are not exported }
defstates:=defstates+[ds_rtti_table_written,ds_init_table_written];
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
