From 237df2bb74a9ecc9bbc72cbdd5286d2a06987611 Mon Sep 17 00:00:00 2001 From: Paolo Pastori <75467826+paolopas@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:28:13 +0200 Subject: [PATCH 1/2] util/order.jam (and engine/mod_order.cpp) review + restored Jam add-pair method, for better error control (Jam does a syntax check for arguments at call site with relative error message emission) and better preconditions checkings too: avoid null arguments, avoid duplicate constraints (which led to edge duplication in final graph,) and avoid self referencing (which led to cycles in final graph.) The old add-pair was even broken, since it spelled .constraits instead of .constraints + removed stale order method implementation, fully replaced by native one + removed all other stale methods, no more useful/functional mod_order.cpp rewritten + to implement the topological sort algorithm using std::vector instead of mem.h + also removed jam_strings.h include + prepared for future warning emission on "cyclic dependency" + no more need for native add_pair --- src/engine/mod_order.cpp | 155 +++++++++++++-------------------------- src/util/order.jam | 122 +++--------------------------- 2 files changed, 64 insertions(+), 213 deletions(-) diff --git a/src/engine/mod_order.cpp b/src/engine/mod_order.cpp index d6f35cc34c..01d4d86e99 100644 --- a/src/engine/mod_order.cpp +++ b/src/engine/mod_order.cpp @@ -1,4 +1,5 @@ /* Copyright 2004. Vladimir Prus + * Copyright 2026 Paolo Pastori * Distributed under the Boost Software License, Version 1.0. * (See accompanying file LICENSE.txt or copy at * https://www.bfgroup.xyz/b2/LICENSE.txt) @@ -6,31 +7,26 @@ #include "frames.h" #include "lists.h" -#include "mem.h" #include "native.h" #include "object.h" -#include "jam_strings.h" #include "variable.h" +#include +#include -/* Use quite klugy approach: when we add order dependency from 'a' to 'b', just - * append 'b' to of value of variable 'a'. - */ -LIST * add_pair( FRAME * frame, int32_t flags ) -{ - LIST * arg = lol_get( frame->args, 0 ); - LISTITER iter = list_begin( arg ); - LISTITER const end = list_end( arg ); - var_set( frame->module, list_item( iter ), list_copy_range( arg, list_next( - iter ), end ), VAR_APPEND ); - return L0; -} +// vertex type, NOTE: limit the max number of vertices in the graph +using node_typ = uint16_t; +using node_vec = std::vector; +using vec_graph = std::vector; + +enum node_state { TO_VISIT, VISITING, VISITED }; +using state_vec = std::vector; /* Given a list and a value, returns position of that value in the list, or -1 * if not found. */ -int32_t list_index( LIST * list, OBJECT * value ) +static int32_t list_index( LIST * list, OBJECT * value ) { int32_t result = 0; LISTITER iter = list_begin( list ); @@ -41,118 +37,73 @@ int32_t list_index( LIST * list, OBJECT * value ) return -1; } -enum colors { white, gray, black }; - - -/* Main routine for topological sort. Calls itself recursively on all adjacent - * vertices which were not yet visited. After that, 'current_vertex' is added to - * '*result_ptr'. +/* Routine for depth first traversal. Calls itself recursively on all adjacent + * vertices which were not yet visited. After that, 'current_vertex' is added + * to 'result'. */ -void do_ts( int32_t * * graph, int32_t current_vertex, int32_t * colors, int32_t * * result_ptr - ) +static void do_df( FRAME * frame, const vec_graph & graph, + int32_t current_vertex, state_vec & state, node_vec & result ) { - int32_t i; - - colors[ current_vertex ] = gray; - for ( i = 0; graph[ current_vertex ][ i ] != -1; ++i ) + state[ current_vertex ] = VISITING; + for ( auto adjacent_vertex : graph[ current_vertex ] ) { - int32_t adjacent_vertex = graph[ current_vertex ][ i ]; - if ( colors[ adjacent_vertex ] == white ) - do_ts( graph, adjacent_vertex, colors, result_ptr ); - /* The vertex is either black, in which case we do not have to do - * anything, or gray, in which case we have a loop. If we have a loop, - * it is not clear what useful diagnostic we can emit, so we emit - * nothing. - */ + if ( state[ adjacent_vertex ] == TO_VISIT ) + do_df( frame, graph, adjacent_vertex, state, result ); + // TODO + //if ( state[ adjacent_vertex ] == VISITING ) + // emit warning "Cyclic order dependency on 'x' and 'y'." } - colors[ current_vertex ] = black; - **result_ptr = current_vertex; - ( *result_ptr )++; + state[ current_vertex ] = VISITED; + result.push_back( static_cast( current_vertex ) ); } - -static void topological_sort( int32_t * * graph, int32_t num_vertices, int32_t * result ) +static void topological_sort( FRAME * frame, const vec_graph & graph, + int32_t size, node_vec & result ) { - int32_t i; - int32_t * colors = ( int32_t * )BJAM_CALLOC( num_vertices, sizeof( int32_t ) ); - for ( i = 0; i < num_vertices; ++i ) - colors[ i ] = white; - - for ( i = num_vertices - 1; i >= 0; --i ) - if ( colors[ i ] == white ) - do_ts( graph, i, colors, &result ); - - BJAM_FREE( colors ); + state_vec state(size, TO_VISIT); + for ( int32_t i = size - 1; i >= 0; --i ) + if ( state[ i ] == TO_VISIT ) + do_df( frame, graph, i, state, result ); } LIST * order( FRAME * frame, int32_t flags ) { - LIST * arg = lol_get( frame->args, 0 ); - LIST * result = L0; - int32_t src; - LISTITER iter = list_begin( arg ); - LISTITER const end = list_end( arg ); - - /* We need to create a graph of order dependencies between the passed - * objects. We assume there are no duplicates passed to 'add_pair'. - */ - int32_t length = list_length( arg ); - int32_t * * graph = ( int32_t * * )BJAM_CALLOC( length, sizeof( int32_t * ) ); - int32_t * order = ( int32_t * )BJAM_MALLOC( ( length + 1 ) * sizeof( int32_t ) ); - - for ( src = 0; iter != end; iter = list_next( iter ), ++src ) + b2::list_cref arg( lol_get( frame->args, 0 ) ); + int32_t length = arg.length(); + if (length == 0) return L0; + + // Build dependency graph + vec_graph graph; + graph.reserve( length ); + for ( auto & obj : arg ) { - /* For all objects this one depends upon, add elements to 'graph'. */ - LIST * dependencies = var_get( frame->module, list_item( iter ) ); - int32_t index = 0; - LISTITER dep_iter = list_begin( dependencies ); - LISTITER const dep_end = list_end( dependencies ); - - graph[ src ] = ( int32_t * )BJAM_CALLOC( list_length( dependencies ) + 1, - sizeof( int32_t ) ); - for ( ; dep_iter != dep_end; dep_iter = list_next( dep_iter ) ) + b2::list_cref deps( var_get( frame->module, obj ) ); + node_vec depl; + depl.reserve( deps.length() ); + for ( auto & dep : deps ) { - int32_t const dst = list_index( arg, list_item( dep_iter ) ); + int32_t dst = list_index( *arg, dep ); if ( dst != -1 ) - graph[ src ][ index++ ] = dst; + depl.push_back( static_cast( dst ) ) ; } - graph[ src ][ index ] = -1; + graph.push_back( std::move( depl ) ); } - topological_sort( graph, length, order ); + node_vec order; + order.reserve( length ); + topological_sort( frame, graph, length, order ); - { - int32_t index = length - 1; - for ( ; index >= 0; --index ) - { - int32_t i; - LISTITER iter = list_begin( arg ); - for ( i = 0; i < order[ index ]; ++i, iter = list_next( iter ) ); - result = list_push_back( result, object_copy( list_item( iter ) ) ); - } - } - - /* Clean up */ - { - int32_t i; - for ( i = 0; i < length; ++i ) - BJAM_FREE( graph[ i ] ); - BJAM_FREE( graph ); - BJAM_FREE( order ); - } + b2::list_ref result; + for ( int32_t i = length - 1; i >= 0; --i ) + result.push_back( object_copy( arg[ order[ i ] ] ) ); - return result; + return result.release(); } void init_order() { - { - char const * args[] = { "first", "second", 0 }; - declare_native_rule( "class@order", "add-pair", args, add_pair, 1 ); - } - { char const * args[] = { "objects", "*", 0 }; declare_native_rule( "class@order", "order", args, order, 1 ); diff --git a/src/util/order.jam b/src/util/order.jam index e90dc750cf..ee3167ca3a 100644 --- a/src/util/order.jam +++ b/src/util/order.jam @@ -27,126 +27,27 @@ class order { - rule __init__ ( ) - { - } + rule __init__ ( ) { } # Adds the constraint that 'first' should preceede 'second'. rule add-pair ( first second ) { - .constraits += $(first)--$(second) ; - } - NATIVE_RULE class@order : add-pair ; - - # Given a list of objects, reorder them so that the constraints specified by - # 'add-pair' are satisfied. - # - # The algorithm was adopted from an awk script by Nikita Youshchenko - # (yoush at cs dot msu dot su) - rule order ( objects * ) - { - # The algorithm used is the same is standard transitive closure, except - # that we're not keeping in-degree for all vertices, but rather removing - # edges. - local result ; - if $(objects) - { - local constraints = [ eliminate-unused-constraits $(objects) ] ; - - # Find some library that nobody depends upon and add it to the - # 'result' array. - local obj ; - while $(objects) - { - local new_objects ; - while $(objects) - { - obj = $(objects[1]) ; - if [ has-no-dependents $(obj) : $(constraints) ] - { - # Emulate break ; - new_objects += $(objects[2-]) ; - objects = ; - } - else - { - new_objects += $(obj) ; - obj = ; - objects = $(objects[2-]) ; - } - } - - if ! $(obj) - { - errors.error "Circular order dependencies" ; - } - # No problem with placing first. - result += $(obj) ; - # Remove all constraints where 'obj' comes first, since they are - # already satisfied. - constraints = [ remove-satisfied $(constraints) : $(obj) ] ; - - # Add the remaining objects for further processing on the next - # iteration - objects = $(new_objects) ; - } - - } - return $(result) ; - } - NATIVE_RULE class@order : order ; - - # Eliminate constraints which mention objects not in 'objects'. In - # graph-theory terms, this is finding a subgraph induced by ordered - # vertices. - rule eliminate-unused-constraits ( objects * ) - { - local result ; - for local c in $(.constraints) + if $(first) && $(second) { - local m = [ MATCH (.*)--(.*) : $(c) ] ; - if $(m[1]) in $(objects) && $(m[2]) in $(objects) + # avoid duplicate and self dependencies + if ! $(second) in $($(first)) && $(second) != $(first) { - result += $(c) ; + $(first) += $(second) ; } } - return $(result) ; } - # Returns true if there's no constraint in 'constaraints' where 'obj' comes - # second. - rule has-no-dependents ( obj : constraints * ) - { - local failed ; - while $(constraints) && ! $(failed) - { - local c = $(constraints[1]) ; - local m = [ MATCH (.*)--(.*) : $(c) ] ; - if $(m[2]) = $(obj) - { - failed = true ; - } - constraints = $(constraints[2-]) ; - } - if ! $(failed) - { - return true ; - } - } + # Given a list of objects, reorder them so that the constraints + # specified by 'add-pair' are satisfied. + # + # rule order ( objects * ) - rule remove-satisfied ( constraints * : obj ) - { - local result ; - for local c in $(constraints) - { - local m = [ MATCH (.*)--(.*) : $(c) ] ; - if $(m[1]) != $(obj) - { - result += $(c) ; - } - } - return $(result) ; - } + NATIVE_RULE class@order : order ; } @@ -167,7 +68,6 @@ rule __test__ ( ) assert.result l1 l2 : $(c1).order l2 l1 ; assert.result l1 l2 l3 : $(c1).order l2 l3 l1 ; - # The output should be stable for unconstrained - # elements. + # The output should be stable for unconstrained elements. assert.result l4 l5 : $(c1).order l4 l5 ; } From 925cbc54ee69a64f8fd0a4671079480dc54420fd Mon Sep 17 00:00:00 2001 From: Paolo Pastori <75467826+paolopas@users.noreply.github.com> Date: Mon, 20 Apr 2026 15:20:33 +0200 Subject: [PATCH 2/2] native add-pair for backward compatibility --- src/engine/mod_order.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/engine/mod_order.cpp b/src/engine/mod_order.cpp index 01d4d86e99..7c281b1f05 100644 --- a/src/engine/mod_order.cpp +++ b/src/engine/mod_order.cpp @@ -23,6 +23,23 @@ using vec_graph = std::vector; enum node_state { TO_VISIT, VISITING, VISITED }; using state_vec = std::vector; + +/* Use quite klugy approach: when we add order dependency from 'a' to 'b', + * just append 'b' to of value of variable 'a'. NOTE: This is still here + * only for backward compatibility reasons since latest order.jam use a + * normal class method rule instead of this. + */ +LIST * add_pair( FRAME * frame, int32_t flags ) +{ + LIST * arg = lol_get( frame->args, 0 ); + LISTITER iter = list_begin( arg ); + LISTITER const end = list_end( arg ); + var_set( frame->module, list_item( iter ), list_copy_range( arg, list_next( + iter ), end ), VAR_APPEND ); + return L0; +} + + /* Given a list and a value, returns position of that value in the list, or -1 * if not found. */ @@ -104,6 +121,11 @@ LIST * order( FRAME * frame, int32_t flags ) void init_order() { + { // for backward compatibility, see #593 + char const * args[] = { "first", "second", 0 }; + declare_native_rule( "class@order", "add-pair", args, add_pair, 1 ); + } + { char const * args[] = { "objects", "*", 0 }; declare_native_rule( "class@order", "order", args, order, 1 );