View Issue Details

IDProjectCategoryView StatusLast Update
0020435FPCCompilerpublic2011-10-16 13:01
ReporterAlexander Shishkin Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status feedbackResolutionopen 
Product Version2.7.1 
Summary0020435: [patch] refactoring
Description* tnodelist commented out as unused
* symdef: refactored code duplication in Definition Helpers
Additional Informationpatch against trunk(rev 19403) git mirror
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

2011-10-07 19:49

 

0001-tnodelist-commented-out.patch (7,435 bytes)   
From a9ca1a98ddee81904f3a192ca75e80ce7e9dab2f Mon Sep 17 00:00:00 2001
From: AlexVinS <alexvins@users.sf.net>
Date: Wed, 5 Oct 2011 01:11:27 +0400
Subject: [PATCH 1/2] tnodelist commented out

---
 compiler/ncal.pas |   16 ++++++++--------
 compiler/nflw.pas |   12 ++++++------
 compiler/node.pas |   38 +++++++++++++++++++-------------------
 compiler/nset.pas |   10 +++++-----
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/compiler/ncal.pas b/compiler/ncal.pas
index 90c7c53..2ca91e1 100644
--- a/compiler/ncal.pas
+++ b/compiler/ncal.pas
@@ -146,7 +146,7 @@        tcallnode = class(tbinarynode)
             is abstract or not, if it is abstract, give out a warning
           }
           procedure verifyabstract(sym:TObject;arg:pointer);
-          procedure insertintolist(l : tnodelist);override;
+          //procedure insertintolist(l : tnodelist);override;
           function  pass_1 : tnode;override;
           function  pass_typecheck:tnode;override;
        {$ifdef state_tracking}
@@ -187,7 +187,7 @@        tcallparanode = class(ttertiarynode)
           constructor ppuload(t:tnodetype;ppufile:tcompilerppufile);override;
           procedure ppuwrite(ppufile:tcompilerppufile);override;
           function dogetcopy : tnode;override;
-          procedure insertintolist(l : tnodelist);override;
+          //procedure insertintolist(l : tnodelist);override;
           function pass_typecheck : tnode;override;
           function pass_1 : tnode;override;
           procedure get_paratype;
@@ -595,9 +595,9 @@      tobjectinfoitem = class(tlinkedlistitem)
       end;
 
 
-    procedure tcallparanode.insertintolist(l : tnodelist);
-      begin
-      end;
+    //procedure tcallparanode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
 
 
     function tcallparanode.pass_typecheck : tnode;
@@ -1319,9 +1319,9 @@      tobjectinfoitem = class(tlinkedlistitem)
       end;
 
 
-    procedure tcallnode.insertintolist(l : tnodelist);
-      begin
-      end;
+    //procedure tcallnode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
 
 
     procedure tcallnode.add_init_statement(n:tnode);
diff --git a/compiler/nflw.pas b/compiler/nflw.pas
index cb47d59..9d2a03f 100644
--- a/compiler/nflw.pas
+++ b/compiler/nflw.pas
@@ -67,7 +67,7 @@        tloopnode = class(tbinarynode)
           procedure ppuwrite(ppufile:tcompilerppufile);override;
           procedure buildderefimpl;override;
           procedure derefimpl;override;
-          procedure insertintolist(l : tnodelist);override;
+          //procedure insertintolist(l : tnodelist);override;
           procedure printnodetree(var t:text);override;
           function docompare(p: tnode): boolean; override;
        end;
@@ -997,11 +997,11 @@ implementation
          p.loopflags:=loopflags;
          dogetcopy:=p;
       end;
-
-    procedure tloopnode.insertintolist(l : tnodelist);
-
-      begin
-      end;
+//
+//    procedure tloopnode.insertintolist(l : tnodelist);
+//
+//      begin
+//      end;
 
 
     procedure tloopnode.printnodetree(var t:text);
diff --git a/compiler/node.pas b/compiler/node.pas
index 6504fa3..1068bb9 100644
--- a/compiler/node.pas
+++ b/compiler/node.pas
@@ -280,8 +280,8 @@ interface
        flagsequal : tnodeflags = [nf_error];
 
     type
-       tnodelist = class
-       end;
+       //tnodelist = class
+       //end;
 
       pnode = ^tnode;
       { basic class for the intermediated representation fpc uses }
