[fpc-devel] Refactoring TVMTBuilder

Blaise at blaise.ru Blaise at blaise.ru
Thu Dec 5 13:48:39 CET 2019


1) Currently, the following snippet
> 	vmtbuilder:=TVMTBuilder.Create(...);
> 	vmtbuilder.generate_vmt;
> 	vmtbuilder.free;
is present @
	types_dec :objectdef
	generate_specialization_phase2 :objectdef
	jvm_maybe_create_enum_class
	jvm_create_procvar_class_intern
and there will be the fifth instance for closures.
The first patch extracts the above into a separate routine -- build_vmt.

2) Because of SVN r41884, I strongly suggest that build_vmt be actually named insert_hidden_paras_and_build_vmt (the second patch), in order that the tight coupling of these two unrelated operations be reflected in the name. Unless you are ready for something generic like postprocess_object, that mouthful is better than simple "build_vmt".
Should this be rejected, at the very least, add the following comment
	// Despite the name, this also inserts method's hidden parameters
above the interface declaration of build_vmt.

3) I would like to draw a strong terminological distinction between creating/building data structures within the compiler, and generating code/data in object files. NCGVMT.TVMTWriter is the one concerned with VMT /generation/, not NObj.TVMTBuilder, which creates & builds the data structures.
The third patch renames
	TVMTBuilder.generate_vmt_def -> .create_vmtdef	// or, maybe, build_vmtdef
	TVMTBuilder.generate_vmt -> .build
Also, .build_interface_mappings becomes private and it is grouped within the TVMTBuilder declaration with the methods it uses.

(This submission is only split into three patches for easy cherry-picking; in my book, they are just fine becoming a single SVN commit.)

-- 
βþ
-------------- next part --------------
# HG changeset patch
# User Blaise.ru
# Date 1575541768 -10800
#      05.12.2019 13:29:28 +0300
# Node ID 188c08727c15514220b9b4d4510d2c89531db6bc
# Parent  da4b079f46f3570d6d00e61def2928d14efa4f19
= extract the VMT-building call sequence into a separate routine

diff -r da4b079f46f3 -r 188c08727c15 jvm/pjvm.pas
--- a/jvm/pjvm.pas
+++ b/jvm/pjvm.pas
@@ -140,7 +140,6 @@
 
     procedure jvm_maybe_create_enum_class(const name: TIDString; def: tdef);
       var
-        vmtbuilder: tvmtbuilder;
         arrdef: tarraydef;
         arrsym: ttypesym;
         juhashmap: tdef;
@@ -319,9 +318,7 @@
 
         symtablestack.pop(enumclass.symtable);
 
-        vmtbuilder:=TVMTBuilder.Create(enumclass);
-        vmtbuilder.generate_vmt;
-        vmtbuilder.free;
+        build_vmt(enumclass);
 
         restore_after_new_class(sstate,islocal,oldsymtablestack);
         current_structdef:=old_current_structdef;
@@ -330,7 +327,6 @@
 
     procedure jvm_create_procvar_class_intern(const name: TIDString; def: tdef; force_no_callback_intf: boolean);
       var
-        vmtbuilder: tvmtbuilder;
         oldsymtablestack: tsymtablestack;
         pvclass,
         pvintf: tobjectdef;
@@ -429,9 +425,7 @@
 
         symtablestack.pop(pvclass.symtable);
 
-        vmtbuilder:=TVMTBuilder.Create(pvclass);
-        vmtbuilder.generate_vmt;
-        vmtbuilder.free;
+        build_vmt(pvclass);
 
         restore_after_new_class(sstate,islocal,oldsymtablestack);
       end;
diff -r da4b079f46f3 -r 188c08727c15 nobj.pas
--- a/nobj.pas
+++ b/nobj.pas
@@ -53,6 +53,7 @@
         procedure  build_interface_mappings;
       end;
 
+    procedure build_vmt(c: tobjectdef);
 
 implementation
 
