Skip to content

[Refactor] Consolidation of node-searching routines (and improvement to #40761)

Summary

What began as a means to improve the bug fix for #40761 (closed) (only stopping on block nodes if they actually contain things like labels and temps) evolved into a refactor that seeks to remove a number of similar subroutines and replace them with a single, generalised one; in this case, functions passed into foreachnodestatic that see if a child node is of a particular type.

For example, in nbas, there's a function to search for exit nodes:

    function is_exit_statement(var n: tnode; arg: pointer): foreachnoderesult;
      begin
        if (n.nodetype<>exitn) then
          result:=fen_false
        else
          result:=fen_norecurse_true;
      end;

In nflw, there's one for continue nodes:

    function checkcontinue(var n:tnode; arg: pointer): foreachnoderesult;
      begin
        if n.nodetype=continuen then
          result:=fen_norecurse_true
        else
          result:=fen_false;
      end;

These have now been removed and replaced with a new generalised function named node_in_list that calls foreachnodestatic on a function named has_node_of_type, with arg holding a pointer to a TNodeTypeSet.

     function node_in_list(var n: tnode; arg: pointer): foreachnoderesult;
       begin
         if (n.nodetype in PNodeTypeSet(arg)^) then
           result := fen_norecurse_true
         else
           result := fen_false;
       end;
     function has_node_of_type(n: TNode; TypeList: TNodeTypeSet): Boolean;
       begin
         Result := foreachnodestatic(n, @node_in_list, @TypeList);
       end;

As a result:

  • Calls to no_exit_statement_in_block(...)) have been replaced with not has_node_of_type(..., [exitn])
  • Call to foreachnodestatic(...,@checkcontinue,nil) has been replaced with has_node_of_type(...,[continuen])
  • In the case of the #40761 (closed) fix, the line if (TStatementNode(p).left.nodetype in [labeln, asmn, tempcreaten, tempdeleten, blockn]) then has been replaced with if has_node_of_type(TStatementNode(p).left, NodeStripTerminators) then, where NodeStripTerminators is a constant: NodeStripTerminators = [labeln, asmn, tempcreaten, tempdeleten]; (blockn has been removed and has_node_of_type will deeply search block nodes to find any of the 4 node types listed).

System

  • Processor architecture: Cross-platform

What is the current bug behavior?

N/A

What is the behavior after applying this patch?

Block nodes are no longer a blanket terminator for node stripping after an exit node etc, but instead they are checked to see if they can be safely removed or not.

Additional notes

  • Generated code should not change. Compiler may be a fraction of a second slower due to the extra check against the data that arg points to.
  • Code over at #40761 (closed) must compile successfully for this request to be acceptable.
Edited by J. Gareth "Kit" Moreton

Merge request reports