@@ -368,7 +368,7 @@       tnode = class
            skipping (absolute) equal type conversions        }
          function actualtargetnode: tnode;virtual;
 
-         procedure insertintolist(l : tnodelist);virtual;
+         //procedure insertintolist(l : tnodelist);virtual;
          { writes a node for debugging purpose, shouldn't be called }
          { direct, because there is no test for nil, use printnode  }
          { to write a complete tree }
@@ -403,7 +403,7 @@       tunarynode = class(tnode)
          function ischild(p : tnode) : boolean;override;
          function docompare(p : tnode) : boolean;override;
          function dogetcopy : tnode;override;
-         procedure insertintolist(l : tnodelist);override;
+         //procedure insertintolist(l : tnodelist);override;
          procedure printnodedata(var t:text);override;
       end;
 
@@ -421,7 +421,7 @@       tbinarynode = class(tunarynode)
          function docompare(p : tnode) : boolean;override;
          procedure swapleftright;
          function dogetcopy : tnode;override;
-         procedure insertintolist(l : tnodelist);override;
+         //procedure insertintolist(l : tnodelist);override;
          procedure printnodedata(var t:text);override;
          procedure printnodelist(var t:text);
       end;
@@ -439,7 +439,7 @@       ttertiarynode = class(tbinarynode)
          function ischild(p : tnode) : boolean;override;
          function docompare(p : tnode) : boolean;override;
          function dogetcopy : tnode;override;
-         procedure insertintolist(l : tnodelist);override;
+         //procedure insertintolist(l : tnodelist);override;
          procedure printnodedata(var t:text);override;
       end;
 
@@ -940,10 +940,10 @@             (p.classtype=classtype) and
       end;
 
 
-    procedure tnode.insertintolist(l : tnodelist);
-      begin
-      end;
-
+    //procedure tnode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
+    //
 
     { ensures that the optimizer info record is allocated }
     function tnode.allocoptinfo : poptinfo;inline;
@@ -1022,9 +1022,9 @@             (p.classtype=classtype) and
       end;
 
 
-    procedure tunarynode.insertintolist(l : tnodelist);
-      begin
-      end;
+    //procedure tunarynode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
 
 
     procedure tunarynode.printnodedata(var t:text);
@@ -1135,9 +1135,9 @@             (p.classtype=classtype) and
       end;
 
 
-    procedure tbinarynode.insertintolist(l : tnodelist);
-      begin
-      end;
+    //procedure tbinarynode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
 
 
     procedure tbinarynode.swapleftright;
@@ -1250,9 +1250,9 @@             (p.classtype=classtype) and
       end;
 
 
-    procedure ttertiarynode.insertintolist(l : tnodelist);
-      begin
-      end;
+    //procedure ttertiarynode.insertintolist(l : tnodelist);
+    //  begin
+    //  end;
 
 
     procedure ttertiarynode.printnodedata(var t:text);
diff --git a/compiler/nset.pas b/compiler/nset.pas
index acc1871..57403f5 100644
--- a/compiler/nset.pas
+++ b/compiler/nset.pas
@@ -98,7 +98,7 @@        tcasenode = class(tunarynode)
           procedure buildderefimpl;override;
           procedure derefimpl;override;
           function dogetcopy : tnode;override;
-          procedure insertintolist(l : tnodelist);override;
+          //procedure insertintolist(l : tnodelist);override;
           function pass_typecheck:tnode;override;
           function pass_1 : tnode;override;
           function docompare(p: tnode): boolean; override;
@@ -854,10 +854,10 @@ implementation
          dogetcopy:=n;
       end;
 
-    procedure tcasenode.insertintolist(l : tnodelist);
-
-      begin
-      end;
+    //procedure tcasenode.insertintolist(l : tnodelist);
+    //
+    //  begin
+    //  end;
 
     function caselabelsequal(n1,n2: pcaselabel): boolean;
       begin
-- 
1.7.6.msysgit.0

2011-10-07 19:49

 

0002-symdef-refactored-code-duplication-in-Definition-Hel.patch (9,642 bytes)   
From 3eaa36c078adc5ecb17fe097e87ea2fdee331e5f Mon Sep 17 00:00:00 2001
From: AlexVinS <alexvins@users.sf.net>
Date: Thu, 6 Oct 2011 23:50:53 +0400
Subject: [PATCH 2/2] symdef: refactored code duplication in Definition
 Helpers

---
 compiler/symconst.pas |    1 +
 compiler/symdef.pas   |  128 ++++++++++++++++---------------------------------
 2 files changed, 42 insertions(+), 87 deletions(-)

diff --git a/compiler/symconst.pas b/compiler/symconst.pas
index 3309d58..f224f9c 100644
--- a/compiler/symconst.pas
+++ b/compiler/symconst.pas
@@ -334,6 +334,7 @@ interface
     odt_objccategory, { note that these are changed into odt_class afterwards }
     odt_helper
   );