@@ -63,6 +64,14 @@
        defcmp,
        pparautl;
 
+    procedure build_vmt(c: tobjectdef);
+      var
+        vmtbuilder: tvmtbuilder;
+      begin
+        vmtbuilder:=TVMTBuilder.Create(c);
+        vmtbuilder.generate_vmt;
+        vmtbuilder.free;
+      end;
 
 {*****************************************************************************
                               TVMTBuilder
diff -r da4b079f46f3 -r 188c08727c15 pdecl.pas
--- a/pdecl.pas
+++ b/pdecl.pas
@@ -661,7 +661,6 @@
          istyperenaming : boolean;
          generictypelist : tfphashobjectlist;
          generictokenbuf : tdynamicarray;
-         vmtbuilder : TVMTBuilder;
          p:tnode;
          gendef : tstoreddef;
          s : shortstring;
@@ -1088,11 +1087,7 @@
                     { Build VMT indexes, skip for type renaming and forward classes }
                     if (hdef.typesym=newtype) and
                        not(oo_is_forward in tobjectdef(hdef).objectoptions) then
-                      begin
-                        vmtbuilder:=TVMTBuilder.Create(tobjectdef(hdef));
-                        vmtbuilder.generate_vmt;
-                        vmtbuilder.free;
-                      end;
+                      build_vmt(tobjectdef(hdef));
 
                     { In case of an objcclass, verify that all methods have a message
                       name set. We only check this now, because message names can be set
diff -r da4b079f46f3 -r 188c08727c15 pgenutil.pas
--- a/pgenutil.pas
+++ b/pgenutil.pas
@@ -675,7 +675,6 @@
         oldcurrent_filepos : tfileposinfo;
         recordbuf : tdynamicarray;
         hadtypetoken : boolean;
-        vmtbuilder : tvmtbuilder;
         i,
         replaydepth : longint;
         item : tobject;
@@ -999,10 +998,7 @@
                           if replaydepth>current_scanner.replay_stack_depth then
                             consume(_SEMICOLON);
                         end;
-
-                      vmtbuilder:=TVMTBuilder.Create(tobjectdef(result));
-                      vmtbuilder.generate_vmt;
-                      vmtbuilder.free;
+                      build_vmt(tobjectdef(result));
                     end;
                   { handle params, calling convention, etc }
                   procvardef:
-------------- next part --------------
# HG changeset patch
# User Blaise.ru
# Date 1575544676 -10800
#      05.12.2019 14:17:56 +0300
# Node ID 79d9ee423e7562b8133c54641c43183a38808702
# Parent  188c08727c15514220b9b4d4510d2c89531db6bc
= build_vmt -> insert_hidden_paras_and_build_vmt

diff -r 188c08727c15 -r 79d9ee423e75 jvm/pjvm.pas
--- a/jvm/pjvm.pas
+++ b/jvm/pjvm.pas
@@ -318,7 +318,7 @@
 
         symtablestack.pop(enumclass.symtable);
 
-        build_vmt(enumclass);
+        insert_hidden_paras_and_build_vmt(enumclass);
 
         restore_after_new_class(sstate,islocal,oldsymtablestack);
         current_structdef:=old_current_structdef;
@@ -425,7 +425,7 @@
 
         symtablestack.pop(pvclass.symtable);
 
-        build_vmt(pvclass);
+        insert_hidden_paras_and_build_vmt(pvclass);
 
         restore_after_new_class(sstate,islocal,oldsymtablestack);
       end;
diff -r 188c08727c15 -r 79d9ee423e75 nobj.pas
--- a/nobj.pas
+++ b/nobj.pas
@@ -53,7 +53,7 @@
         procedure  build_interface_mappings;
       end;
 
-    procedure build_vmt(c: tobjectdef);
+    procedure insert_hidden_paras_and_build_vmt(c: tobjectdef);
 
 implementation
 
@@ -64,7 +64,7 @@
        defcmp,
        pparautl;
 
-    procedure build_vmt(c: tobjectdef);
+    procedure insert_hidden_paras_and_build_vmt(c: tobjectdef);
       var
         vmtbuilder: tvmtbuilder;
       begin
diff -r 188c08727c15 -r 79d9ee423e75 pdecl.pas
--- a/pdecl.pas
+++ b/pdecl.pas
@@ -1087,7 +1087,7 @@
                     { Build VMT indexes, skip for type renaming and forward classes }
                     if (hdef.typesym=newtype) and
                        not(oo_is_forward in tobjectdef(hdef).objectoptions) then
-                      build_vmt(tobjectdef(hdef));
+                      insert_hidden_paras_and_build_vmt(tobjectdef(hdef));
 
                     { In case of an objcclass, verify that all methods have a message
                       name set. We only check this now, because message names can be set
diff -r 188c08727c15 -r 79d9ee423e75 pgenutil.pas
--- a/pgenutil.pas
+++ b/pgenutil.pas
@@ -998,7 +998,7 @@
                           if replaydepth>current_scanner.replay_stack_depth then
                             consume(_SEMICOLON);
                         end;
-                      build_vmt(tobjectdef(result));
+                      insert_hidden_paras_and_build_vmt(tobjectdef(result));
                     end;
                   { handle params, calling convention, etc }
                   procvardef:
-------------- next part --------------
# HG changeset patch
# User Blaise.ru
# Date 1575548758 -10800
#      05.12.2019 15:25:58 +0300
# Node ID 18159973ba8fc972b0d81060f4e3e62cce54d128
# Parent  79d9ee423e7562b8133c54641c43183a38808702
= proper naming: TVMTBuilder does create & build but does not generate

diff -r 79d9ee423e75 -r 18159973ba8f nobj.pas
--- a/nobj.pas
+++ b/nobj.pas
@@ -1,8 +1,7 @@
 {
     Copyright (c) 1998-2002 by Florian Klaempfl
 
-    Routines for the code generation of data structures
-    like VMT, Messages, VTables, Interfaces descs
+    Builds data structures like VMT, Messages, VTables, Interfaces descs
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -44,13 +43,13 @@
         procedure intf_get_procdefs(ImplIntf:TImplementedInterface;IntfDef:TObjectDef);
         procedure intf_get_procdefs_recursive(ImplIntf:TImplementedInterface;IntfDef:TObjectDef);
         procedure prot_get_procdefs_recursive(ImplProt:TImplementedInterface;ProtDef:TObjectDef);
+        procedure build_interface_mappings;
         procedure intf_optimize_vtbls;
         procedure intf_allocate_vtbls;
-        procedure generate_vmt_def;
+        procedure create_vmtdef;
       public
         constructor create(c:tobjectdef);
-        procedure  generate_vmt;
-        procedure  build_interface_mappings;
+        procedure build;
       end;
 
     procedure insert_hidden_paras_and_build_vmt(c: tobjectdef);
@@ -69,7 +68,7 @@
         vmtbuilder: tvmtbuilder;
       begin
         vmtbuilder:=TVMTBuilder.Create(c);
-        vmtbuilder.generate_vmt;
+        vmtbuilder.build;
         vmtbuilder.free;
       end;
 
@@ -804,7 +803,7 @@
       end;
 
 
-    procedure TVMTBuilder.generate_vmt_def;
+    procedure TVMTBuilder.create_vmtdef;
       var
         i: longint;
         vmtdef: trecorddef;
@@ -900,7 +899,7 @@
       end;
 
 
-    procedure TVMTBuilder.generate_vmt;
+    procedure TVMTBuilder.build;
       var
         i : longint;
         def : tdef;
@@ -938,7 +937,7 @@
             { Allocate interface tables }
             intf_allocate_vtbls;
           end;
-        generate_vmt_def;
+        create_vmtdef;
         current_structdef:=old_current_structdef;
       end;
 


More information about the fpc-devel mailing list