+  tobjecttyps = set of tobjecttyp;
 
   { defines the type of the extended "structure"; only used for parsing }
   thelpertype=(ht_none,
diff --git a/compiler/symdef.pas b/compiler/symdef.pas
index bb56514..8d7a7a7 100644
--- a/compiler/symdef.pas
+++ b/compiler/symdef.pas
@@ -797,6 +797,9 @@        tdefawaresymtablestack = class(TSymtablestack)
                                  import_nr : word; pco : tproccalloption):string;
 
     { should be in the types unit, but the types unit uses the node stuff :( }
+    { generic version}
+    function is_object_of_type(def: tdef; typs: tobjecttyps): boolean;
+    { special versions}
     function is_interfacecom(def: tdef): boolean;
     function is_interfacecom_or_dispinterface(def: tdef): boolean;
     function is_interfacecorba(def: tdef): boolean;
@@ -5646,97 +5649,73 @@         nname:=class_helper_prefix+tprocsym(pd.procsym).name;
                            Definition Helpers
 ****************************************************************************}
 
-    function is_interfacecom(def: tdef): boolean;
+    function is_object_of_type(def: tdef; typs: tobjecttyps): boolean;
       begin
-        is_interfacecom:=
-          assigned(def) and
+        result:=Assigned(def) and
           (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_interfacecom);
+          (tobjectdef(def).objecttype in typs);
+      end;
+
+    function is_interfacecom(def: tdef): boolean;
+      begin
+        result:=is_object_of_type(def,[odt_interfacecom]);
       end;
 
     function is_interfacecom_or_dispinterface(def: tdef): boolean;
       begin
-        is_interfacecom_or_dispinterface:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_interfacecom,odt_dispinterface]);
+        result:=is_object_of_type(def,[odt_interfacecom,odt_dispinterface]);
       end;
 
     function is_interfacecorba(def: tdef): boolean;
       begin
-        is_interfacecorba:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_interfacecorba);
+        result:=is_object_of_type(def,[odt_interfacecorba]);
       end;
 
     function is_interface(def: tdef): boolean;
       begin
-        is_interface:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_interfacecom,odt_interfacecorba]);
+        result:=is_object_of_type(def,[odt_interfacecom,odt_interfacecorba])
       end;
 
 
     function is_dispinterface(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_dispinterface);
+        result:=is_object_of_type(def,[odt_dispinterface]);
       end;
 
 
     function is_class(def: tdef): boolean;
       begin
-        is_class:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_class);
+        result:=is_object_of_type(def,[odt_class]);
       end;
 
 
     function is_object(def: tdef): boolean;
       begin
-        is_object:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_object);
+        result:=is_object_of_type(def,[odt_object]);
       end;
 
 
     function is_cppclass(def: tdef): boolean;
       begin
-        is_cppclass:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_cppclass);
+        result:=is_object_of_type(def,[odt_cppclass]);
       end;
 
 
     function is_objcclass(def: tdef): boolean;
       begin
-        is_objcclass:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_objcclass);
+        result:=is_object_of_type(def,[odt_objcclass]);
       end;
 
 
     function is_objectpascal_helper(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_helper);
+        result:=is_object_of_type(def,[odt_helper]);
       end;
 
 
     function is_objcclassref(def: tdef): boolean;
       begin
-        is_objcclassref:=
-          assigned(def) and
+        is_objcclassref:=assigned(def) and
           (def.typ=classrefdef) and
           is_objcclass(tclassrefdef(def).pointeddef);
       end;
@@ -5744,42 +5723,30 @@           (def.typ=classrefdef) and
 
     function is_objcprotocol(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_objcprotocol);
+        result:=is_object_of_type(def,[odt_objcprotocol]);
       end;
 
 
     function is_objccategory(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          { if used as a forward type }
-          ((tobjectdef(def).objecttype=odt_objccategory) or
-          { if used as after it has been resolved }
-           ((tobjectdef(def).objecttype=odt_objcclass) and
-            (oo_is_classhelper in tobjectdef(def).objectoptions)));
+        Result:=is_object_of_type(def,[odt_objccategory,odt_objcclass]) and
+            { if used as a forward type }
+            ((tobjectdef(def).objecttype<>odt_objcclass) or
+            { if used as after it has been resolved }
+              (oo_is_classhelper in tobjectdef(def).objectoptions));
       end;
 
     function is_objc_class_or_protocol(def: tdef): boolean;
       begin
-         result:=
-           assigned(def) and
-           (def.typ=objectdef) and
-           (tobjectdef(def).objecttype in [odt_objcclass,odt_objcprotocol]);
+        result:=is_object_of_type(def,[odt_objcclass,odt_objcprotocol]);
       end;
 
 
     function is_objc_protocol_or_category(def: tdef): boolean;
       begin
-         result:=
-           assigned(def) and
-           (def.typ=objectdef) and
-           ((tobjectdef(def).objecttype = odt_objcprotocol) or
-            ((tobjectdef(def).objecttype = odt_objcclass) and
-             (oo_is_classhelper in tobjectdef(def).objectoptions)));
+         result:=is_object_of_type(def,[odt_objcprotocol,odt_objcclass]) and
+           ((tobjectdef(def).objecttype <> odt_objcclass) or
+             (oo_is_classhelper in tobjectdef(def).objectoptions));
       end;
 
     function is_classhelper(def: tdef): boolean;
@@ -5792,53 +5759,40 @@           (def.typ=classrefdef) and
     function is_class_or_interface(def: tdef): boolean;
       begin
         result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba]);
+          is_object_of_type(def,[odt_class,odt_interfacecom,odt_interfacecorba]);
       end;
 
 
     function is_class_or_interface_or_objc(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_objcclass,odt_objcprotocol]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_objcclass,odt_objcprotocol]);
       end;
 
 
     function is_class_or_interface_or_object(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_object]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_object]);
       end;
 
 
     function is_class_or_interface_or_dispinterface(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface]);
       end;
 
 
     function is_implicit_pointer_object_type(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface,odt_objcclass,odt_objcprotocol,odt_helper]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface,odt_objcclass,odt_objcprotocol,odt_helper]);
       end;
 
     function is_class_or_object(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_object]);
+        result:=is_object_of_type(def,[odt_class,odt_object]);
       end;
 
     function is_record(def: tdef): boolean;
-- 
1.7.6.msysgit.0

Florian

2011-10-09 17:57

administrator   ~0052806

What's the real advantage of the second patch? The factored out code is little and these routines are used quite often though a speed penalty might not measurable but those changes add up.

Alexander Shishkin

2011-10-09 22:44

reporter   ~0052812

So you think that making code more laconic is not worth possible little speed penalty? OK. I`ll do some benchmark.

2011-10-10 03:35

 

symdef-refactored2.patch (9,173 bytes)   
745c672e20def0310d4b5869e53030c6f5563fac
 compiler/symconst.pas |    1 +
 compiler/symdef.pas   |  126 ++++++++++++++++++------------------------------
 2 files changed, 48 insertions(+), 79 deletions(-)

diff --git a/compiler/symconst.pas b/compiler/symconst.pas
index 3309d58..f224f9c 100644
--- a/compiler/symconst.pas
+++ b/compiler/symconst.pas
@@ -334,6 +334,7 @@ interface
     odt_objccategory, { note that these are changed into odt_class afterwards }
     odt_helper
   );
+  tobjecttyps = set of tobjecttyp;
 
   { defines the type of the extended "structure"; only used for parsing }
   thelpertype=(ht_none,
diff --git a/compiler/symdef.pas b/compiler/symdef.pas
index bb56514..752b48d 100644
--- a/compiler/symdef.pas
+++ b/compiler/symdef.pas
@@ -797,6 +797,10 @@        tdefawaresymtablestack = class(TSymtablestack)
                                  import_nr : word; pco : tproccalloption):string;
 
     { should be in the types unit, but the types unit uses the node stuff :( }
+    { generic version}
+    function is_object_of_type(def: tdef; typs: tobjecttyps): boolean;{$IFDEF USEINLINE}inline;{$ENDIF}
+    function is_object_of_type(def: tdef; typ: tobjecttyp): boolean;{$IFDEF USEINLINE}inline;{$ENDIF}
+    { special versions}
     function is_interfacecom(def: tdef): boolean;
     function is_interfacecom_or_dispinterface(def: tdef): boolean;
     function is_interfacecorba(def: tdef): boolean;
@@ -5646,97 +5650,80 @@         nname:=class_helper_prefix+tprocsym(pd.procsym).name;
                            Definition Helpers
 ****************************************************************************}
 
-    function is_interfacecom(def: tdef): boolean;
+    function is_object_of_type(def: tdef; typs: tobjecttyps): boolean;
       begin
-        is_interfacecom:=
-          assigned(def) and
+        result:=Assigned(def) and
           (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_interfacecom);
+          (tobjectdef(def).objecttype in typs);
       end;
 
-    function is_interfacecom_or_dispinterface(def: tdef): boolean;
+    function is_object_of_type(def: tdef; typ: tobjecttyp): boolean;
       begin
-        is_interfacecom_or_dispinterface:=
-          assigned(def) and
+        result:=Assigned(def) and
           (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_interfacecom,odt_dispinterface]);
+          (tobjectdef(def).objecttype = typ);
+      end;
+
+    function is_interfacecom(def: tdef): boolean;
+      begin
+        result:=is_object_of_type(def,odt_interfacecom);
+      end;
+
+    function is_interfacecom_or_dispinterface(def: tdef): boolean;
+      begin
+        result:=is_object_of_type(def,[odt_interfacecom,odt_dispinterface]);
       end;
 
     function is_interfacecorba(def: tdef): boolean;
       begin
-        is_interfacecorba:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_interfacecorba);
+        result:=is_object_of_type(def,odt_interfacecorba);
       end;
 
     function is_interface(def: tdef): boolean;
       begin
-        is_interface:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_interfacecom,odt_interfacecorba]);
+        result:=is_object_of_type(def,[odt_interfacecom,odt_interfacecorba])
       end;
 
 
     function is_dispinterface(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_dispinterface);
+        result:=is_object_of_type(def,odt_dispinterface);
       end;
 
 
     function is_class(def: tdef): boolean;
       begin
-        is_class:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_class);
+        result:=is_object_of_type(def,odt_class);
       end;
 
 
     function is_object(def: tdef): boolean;
       begin
-        is_object:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_object);
+        result:=is_object_of_type(def,odt_object);
       end;
 
 
     function is_cppclass(def: tdef): boolean;
       begin
-        is_cppclass:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_cppclass);
+        result:=is_object_of_type(def,odt_cppclass);
       end;
 
 
     function is_objcclass(def: tdef): boolean;
       begin
-        is_objcclass:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_objcclass);
+        result:=is_object_of_type(def,odt_objcclass);
       end;
 
 
     function is_objectpascal_helper(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_helper);
+        result:=is_object_of_type(def,odt_helper);
       end;
 
 
     function is_objcclassref(def: tdef): boolean;
       begin
-        is_objcclassref:=
-          assigned(def) and
+        is_objcclassref:=assigned(def) and
           (def.typ=classrefdef) and
           is_objcclass(tclassrefdef(def).pointeddef);
       end;
@@ -5744,10 +5731,7 @@           (def.typ=classrefdef) and
 
     function is_objcprotocol(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype=odt_objcprotocol);
+        result:=is_object_of_type(def,odt_objcprotocol);
       end;
 
 
@@ -5765,21 +5749,18 @@           (def.typ=classrefdef) and
 
     function is_objc_class_or_protocol(def: tdef): boolean;
       begin
-         result:=
-           assigned(def) and
-           (def.typ=objectdef) and
-           (tobjectdef(def).objecttype in [odt_objcclass,odt_objcprotocol]);
+        result:=is_object_of_type(def,[odt_objcclass,odt_objcprotocol]);
       end;
 
 
     function is_objc_protocol_or_category(def: tdef): boolean;
       begin
-         result:=
-           assigned(def) and
-           (def.typ=objectdef) and
-           ((tobjectdef(def).objecttype = odt_objcprotocol) or
-            ((tobjectdef(def).objecttype = odt_objcclass) and
-             (oo_is_classhelper in tobjectdef(def).objectoptions)));
+        result:=
+          assigned(def) and
+          (def.typ=objectdef) and
+          ((tobjectdef(def).objecttype = odt_objcprotocol) or
+           ((tobjectdef(def).objecttype = odt_objcclass) and
+            (oo_is_classhelper in tobjectdef(def).objectoptions)));
       end;
 
     function is_classhelper(def: tdef): boolean;
@@ -5792,53 +5773,40 @@           (def.typ=classrefdef) and
     function is_class_or_interface(def: tdef): boolean;
       begin
         result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba]);
+          is_object_of_type(def,[odt_class,odt_interfacecom,odt_interfacecorba]);
       end;
 
 
     function is_class_or_interface_or_objc(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_objcclass,odt_objcprotocol]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_objcclass,odt_objcprotocol]);
       end;
 
 
     function is_class_or_interface_or_object(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_object]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_object]);
       end;
 
 
     function is_class_or_interface_or_dispinterface(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface]);
       end;
 
 
     function is_implicit_pointer_object_type(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface,odt_objcclass,odt_objcprotocol,odt_helper]);
+        result:=is_object_of_type(def,
+          [odt_class,odt_interfacecom,odt_interfacecorba,odt_dispinterface,odt_objcclass,odt_objcprotocol,odt_helper]);
       end;
 
     function is_class_or_object(def: tdef): boolean;
       begin
-        result:=
-          assigned(def) and
-          (def.typ=objectdef) and
-          (tobjectdef(def).objecttype in [odt_class,odt_object]);
+        result:=is_object_of_type(def,[odt_class,odt_object]);
       end;
 
     function is_record(def: tdef): boolean;
symdef-refactored2.patch (9,173 bytes)   

Alexander Shishkin

2011-10-10 03:38

reporter   ~0052814

symdef-refactored2.patch: New version w/o speed penalty.

Florian

2011-10-15 22:38

administrator   ~0053032

There is still the additional call? I don't think that this alone is measureable but the problem is: 100 of such changes might add up.

Alexander Shishkin

2011-10-16 11:17

reporter   ~0053041

Additional call is inlined. BTW all these should be inlined. I attached test project which I used to test performance.

2011-10-16 11:18

 

refactor bench.zip (3,543 bytes)

Sergei Gorelkin

2011-10-16 13:01

developer   ~0053042

Manaul inlining is IMHO not an option. It causes additional dependencies on implementation which lead to unwanted recompilations, it also builds up: first the inilined function is simple, later it is 'improved' and starts calling another function and so on. In RTL it is not uncommon already that some functions are inlined 3-4 levels deep.

IMHO we should improve the compiler so it becomes able to inline automatically, based on their actual function complexity, not to waste effort on tweaks like this.

Issue History

Date Modified Username Field Change
2011-10-07 19:48 Alexander Shishkin New Issue
2011-10-07 19:49 Alexander Shishkin File Added: 0001-tnodelist-commented-out.patch
2011-10-07 19:49 Alexander Shishkin File Added: 0002-symdef-refactored-code-duplication-in-Definition-Hel.patch
2011-10-09 17:57 Florian Note Added: 0052806
2011-10-09 17:57 Florian Status new => feedback
2011-10-09 22:44 Alexander Shishkin Note Added: 0052812
2011-10-10 03:35 Alexander Shishkin File Added: symdef-refactored2.patch
2011-10-10 03:38 Alexander Shishkin Note Added: 0052814
2011-10-15 22:38 Florian Note Added: 0053032
2011-10-16 11:17 Alexander Shishkin Note Added: 0053041
2011-10-16 11:18 Alexander Shishkin File Added: refactor bench.zip
2011-10-16 13:01 Sergei Gorelkin Note Added: 0053042