From c76634025949537e9f6ddc43b755c0ce818a8b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 8 Jan 2024 12:43:56 +0100 Subject: [PATCH 01/25] add a way to check that only named parameters used by a function are passed --- .../include/CGAL/Named_function_parameters.h | 58 +++++++++++++++++++ .../STL_Extension/test_cgal_named_params.cpp | 24 ++++++++ 2 files changed, 82 insertions(+) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index cfa5a7a9c5af..aa2d87cdc1c8 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -549,6 +549,64 @@ struct is_default_parameter typedef CGAL::Boolean_tag type; }; + +// code used to make sure all options passed are used by a function +namespace authorized_parameters_impl +{ + +template +struct Tag_wrapper{}; + +template +constexpr +bool is_tag_present(Tag_wrapper, Tag) +{ + if (std::is_same_v) + return true; + else + return is_tag_present(Tag_wrapper(), Tag()); +} + +template +constexpr +bool is_tag_present(Tag_wrapper, Tag) +{ + return true; +} + +template +constexpr +bool is_tag_present(Tag_wrapper, Tag) +{ + return false; +} + +template +constexpr +bool authorized_options_rec(const Named_function_parameters&) +{ + return is_tag_present(Tag_wrapper(), Tag()); +} + +template +constexpr +bool authorized_options_rec(const Named_function_parameters& np) +{ + if (is_tag_present(Tag_wrapper(), Tag())) + return authorized_options_rec(static_cast(np)); + return false; +} + +}// impl namespace + +template +constexpr +bool authorized_options(const Named_function_parameters& np) +{ + return authorized_parameters_impl::authorized_options_rec + (np); +} + } // end of parameters namespace #ifndef CGAL_NO_DEPRECATED_CODE diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index c5fba6ce23b1..a82ff071ee98 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -89,6 +89,28 @@ void test_references(const NamedParameters& np) assert(&eim_ref==&default_value); } +void test_authorized_options() +{ + auto np_ok1 = CGAL::parameters::vertex_point_map(0).edge_index_map(2).face_index_map(3); + auto np_ok2 = CGAL::parameters::face_index_map(3).vertex_point_map(0).edge_index_map(4); + auto np_ko = CGAL::parameters::face_index_map(3).vertex_point_map(0).edge_index_map(4).halfedge_index_map(4); + auto np_default = CGAL::parameters::default_values(); + + + static_assert(CGAL::parameters::authorized_options(np_default)); + static_assert(CGAL::parameters::authorized_options(np_ok1)); + static_assert(CGAL::parameters::authorized_options(np_ok2)); + static_assert(!CGAL::parameters::authorized_options(np_ko)); +} + int main() { test_values_and_types(params::vertex_index_map(A<0>(0)).visitor(A<1>(1))); @@ -111,5 +133,7 @@ int main() static_assert(std::is_same::value); #endif + test_authorized_options(); + return EXIT_SUCCESS; } From 5485062623e3963505a9bb3e6a639a786a929d11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 8 Jan 2024 12:49:45 +0100 Subject: [PATCH 02/25] add a way to disable the check --- STL_Extension/include/CGAL/Named_function_parameters.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index aa2d87cdc1c8..0c100d2eea42 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -603,8 +603,12 @@ template constexpr bool authorized_options(const Named_function_parameters& np) { +#ifndef CGAL_DISABLE_NAMED_FUNCTION_PARAMETERS_CHECKS return authorized_parameters_impl::authorized_options_rec (np); +#else + return true; +#endif } } // end of parameters namespace From adff72c83f7c99405696f1795880ab620c7c821a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 14 Mar 2024 15:56:37 +0100 Subject: [PATCH 03/25] fix ambiguity --- .../include/CGAL/Named_function_parameters.h | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 0c100d2eea42..f65436e20c96 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -557,16 +557,6 @@ namespace authorized_parameters_impl template struct Tag_wrapper{}; -template -constexpr -bool is_tag_present(Tag_wrapper, Tag) -{ - if (std::is_same_v) - return true; - else - return is_tag_present(Tag_wrapper(), Tag()); -} - template constexpr bool is_tag_present(Tag_wrapper, Tag) @@ -581,6 +571,16 @@ bool is_tag_present(Tag_wrapper, Tag) return false; } +template = 1>::type> +constexpr +bool is_tag_present(Tag_wrapper, Tag) +{ + if (std::is_same_v) + return true; + else + return is_tag_present(Tag_wrapper(), Tag()); +} + template constexpr bool authorized_options_rec(const Named_function_parameters&) From c4b113a390f8fd062a78630957a829c4f32d33da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 14 Mar 2024 16:12:11 +0100 Subject: [PATCH 04/25] use enable_if_t --- STL_Extension/include/CGAL/Named_function_parameters.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index f65436e20c96..8c7b749e833e 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -571,7 +571,7 @@ bool is_tag_present(Tag_wrapper, Tag) return false; } -template = 1>::type> +template = 1>> constexpr bool is_tag_present(Tag_wrapper, Tag) { From c0ef2b1e80f9794cdc5be0f1bbbc09f1b113dbca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 25 Mar 2024 11:17:51 +0100 Subject: [PATCH 05/25] add a macro to ease the check --- STL_Extension/include/CGAL/Named_function_parameters.h | 10 ++++++++++ .../test/STL_Extension/test_cgal_named_params.cpp | 3 +++ 2 files changed, 13 insertions(+) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 8c7b749e833e..0677ea30b60a 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -613,6 +613,16 @@ bool authorized_options(const Named_function_parameters& np) } // end of parameters namespace +#ifdef CGAL_NDEBUG +#define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) +#else +#define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) \ +{ \ +using namespace ::CGAL::internal_np; \ +static_assert(::CGAL::parameters::authorized_options<__VA_ARGS__>(np)); \ +} +#endif + #ifndef CGAL_NO_DEPRECATED_CODE namespace Polygon_mesh_processing { diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index a82ff071ee98..99edb6273fd0 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -109,6 +109,9 @@ void test_authorized_options() static_assert(!CGAL::parameters::authorized_options(np_ko)); + + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok1, vertex_point_t, edge_index_t, face_index_t); + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok2, vertex_point_t, edge_index_t, face_index_t); } int main() From 29679af5a87fe6a113363341b68fb546592eb1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 18 Apr 2024 09:35:49 +0200 Subject: [PATCH 06/25] add constexpr for NP and a flag for to disable allowed nps --- .../include/CGAL/Named_function_parameters.h | 32 +++++++++++++------ .../internal/parameters_interface.h | 1 + .../STL_Extension/test_cgal_named_params.cpp | 9 +++++- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 0677ea30b60a..2969874283c4 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -67,7 +67,7 @@ struct Named_params_impl : Base { typename std::conditional::value, T, std::reference_wrapper >::type v; // copy of the parameter if copyable - Named_params_impl(const T& v, const Base& b) + constexpr Named_params_impl(const T& v, const Base& b) : Base(b) , v(v) {} @@ -79,7 +79,7 @@ struct Named_params_impl { typename std::conditional::value, T, std::reference_wrapper >::type v; // copy of the parameter if copyable - Named_params_impl(const T& v) + constexpr Named_params_impl(const T& v) : v(v) {} }; @@ -256,7 +256,7 @@ namespace parameters{ typedef Named_function_parameters Default_named_parameters; Default_named_parameters -inline default_values(); +inline constexpr default_values(); // function to extract a parameter template @@ -339,14 +339,15 @@ struct Named_function_parameters typedef internal_np::Named_params_impl base; typedef Named_function_parameters self; - Named_function_parameters() : base(T()) {} - Named_function_parameters(const T& v) : base(v) {} - Named_function_parameters(const T& v, const Base& b) : base(v, b) {} + constexpr Named_function_parameters() : base(T()) {} + constexpr Named_function_parameters(const T& v) : base(v) {} + constexpr Named_function_parameters(const T& v, const Base& b) : base(v, b) {} // create the functions for new named parameters and the one imported boost // used to concatenate several parameters #define CGAL_add_named_parameter(X, Y, Z) \ template \ + constexpr \ Named_function_parameters \ Z(const K& k) const \ { \ @@ -355,7 +356,7 @@ struct Named_function_parameters } #define CGAL_add_named_parameter_with_compatibility(X, Y, Z) \ template \ - Named_function_parameters \ + constexpr Named_function_parameters \ Z(const K& k) const \ { \ typedef Named_function_parameters Params;\ @@ -363,6 +364,7 @@ struct Named_function_parameters } #define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z) \ template \ + constexpr \ Named_function_parameters, \ internal_np::X, self> \ Z(const K& k) const \ @@ -373,6 +375,7 @@ struct Named_function_parameters } #define CGAL_add_named_parameter_with_compatibility_ref_only(X, Y, Z) \ template \ + constexpr \ Named_function_parameters, \ internal_np::X, self> \ Z(K& k) const \ @@ -383,6 +386,7 @@ struct Named_function_parameters } #define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) \ template \ + constexpr \ Named_function_parameters \ Z(const K& k) const \ { \ @@ -406,6 +410,7 @@ struct Named_function_parameters #undef CGAL_NP_BUILD template + constexpr Named_function_parameters combine(const Named_function_parameters& np) const { @@ -413,6 +418,7 @@ struct Named_function_parameters } template + constexpr auto combine(const Named_function_parameters& np, const NPS& ... nps) const { @@ -426,14 +432,14 @@ struct Named_function_parameters namespace parameters { Default_named_parameters -inline default_values() +inline constexpr default_values() { return Default_named_parameters(); } #ifndef CGAL_NO_DEPRECATED_CODE Default_named_parameters -inline all_default() +inline constexpr all_default() { return Default_named_parameters(); } @@ -443,6 +449,7 @@ template struct Boost_parameter_compatibility_wrapper { template + constexpr Named_function_parameters operator()(const K& p) const { @@ -452,6 +459,7 @@ struct Boost_parameter_compatibility_wrapper template Named_function_parameters + constexpr operator=(const K& p) const { typedef Named_function_parameters Params; @@ -502,7 +510,7 @@ struct Boost_parameter_compatibility_wrapper // define free functions and Boost_parameter_compatibility_wrapper for named parameters #define CGAL_add_named_parameter(X, Y, Z) \ template \ - Named_function_parameters \ + constexpr Named_function_parameters \ Z(const K& p) \ { \ typedef Named_function_parameters Params;\ @@ -585,6 +593,8 @@ template constexpr bool authorized_options_rec(const Named_function_parameters&) { + if constexpr (std::is_same_v) + return true; return is_tag_present(Tag_wrapper(), Tag()); } @@ -592,6 +602,8 @@ template constexpr bool authorized_options_rec(const Named_function_parameters& np) { + if constexpr (std::is_same_v) + return true; if (is_tag_present(Tag_wrapper(), Tag())) return authorized_options_rec(static_cast(np)); return false; diff --git a/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h b/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h index ffe386d086cb..8d990a008455 100644 --- a/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h +++ b/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h @@ -198,6 +198,7 @@ CGAL_add_named_parameter(force_filtering_t, force_filtering, force_filtering) //internal CGAL_add_named_parameter(weight_calculator_t, weight_calculator, weight_calculator) CGAL_add_named_parameter(use_bool_op_to_clip_surface_t, use_bool_op_to_clip_surface, use_bool_op_to_clip_surface) +CGAL_add_named_parameter(do_not_check_allowed_np_t, do_not_check_allowed_np, do_not_check_allowed_np) // List of named parameters used in the Point Set Processing package CGAL_add_named_parameter(query_point_t, query_point_map, query_point_map) diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index 99edb6273fd0..623027b17b8b 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -93,7 +93,8 @@ void test_authorized_options() { auto np_ok1 = CGAL::parameters::vertex_point_map(0).edge_index_map(2).face_index_map(3); auto np_ok2 = CGAL::parameters::face_index_map(3).vertex_point_map(0).edge_index_map(4); - auto np_ko = CGAL::parameters::face_index_map(3).vertex_point_map(0).edge_index_map(4).halfedge_index_map(4); + constexpr auto np_ko = CGAL::parameters::face_index_map(3).vertex_point_map(0).edge_index_map(4).halfedge_index_map(4); + auto np_permissive = np_ko.do_not_check_allowed_np(true); auto np_default = CGAL::parameters::default_values(); @@ -109,6 +110,12 @@ void test_authorized_options() static_assert(!CGAL::parameters::authorized_options(np_ko)); + static_assert(CGAL::parameters::authorized_options(np_permissive)); + static_assert(CGAL::parameters::authorized_options(np_ko.do_not_check_allowed_np(true))); CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok1, vertex_point_t, edge_index_t, face_index_t); CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok2, vertex_point_t, edge_index_t, face_index_t); From 57116c8db00f432aaac1772d8af779904426f2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 24 Oct 2024 00:04:10 +0200 Subject: [PATCH 07/25] start adding checks --- Convex_hull_3/include/CGAL/convex_hull_3.h | 2 ++ .../include/CGAL/Kinetic_space_partition_3.h | 20 +++++++++++--- .../CGAL/Kinetic_surface_reconstruction_3.h | 27 ++++++++++++++++--- .../edge_collapse.h | 5 ++++ .../CGAL/Adaptive_remeshing_sizing_field.h | 2 ++ .../include/CGAL/tetrahedral_remeshing.h | 6 +++++ 6 files changed, 54 insertions(+), 8 deletions(-) diff --git a/Convex_hull_3/include/CGAL/convex_hull_3.h b/Convex_hull_3/include/CGAL/convex_hull_3.h index 38053786f0e9..edec1ea39d83 100644 --- a/Convex_hull_3/include/CGAL/convex_hull_3.h +++ b/Convex_hull_3/include/CGAL/convex_hull_3.h @@ -1068,6 +1068,8 @@ void convex_hull_3(const VertexListGraph& g, PolygonMesh& pm, const NamedParameters& np = parameters::default_values()) { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, vertex_point_t); + using CGAL::parameters::choose_parameter; using CGAL::parameters::get_parameter; diff --git a/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h b/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h index 214818f3f7ba..c2dfc264b759 100644 --- a/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h +++ b/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h @@ -278,7 +278,10 @@ class Kinetic_space_partition_3 { m_parameters( parameters::choose_parameter(parameters::get_parameter(np, internal_np::verbose), false), parameters::choose_parameter(parameters::get_parameter(np, internal_np::debug), false)), // use true here to export all steps - m_input2regularized() {} + m_input2regularized() + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, verbose_t, debug_t); + } /*! \brief constructs a kinetic space partition object and initializes it. @@ -354,7 +357,11 @@ class Kinetic_space_partition_3 { m_parameters( parameters::choose_parameter(parameters::get_parameter(np, internal_np::verbose), false), parameters::choose_parameter(parameters::get_parameter(np, internal_np::debug), false)), // use true here to export all steps - m_input2regularized() { + m_input2regularized() + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t, debug_t, verbose_t, reorient_bbox_t, + bbox_dilation_ratio_t, max_octree_depth_t, max_octree_node_size_t); + insert(points, polygons, np); initialize(np); } @@ -392,7 +399,9 @@ class Kinetic_space_partition_3 { void insert( const PointRange& points, const PolygonRange& polygons, - const NamedParameters& np = CGAL::parameters::default_values()) { + const NamedParameters& np = CGAL::parameters::default_values()) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t); using NP_helper = Point_set_processing_3_np_helper; using PointMap = typename NP_helper::Point_map; @@ -481,7 +490,10 @@ class Kinetic_space_partition_3 { template< typename NamedParameters = parameters::Default_named_parameters> void initialize( - const NamedParameters& np = CGAL::parameters::default_values()) { + const NamedParameters& np = CGAL::parameters::default_values()) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, reorient_bbox_t, bbox_dilation_ratio_t, max_octree_depth_t, max_octree_node_size_t); + Timer timer; m_parameters.bbox_dilation_ratio = parameters::choose_parameter( parameters::get_parameter(np, internal_np::bbox_dilation_ratio), FT(11) / FT(10)); diff --git a/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h b/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h index 5873b7df6723..f93dd2c85ce2 100644 --- a/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h +++ b/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h @@ -90,7 +90,10 @@ class Kinetic_surface_reconstruction_3 { */ template Kinetic_surface_reconstruction_3(Point_range& points, - const NamedParameters& np = CGAL::parameters::default_values()) : m_points(points), m_ground_polygon_index(-1), m_kinetic_partition(np) { + const NamedParameters& np = CGAL::parameters::default_values()) : m_points(points), m_ground_polygon_index(-1), m_kinetic_partition(np) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t); + m_verbose = parameters::choose_parameter(parameters::get_parameter(np, internal_np::verbose), false); m_debug = parameters::choose_parameter(parameters::get_parameter(np, internal_np::debug), false); @@ -177,7 +180,14 @@ class Kinetic_surface_reconstruction_3 { */ template - std::size_t detect_planar_shapes(const CGAL_NP_CLASS& np = parameters::default_values()) { + std::size_t detect_planar_shapes(const CGAL_NP_CLASS& np = parameters::default_values()) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t, normal_t, k_neighbors_t, maximum_distance_t, maximum_angle_t, + minimum_region_size_t, angle_tolerance_t, maximum_offset_t, + regularize_parallelism_t, regularize_orthogonality_t, regularize_coplanarity_t, + regularize_axis_symmetry_t, symmetry_direction_t); + + m_verbose = parameters::choose_parameter(parameters::get_parameter(np, internal_np::verbose), m_verbose); m_debug = parameters::choose_parameter(parameters::get_parameter(np, internal_np::debug), m_debug); @@ -318,7 +328,13 @@ class Kinetic_surface_reconstruction_3 { */ template - void detection_and_partition(std::size_t k, const CGAL_NP_CLASS& np = parameters::default_values()) { + void detection_and_partition(std::size_t k, const CGAL_NP_CLASS& np = parameters::default_values()) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t, normal_t, k_neighbors_t, maximum_distance_t, maximum_angle_t, + minimum_region_size_t, angle_tolerance_t, maximum_offset_t, regularize_parallelism_t, + regularize_orthogonality_t, regularize_coplanarity_t, regularize_axis_symmetry_t, + symmetry_direction_t, reorient_bbox_t, bbox_dilation_ratio_t); + detect_planar_shapes(np); initialize_partition(np); partition(k); @@ -372,7 +388,10 @@ class Kinetic_surface_reconstruction_3 { \pre shape detection performed */ template - void initialize_partition(const CGAL_NP_CLASS& np = parameters::default_values()) { + void initialize_partition(const CGAL_NP_CLASS& np = parameters::default_values()) + { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, reorient_bbox_t, bbox_dilation_ratio_t); + m_kinetic_partition.initialize(np); } diff --git a/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/edge_collapse.h b/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/edge_collapse.h index aa7ff01d31ee..f7a58f8f714e 100644 --- a/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/edge_collapse.h +++ b/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/edge_collapse.h @@ -87,6 +87,11 @@ int edge_collapse(TM& tmesh, const ShouldStop& should_stop, const NamedParameters& np = parameters::default_values()) { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, vertex_point_t, geom_traits_t, halfedge_index_t, get_cost_policy_t, + get_placement_policy_t, filter_t, edge_is_constrained_t, + visitor_t, vertex_index_t, use_relaxed_order_t); + + using parameters::choose_parameter; using parameters::get_parameter; diff --git a/Tetrahedral_remeshing/include/CGAL/Adaptive_remeshing_sizing_field.h b/Tetrahedral_remeshing/include/CGAL/Adaptive_remeshing_sizing_field.h index 62f97115f82b..d935719e25cc 100644 --- a/Tetrahedral_remeshing/include/CGAL/Adaptive_remeshing_sizing_field.h +++ b/Tetrahedral_remeshing/include/CGAL/Adaptive_remeshing_sizing_field.h @@ -346,6 +346,8 @@ Adaptive_remeshing_sizing_field create_adaptive_remeshing_sizing_field(const Tr& tr, const CGAL_NP_CLASS& np = parameters::default_values()) { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, edge_is_constrained_t, facet_is_constrained_t, cell_selector_t); + using parameters::choose_parameter; using parameters::get_parameter; diff --git a/Tetrahedral_remeshing/include/CGAL/tetrahedral_remeshing.h b/Tetrahedral_remeshing/include/CGAL/tetrahedral_remeshing.h index 37414e3484e3..90f201b1c369 100644 --- a/Tetrahedral_remeshing/include/CGAL/tetrahedral_remeshing.h +++ b/Tetrahedral_remeshing/include/CGAL/tetrahedral_remeshing.h @@ -188,6 +188,10 @@ void tetrahedral_isotropic_remeshing( const SizingFunction& sizing, const NamedParameters& np = parameters::default_values()) { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, number_of_iterations_t, remesh_boundaries_t, facet_is_constrained_t, + edge_is_constrained_t, vertex_is_constrained_t, cell_selector_t, + smooth_constrained_edges_t); + CGAL_expensive_assertion(tr.is_valid()); using Tr = CGAL::Triangulation_3; @@ -357,6 +361,8 @@ convert_to_triangulation_3( CGAL::Mesh_complex_3_in_triangulation_3 c3t3, const NamedParameters& np = parameters::default_values()) { + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, edge_is_constrained_t, vertex_is_constrained_t); + using parameters::get_parameter; using parameters::choose_parameter; From 3dc32c517c7b59014e807250b357e8bbbc68e58b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 18 Apr 2025 19:44:22 +0200 Subject: [PATCH 08/25] fix version of the script to add the np macro checking --- .../add_checks_for_named_parameters.py | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 Scripts/developer_scripts/add_checks_for_named_parameters.py diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py new file mode 100644 index 000000000000..442162999a7a --- /dev/null +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -0,0 +1,76 @@ +from sys import argv +from sys import stderr +import codecs +import re + +f=codecs.open(argv[1], encoding='utf-8') +res="" +np_name="np" +while True: + line = f.readline() + if not line: + break; + res+=line + + #possible np name + if re.search("bgl_namedparameters", line): + m = re.search("[@\\\]param\s+([^ ]+)\s", line) + if m: + np_name=m.group(1) + print("found "+np_name) + + if re.search("cgalNamedParamsBegin", line): + params=[] + while True: + line = f.readline() + if not line: + break + res+=line + if re.search("cgalNamedParamsEnd", line): + break + m = re.search(r"cgalParamNBegin{\s*([^ ]+)\s*}", line) + if m: + params.append(m.group(1).strip()) + if not line: + break + while True: + line = f.readline() + if not line: + break + res+=line + if re.search("cgalNamedParamsBegin", line): + stderr.write("Function with several nps! Not handled yet\n") + exit(1) + m = re.search("^(\s*).*{", line) + if m: + s=m.group(1)+" CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS("+np_name + for p in params: + s+=", "+p+"_t" + s+=");" + + add_macro=True + + #do nothing if the macro is already there + while True: + line = f.readline() + if not line: + stderr.write("Error parsing the function\n") + exit(1) + if re.search("CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS", line): + add_macro=False + res+=line + break + else: + if line=="\n": + res+=line + else: + break # do we want to support comments? + + if add_macro: + res+=s+"\n" + break; + +o=open("/tmp/out.h", 'w') +o.write(res) +o.close() + From d8be4cb751f6021e3be2609a580ed6faa95d9972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 18 Apr 2025 20:26:02 +0200 Subject: [PATCH 09/25] improvements + inplace writing --- .../add_checks_for_named_parameters.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index 442162999a7a..85ea3cb5b84c 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -6,13 +6,16 @@ f=codecs.open(argv[1], encoding='utf-8') res="" np_name="np" + +modified=False + while True: line = f.readline() if not line: break; res+=line - #possible np name + #possible np name (TODO: does not work if not on the same line) if re.search("bgl_namedparameters", line): m = re.search("[@\\\]param\s+([^ ]+)\s", line) if m: @@ -67,10 +70,17 @@ break # do we want to support comments? if add_macro: - res+=s+"\n" + res+=s+"\n\n" + res+=line + modified=True break; -o=open("/tmp/out.h", 'w') -o.write(res) -o.close() +f.close() +if modified: + f=codecs.open(argv[1], encoding='utf-8', mode='w') + f.write(res) + f.close() + #o=open("/tmp/out.h", 'w') + #o.write(res) + #o.close() From 28d11fc980ad0d0c97c9839feea1ee3113fddf1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 18 Apr 2025 21:28:44 +0200 Subject: [PATCH 10/25] handle some particular cases --- .../add_checks_for_named_parameters.py | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index 85ea3cb5b84c..f8b78c977c7f 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -9,18 +9,38 @@ modified=False + +def is_in_comment_section(line): + global in_comment_section + + if re.search("^\s*"+re.escape("//"), line): + return True + if re.search("^\s*"+re.escape("/*"), line): + in_comment_section=True + return True + if not in_comment_section: + return False + if re.search(re.escape("*/"), line): + in_comment_section=False; + return True + +in_comment_section = False + while True: line = f.readline() if not line: break; res+=line + if not is_in_comment_section(line): + continue + #possible np name (TODO: does not work if not on the same line) if re.search("bgl_namedparameters", line): m = re.search("[@\\\]param\s+([^ ]+)\s", line) if m: np_name=m.group(1) - print("found "+np_name) + #print("found "+np_name) if re.search("cgalNamedParamsBegin", line): params=[] @@ -30,6 +50,7 @@ break res+=line if re.search("cgalNamedParamsEnd", line): + is_in_comment_section(line) break m = re.search(r"cgalParamNBegin{\s*([^ ]+)\s*}", line) if m: @@ -42,8 +63,11 @@ break res+=line if re.search("cgalNamedParamsBegin", line): - stderr.write("Function with several nps! Not handled yet\n") + stderr.write("Function with several nps! Not handled yet ("+argv[1]+")\n") exit(1) + if is_in_comment_section(line): + continue + m = re.search("^(\s*).*{", line) if m: s=m.group(1)+" CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS("+np_name @@ -51,6 +75,17 @@ s+=", "+p+"_t" s+=");" + mb = re.search("(^\s+{)(.*)}$", line) + if mb: + res+=mb.group(1)+"\n"; + res+=s+"\n" + if mb.group(2).strip(): + res+=m.group(1)+" "+mb.group(2)+"\n" + res+=m.group(1)+"}\n" + modified=True + break + + add_macro=True #do nothing if the macro is already there @@ -60,6 +95,7 @@ stderr.write("Error parsing the function\n") exit(1) if re.search("CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS", line): + #TODO: in check mode, we should check that there is a match between code and `params` add_macro=False res+=line break From e53eb25c916fd1e98b1f6eef59f42f31e9380509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Sat, 25 Oct 2025 20:21:45 +0200 Subject: [PATCH 11/25] always replace the macro to handle the update Even if we don't check it does not hurt to replace it (if not manually added) --- .../add_checks_for_named_parameters.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index f8b78c977c7f..fe2226d10e93 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -87,6 +87,7 @@ def is_in_comment_section(line): add_macro=True + macro_already_here=False #do nothing if the macro is already there while True: @@ -95,9 +96,7 @@ def is_in_comment_section(line): stderr.write("Error parsing the function\n") exit(1) if re.search("CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS", line): - #TODO: in check mode, we should check that there is a match between code and `params` - add_macro=False - res+=line + macro_already_here=True break else: if line=="\n": @@ -106,8 +105,9 @@ def is_in_comment_section(line): break # do we want to support comments? if add_macro: - res+=s+"\n\n" - res+=line + res+=s+"\n" + if not macro_already_here: + res+="\n"+line modified=True break; From ea3f73d74035b085f09c908d3bbd4fc1c366a9db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Sat, 25 Oct 2025 20:34:01 +0200 Subject: [PATCH 12/25] emit an error if no parameter was found --- Scripts/developer_scripts/add_checks_for_named_parameters.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index fe2226d10e93..b1b2822877de 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -70,6 +70,9 @@ def is_in_comment_section(line): m = re.search("^(\s*).*{", line) if m: + if len(params)==0: + stderr.write("Cannot parse documented named parameters ("+argv[1]+")\n") + exit(1) s=m.group(1)+" CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS("+np_name for p in params: s+=", "+p+"_t" From 7475b9e657c0a93de2ebd5a6c962d270826961a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Sat, 25 Oct 2025 22:17:17 +0200 Subject: [PATCH 13/25] handle several nps --- .../add_checks_for_named_parameters.py | 110 +++++++++--------- 1 file changed, 56 insertions(+), 54 deletions(-) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index b1b2822877de..5b4d615fbfb5 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -26,57 +26,28 @@ def is_in_comment_section(line): in_comment_section = False +params_stack={} +np_names=[] while True: line = f.readline() if not line: break; res+=line - if not is_in_comment_section(line): - continue - - #possible np name (TODO: does not work if not on the same line) - if re.search("bgl_namedparameters", line): - m = re.search("[@\\\]param\s+([^ ]+)\s", line) - if m: - np_name=m.group(1) - #print("found "+np_name) - - if re.search("cgalNamedParamsBegin", line): - params=[] - while True: - line = f.readline() - if not line: - break - res+=line - if re.search("cgalNamedParamsEnd", line): - is_in_comment_section(line) - break - m = re.search(r"cgalParamNBegin{\s*([^ ]+)\s*}", line) - if m: - params.append(m.group(1).strip()) - if not line: - break - while True: - line = f.readline() - if not line: - break - res+=line - if re.search("cgalNamedParamsBegin", line): - stderr.write("Function with several nps! Not handled yet ("+argv[1]+")\n") - exit(1) - if is_in_comment_section(line): - continue - + if bool(params_stack): + if not is_in_comment_section(line): m = re.search("^(\s*).*{", line) if m: - if len(params)==0: - stderr.write("Cannot parse documented named parameters ("+argv[1]+")\n") - exit(1) - s=m.group(1)+" CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS("+np_name - for p in params: - s+=", "+p+"_t" - s+=");" + s="" + for np_name, params in params_stack.items(): + if len(params)==0: + stderr.write("Cannot parse documented named parameters ("+argv[1]+")\n") + exit(1) + s+=m.group(1)+" CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS("+np_name + for p in params: + s+=", "+p+"_t" + s+=");\n" + params_stack={} mb = re.search("(^\s+{)(.*)}$", line) if mb: @@ -99,20 +70,51 @@ def is_in_comment_section(line): stderr.write("Error parsing the function\n") exit(1) if re.search("CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS", line): - macro_already_here=True - break + continue # in case there are more than one macro else: - if line=="\n": - res+=line - else: + #~ if line=="\n": + #~ res+=line + #~ else: break # do we want to support comments? - if add_macro: - res+=s+"\n" - if not macro_already_here: - res+="\n"+line - modified=True - break; + res+=s+"\n" + if line != "\n": + res+=line + modified=True + continue; + else: + if not is_in_comment_section(line): + continue + + #possible np name (TODO: does not work if not on the same line) + if re.search("bgl_namedparameters", line): + m = re.search("[@\\\]param\s+([^ ]+)\s", line) + if m: + np_name=m.group(1) + np_names.append(np_name) + + if re.search("cgalNamedParamsBegin", line): + params=[] + while True: + line = f.readline() + if not line: + break + res+=line + if re.search("cgalNamedParamsEnd", line): + is_in_comment_section(line) + for np_name in np_names: + params_stack[np_name]=params + np_names=[] + break + m = re.search(r"cgalParamNBegin{\s*([^ ]+)\s*}", line) + if m: + params.append(m.group(1).strip()) + if not line: + stderr.write("ERROR: don't remember why I put that ("+argv[1]+")\n") + break + + + f.close() if modified: From 8e0edd5f30d1a4a234a948e628ab4234294b7ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Sun, 26 Oct 2025 11:06:01 +0100 Subject: [PATCH 14/25] handle np only --- .../add_checks_for_named_parameters.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/Scripts/developer_scripts/add_checks_for_named_parameters.py b/Scripts/developer_scripts/add_checks_for_named_parameters.py index 5b4d615fbfb5..7a1c1a70b8ba 100644 --- a/Scripts/developer_scripts/add_checks_for_named_parameters.py +++ b/Scripts/developer_scripts/add_checks_for_named_parameters.py @@ -28,6 +28,7 @@ def is_in_comment_section(line): params_stack={} np_names=[] +np_only={} while True: line = f.readline() if not line: @@ -59,10 +60,6 @@ def is_in_comment_section(line): modified=True break - - add_macro=True - macro_already_here=False - #do nothing if the macro is already there while True: line = f.readline() @@ -103,12 +100,29 @@ def is_in_comment_section(line): if re.search("cgalNamedParamsEnd", line): is_in_comment_section(line) for np_name in np_names: - params_stack[np_name]=params + if not np_only: + params_stack[np_name]=params + else: + #filter params + filtered_params=[] + for p in params: + if p in np_only.keys(): + if np_name!=np_only[p]: + continue; + filtered_params.append(p) + params_stack[np_name]=filtered_params np_names=[] + np_only={} break m = re.search(r"cgalParamNBegin{\s*([^ ]+)\s*}", line) if m: - params.append(m.group(1).strip()) + last_param=m.group(1).strip() + params.append(last_param) + else: + m = re.search(r"cgalParamExtra{\s*`([^ ]+)`\s+only}", line) + if m: + np_only[last_param]=m.group(1) + #print(last_param + " is only for "+m.group(1)) if not line: stderr.write("ERROR: don't remember why I put that ("+argv[1]+")\n") break From eba48ec7b1da971cb4c924c070ddc68e534191a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Sun, 26 Oct 2025 11:14:11 +0100 Subject: [PATCH 15/25] fix macro name --- .../Polygon_mesh_processing/internal/Snapping/snap.h | 12 ++++++------ .../internal/Snapping/snap_vertices.h | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 62c893b7f4e4..770fe4f695d5 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -1077,24 +1077,24 @@ std::size_t snap_non_conformal_one_way(const HalfedgeRange& halfedge_range_S, // \param np_A an optional sequence of \ref bgl_namedparameters "Named Parameters" among the ones listed below // // \cgalNamedParamsBegin -// \cgalParamBegin{vertex_point_map} +// \cgalParamNBegin{vertex_point_map} // the property map with the points associated to the vertices of the source mesh. // The type of this map is model of `ReadWritePropertyMap`. If this parameter is omitted, // an internal property map for `CGAL::vertex_point_t` must be available in `TriangleMesh`. -// \cgalParamEnd -// \cgalParamBegin{geom_traits} +// \cgalParamNEnd +// \cgalParamNBegin{geom_traits} // a geometric traits class instance, must be a model of `Kernel` -// \cgalParamEnd +// \cgalParamNEnd // \cgalNamedParamsEnd // // \param np_B an optional sequence of \ref bgl_namedparameters "Named Parameters" among the ones listed below // // \cgalNamedParamsBegin -// \cgalParamBegin{vertex_point_map} +// \cgalParamNBegin{vertex_point_map} // the property map with the points associated to the vertices of the target mesh. // The type of this map is model of `ReadablePropertyMap`. If this parameter is omitted, // an internal property map for `CGAL::vertex_point_t` must be available in `TriangleMesh`. -// \cgalParamEnd +// \cgalParamNEnd // \cgalNamedParamsEnd // // \return the number of snapped vertex pairs diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h index 5adbf99ff6f5..e2e38d004ff3 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h @@ -1215,24 +1215,24 @@ namespace experimental { // \param np_A an optional sequence of \ref bgl_namedparameters "Named Parameters" among the ones listed below // // \cgalNamedParamsBegin -// \cgalParamBegin{vertex_point_map} +// \cgalParamNBegin{vertex_point_map} // the property map with the points associated to the vertices of the source mesh. // The type of this map is model of `ReadWritePropertyMap`. If this parameter is omitted, // an internal property map for `CGAL::vertex_point_t` must be available in `PolygonMesh`. -// \cgalParamEnd -// \cgalParamBegin{geom_traits} +// \cgalParamNEnd +// \cgalParamNBegin{geom_traits} // a geometric traits class instance, must be a model of `Kernel` -// \cgalParamEnd +// \cgalParamNEnd // \cgalNamedParamsEnd // // \param np_B an optional sequence of \ref bgl_namedparameters "Named Parameters" among the ones listed below // // \cgalNamedParamsBegin -// \cgalParamBegin{vertex_point_map} +// \cgalParamNBegin{vertex_point_map} // the property map with the points associated to the vertices of the target mesh. // The type of this map is model of `ReadablePropertyMap`. If this parameter is omitted, // an internal property map for `CGAL::vertex_point_t` must be available in `PolygonMesh`. -// \cgalParamEnd +// \cgalParamNEnd // \cgalNamedParamsEnd // // \return the number of snapped vertex pairs From ea8d366db10a149d76163f3048c48a7cf73702f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 26 Jan 2026 07:51:43 +0100 Subject: [PATCH 16/25] use struct as np variable is not a constexpr --- .../include/CGAL/Named_function_parameters.h | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 2969874283c4..a2ab34dd30dc 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -589,35 +589,35 @@ bool is_tag_present(Tag_wrapper, Tag) return is_tag_present(Tag_wrapper(), Tag()); } -template -constexpr -bool authorized_options_rec(const Named_function_parameters&) -{ - if constexpr (std::is_same_v) - return true; - return is_tag_present(Tag_wrapper(), Tag()); -} +template +struct Authorized_options_rec; template -constexpr -bool authorized_options_rec(const Named_function_parameters& np) +struct Authorized_options_rec< Named_function_parameters, TagsAllowed...> { - if constexpr (std::is_same_v) - return true; - if (is_tag_present(Tag_wrapper(), Tag())) - return authorized_options_rec(static_cast(np)); - return false; -} + static constexpr bool value = + std::is_same_v || + (is_tag_present(Tag_wrapper(), Tag()) || Authorized_options_rec::value); + +}; + +template +struct Authorized_options_rec< Named_function_parameters, TagsAllowed...> +{ + static constexpr bool value = + std::is_same_v || is_tag_present(Tag_wrapper(), Tag()); +}; }// impl namespace -template +template constexpr -bool authorized_options(const Named_function_parameters& np) +bool authorized_options() { #ifndef CGAL_DISABLE_NAMED_FUNCTION_PARAMETERS_CHECKS - return authorized_parameters_impl::authorized_options_rec - (np); + using NP = cpp20::remove_cvref_t; + return authorized_parameters_impl::Authorized_options_rec + ::value; #else return true; #endif @@ -631,7 +631,7 @@ bool authorized_options(const Named_function_parameters& np) #define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) \ { \ using namespace ::CGAL::internal_np; \ -static_assert(::CGAL::parameters::authorized_options<__VA_ARGS__>(np)); \ +static_assert(::CGAL::parameters::authorized_options()); \ } #endif From 3733f17044e548a20f673f82a5753c91611d87b5 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 10:29:07 +0200 Subject: [PATCH 17/25] improve the CGAL named parameters API and implementation I have also added new tests. --- .../test_pmp_np_function.cpp | 28 +- .../include/CGAL/Named_function_parameters.h | 499 ++++++------------ .../internal/parameters_interface.h | 24 + .../STL_Extension/test_cgal_named_params.cpp | 181 +++++-- 4 files changed, 339 insertions(+), 393 deletions(-) diff --git a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_pmp_np_function.cpp b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_pmp_np_function.cpp index 4499c0b7bbb6..ce21db8ae42b 100644 --- a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_pmp_np_function.cpp +++ b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_pmp_np_function.cpp @@ -1,4 +1,7 @@ +#include #include +#include +#include #include #include @@ -6,9 +9,18 @@ #include +#include +#include +#include + +#include +#include +#include + +#include +#include #include #include -#include namespace CGAL { @@ -65,11 +77,15 @@ void my_function_with_named_parameters(PolygonMesh& mesh, const NamedParameters& VCM vcm_np = choose_parameter(get_parameter(np, internal_np::vertex_is_constrained)); + auto vcmap = choose_parameter(get_parameter(np, internal_np::vertex_color_map), + CGAL::dynamic_vertex_property_t{}, mesh, false); //demonstrates usage for those values. for(auto v : vertices(mesh)) { std::cout<<"vertex #"< Vertex_constrained_pmap; Vertex_constrained_pmap vcm_pmap(vcm); CGAL::my_function_with_named_parameters(sm); + + VCmap colors; + for(auto v : vertices(sm)) + colors[v] = false; + Vertex_constrained_pmap color_pmap(colors); + CGAL::my_function_with_named_parameters(sm, CGAL::parameters::vertex_is_constrained_map(vcm_pmap) + .vertex_color_map(color_pmap) .do_project(true)); + for(auto v : vertices(sm)) + assert(colors[v]); + return 0; } diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 6d29bfc39405..0ac50aa8aa0d 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -12,6 +12,7 @@ #ifndef CGAL_NAMED_FUNCTION_PARAMETERS_H #define CGAL_NAMED_FUNCTION_PARAMETERS_H +#include #ifndef CGAL_NO_STATIC_ASSERTION_TESTS #include #endif @@ -21,6 +22,7 @@ #include +#include #include #include @@ -43,25 +45,36 @@ namespace internal_np{ struct No_property {}; struct Param_not_found {}; +template +inline constexpr bool is_param_not_found_v = std::is_same_v, Param_not_found>; + enum all_default_t { all_default }; +// helper for getting references +template +T get_reference(const T& t) +{ + return t; +} + +template +T& get_reference(const std::reference_wrapper& r) +{ + return r.get(); +} + // define enum types and values for new named parameters -#define CGAL_add_named_parameter(X, Y, Z) \ - enum X { Y }; -#define CGAL_add_named_parameter_with_compatibility(X, Y, Z) \ - enum X { Y }; -#define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z) \ - enum X { Y }; -#define CGAL_add_named_parameter_with_compatibility_ref_only(X, Y, Z) \ +#define CGAL_add_named_parameter(X, Y, Z) \ enum X { Y }; -#define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) #include -#undef CGAL_add_named_parameter -#undef CGAL_add_named_parameter_with_compatibility -#undef CGAL_add_named_parameter_with_compatibility_cref_only -#undef CGAL_add_named_parameter_with_compatibility_ref_only -#undef CGAL_add_extra_named_parameter_with_compatibility +} // end namespace internal_np + +// forward-declaration of Named_function_parameters +template +struct Named_function_parameters; + +namespace internal_np { template struct Named_params_impl : Base { @@ -71,6 +84,11 @@ struct Named_params_impl : Base : Base(b) , v(v) {} + + constexpr decltype(auto) parameter(Tag) const noexcept { return v; } + static constexpr bool has_parameter(Tag) noexcept { return true; } + using Base::parameter; + using Base::has_parameter; }; // partial specialization for base class of the recursive nesting @@ -79,60 +97,23 @@ struct Named_params_impl { typename std::conditional::value, T, std::reference_wrapper >::type v; // copy of the parameter if copyable - Named_params_impl(const T& v) + constexpr Named_params_impl(const T& v) : v(v) {} + constexpr decltype(auto) parameter(Tag) const noexcept { return v; } + static constexpr bool has_parameter(Tag) noexcept { return true; } }; // Helper class to get the type of a named parameter pack given a query tag template struct Get_param; -template< typename T, typename Tag, typename Query_tag> -struct Get_param< Named_params_impl, Query_tag > -{ - typedef Param_not_found type; - typedef Param_not_found reference; -}; - -template< typename T, typename Tag, typename Base> -struct Get_param< Named_params_impl, Tag > -{ - typedef typename std::conditional::value, - T, std::reference_wrapper >::type type; - typedef typename std::conditional::value, - T, const T&>::type reference; -}; - -template< typename T, typename Tag> -struct Get_param< Named_params_impl, Tag > -{ - typedef typename std::conditional::value, - T, std::reference_wrapper >::type type; - typedef typename std::conditional::value, - T, const T&>::type reference; -}; - -template< typename T, typename Tag, typename Base> -struct Get_param< Named_params_impl, Tag, Base>, Tag > -{ - typedef std::reference_wrapper type; - typedef T& reference; -}; - -template< typename T, typename Tag> -struct Get_param< Named_params_impl, Tag, No_property>, Tag > -{ - typedef std::reference_wrapper type; - typedef T& reference; -}; - - -template< typename T, typename Tag, typename Base, typename Query_tag> -struct Get_param< Named_params_impl, Query_tag> +template +struct Get_param, Query_tag> { - typedef typename Get_param::type type; - typedef typename Get_param::reference reference; + using type = decltype(std::declval>().parameter(Query_tag{})); + using reference = + decltype(get_reference(std::declval>().parameter(Query_tag{}))); }; // helper to choose the default @@ -143,207 +124,97 @@ struct Lookup_named_param_def typedef typename internal_np::Get_param::reference NP_reference; typedef std::conditional_t< - std::is_same_v, + internal_np::is_param_not_found_v, D, NP_type> type; typedef std::conditional_t< - std::is_same_v, + internal_np::is_param_not_found_v, D&, NP_reference> reference; }; -// helper function to extract the value from a named parameter pack given a query tag -template -typename std::conditional::value, - T, std::reference_wrapper >::type -get_parameter_impl(const Named_params_impl& np, Tag) -{ - return np.v; -} - -template< typename T, typename Tag, typename Query_tag> -Param_not_found get_parameter_impl(const Named_params_impl&, Query_tag) -{ - return Param_not_found(); -} - -template< typename T, typename Tag> -typename std::conditional::value, - T, std::reference_wrapper >::type -get_parameter_impl(const Named_params_impl& np, Tag) -{ - return np.v; -} - -template -typename Get_param, Query_tag>::type -get_parameter_impl(const Named_params_impl& np, Query_tag tag) -{ -#ifndef CGAL_NO_STATIC_ASSERTION_TEST - static_assert(!std::is_same::value); -#endif - return get_parameter_impl(static_cast(np), tag); -} - - -// helper for getting references -template -const T& get_reference(const T& t) -{ - return t; -} - -template -T& get_reference(const std::reference_wrapper& r) -{ - return r.get(); -} - -// helper function to extract the reference from a named parameter pack given a query tag -template -typename std::conditional::value, - T, const T& >::type -get_parameter_reference_impl(const Named_params_impl& np, Tag) -{ - return get_reference(np.v); -} - -template< typename T, typename Tag, typename Query_tag> -Param_not_found -get_parameter_reference_impl(const Named_params_impl&, Query_tag) -{ - return Param_not_found(); -} - -template< typename T, typename Tag> -typename std::conditional::value, - T, const T& >::type -get_parameter_reference_impl(const Named_params_impl& np, Tag) -{ - return get_reference(np.v); -} - -template -T& -get_parameter_reference_impl(const Named_params_impl, Tag, Base>& np, Tag) -{ - return np.v.get(); -} +} // end of internal_np namespace -template< typename T, typename Tag> -T& -get_parameter_reference_impl(const Named_params_impl, Tag, No_property>& np, Tag) +template +constexpr decltype(auto) parameter_or([[maybe_unused]] const Named_function_parameters& np, + [[maybe_unused]] Query_tag tag, + [[maybe_unused]] U&& default_value) { - return np.v.get(); + return np.parameter_or(tag, std::forward(default_value)); } -template -typename Get_param, Query_tag>::reference -get_parameter_reference_impl(const Named_params_impl& np, Query_tag tag) +template +constexpr decltype(auto) parameter_or([[maybe_unused]] const Named_function_parameters& np, + [[maybe_unused]] Query_tag tag) { - static_assert(!std::is_same::value); - return get_parameter_reference_impl(static_cast(np), tag); + return np.template parameter_or(tag); } -} // end of internal_np namespace - -template -struct Named_function_parameters; - namespace parameters{ typedef Named_function_parameters Default_named_parameters; -Default_named_parameters -inline default_values(); +inline constexpr Default_named_parameters default_values(); // function to extract a parameter template -typename internal_np::Get_param, Query_tag>::type +constexpr decltype(auto) get_parameter(const Named_function_parameters& np, Query_tag tag) { - return internal_np::get_parameter_impl(static_cast&>(np), tag); + return np.parameter(tag); } template -typename internal_np::Get_param, Query_tag>::reference +constexpr decltype(auto) get_parameter_reference(const Named_function_parameters& np, Query_tag tag) { - return internal_np::get_parameter_reference_impl( - static_cast&>(np), - tag); + return internal_np::get_reference(np.parameter(tag)); } // Two parameters, non-trivial default value -template -D& choose_parameter(const internal_np::Param_not_found&, D& d) -{ - return d; -} - -template -const D& choose_parameter(const internal_np::Param_not_found&, const D& d) -{ - return d; -} - -template -D choose_parameter(const internal_np::Param_not_found&, D&& d) -{ - return std::forward(d); -} - -template -T& choose_parameter(T& t, D&) -{ - return t; -} - template -const T& choose_parameter(const T& t, const D&) -{ - return t; +constexpr decltype(auto) choose_parameter([[maybe_unused]] T&& t, [[maybe_unused]] D&& d) { + if constexpr (internal_np::is_param_not_found_v) { + return std::forward(d); + } else { + return std::forward(t); + } } // single parameter so that we can avoid a default construction -template -D choose_parameter(const internal_np::Param_not_found&) -{ - return D(); -} - template -const T& choose_parameter(const T& t) +constexpr decltype(auto) choose_parameter([[maybe_unused]] T&& t) { - return t; + if constexpr (internal_np::is_param_not_found_v) { + return D{}; + } else { + return std::forward(t); + } } // version with a dynamic property tag with initialization -template -auto -choose_parameter(const internal_np::Param_not_found&, Tag tag, Graph& graph, const V& default_value) -{ - return get(tag, graph, default_value); -} - -template -auto -choose_parameter(const internal_np::Param_not_found&, Tag tag, Graph& graph) -{ - return get(tag, graph); -} - template -const T& choose_parameter(const T& t, Tag, Graph&, const V&) -{ - return t; +constexpr decltype(auto) choose_parameter([[maybe_unused]] T&& t, + [[maybe_unused]] Tag tag, + [[maybe_unused]] Graph& graph, + [[maybe_unused]] const V& default_value) { + if constexpr (internal_np::is_param_not_found_v) { + return get(tag, graph, default_value); + } else { + return std::forward(t); + } } template -const T& choose_parameter(const T& t, Tag, Graph&) +constexpr decltype(auto) +choose_parameter([[maybe_unused]] T&& t, [[maybe_unused]] Tag tag, [[maybe_unused]] Graph& graph) { - return t; + if constexpr (internal_np::is_param_not_found_v) { + return get(tag, graph); + } else { + return std::forward(t); + } } } // parameters namespace @@ -366,62 +237,65 @@ struct Named_function_parameters typedef internal_np::Named_params_impl base; typedef Named_function_parameters self; - Named_function_parameters() : base(T()) {} - Named_function_parameters(const T& v) : base(v) {} - Named_function_parameters(const T& v, const Base& b) : base(v, b) {} + using base::parameter; + using base::has_parameter; + constexpr auto parameter(...) const { return internal_np::Param_not_found(); } + static constexpr bool has_parameter(...) { return false; } + + template + constexpr decltype(auto) parameter_or([[maybe_unused]] Query_tag tag, [[maybe_unused]] D&& default_value) const { + if constexpr (has_parameter(Query_tag())) { + return parameter(tag); + } else { + return std::forward(default_value); + } + } + + template + constexpr decltype(auto) parameter_or([[maybe_unused]] Query_tag tag) const { + if constexpr (has_parameter(Query_tag())) { + return parameter(tag); + } else { + return D{}; + } + } + + constexpr Named_function_parameters() : base(T()) {} + constexpr Named_function_parameters(const T& v) : base(v) {} + constexpr Named_function_parameters(const T& v, const Base& b) : base(v, b) {} // create the functions for new named parameters and the one imported boost // used to concatenate several parameters #define CGAL_add_named_parameter(X, Y, Z) \ template \ - Named_function_parameters \ - Z(const K& k) const \ + constexpr auto Z(const K& k) const \ { \ - typedef Named_function_parameters Params;\ + using Params = Named_function_parameters;\ return Params(k, *this); \ } #define CGAL_add_named_parameter_with_compatibility(X, Y, Z) \ + CGAL_add_named_parameter(X, Y, Z) +#define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) \ + CGAL_add_named_parameter(X, Y, Z) +#define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z)\ template \ - Named_function_parameters \ - Z(const K& k) const \ - { \ - typedef Named_function_parameters Params;\ - return Params(k, *this); \ - } -#define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z) \ - template \ - Named_function_parameters, \ - internal_np::X, self> \ - Z(const K& k) const \ + constexpr auto Z(const K& k) const \ { \ - typedef Named_function_parameters,\ - internal_np::X, self> Params; \ + using Params = \ + Named_function_parameters, \ + internal_np::X, self>; \ return Params(std::cref(k), *this); \ } #define CGAL_add_named_parameter_with_compatibility_ref_only(X, Y, Z) \ template \ - Named_function_parameters, \ - internal_np::X, self> \ - Z(K& k) const \ + constexpr auto Z(K& k) const \ { \ - typedef Named_function_parameters, \ - internal_np::X, self> Params; \ + using Params = \ + Named_function_parameters, \ + internal_np::X, self>; \ return Params(std::ref(k), *this); \ } -#define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) \ - template \ - Named_function_parameters \ - Z(const K& k) const \ - { \ - typedef Named_function_parameters Params;\ - return Params(k, *this); \ - } #include -#undef CGAL_add_named_parameter -#undef CGAL_add_named_parameter_with_compatibility -#undef CGAL_add_named_parameter_with_compatibility_cref_only -#undef CGAL_add_named_parameter_with_compatibility_ref_only -#undef CGAL_add_extra_named_parameter_with_compatibility // inject mesh specific named parameter functions #define CGAL_NP_BASE self @@ -433,14 +307,14 @@ struct Named_function_parameters #undef CGAL_NP_BUILD template - Named_function_parameters + constexpr Named_function_parameters combine(const Named_function_parameters& np) const { return Named_function_parameters(np.v,*this); } template - auto + constexpr auto combine(const Named_function_parameters& np, const NPS& ... nps) const { return Named_function_parameters(np.v,*this).combine(nps...); @@ -452,8 +326,7 @@ struct Named_function_parameters namespace parameters { -Default_named_parameters -inline default_values() +inline constexpr Default_named_parameters default_values() { return Default_named_parameters(); } @@ -470,111 +343,55 @@ template struct Boost_parameter_compatibility_wrapper { template - Named_function_parameters - operator()(const K& p) const - { - typedef Named_function_parameters Params; - return Params(p); - } - - template - Named_function_parameters - operator=(const K& p) const - { - typedef Named_function_parameters Params; - return Params(p); - } -}; - -template -struct Boost_parameter_compatibility_wrapper -{ - template - Named_function_parameters, Tag> - operator()(const K& p) const - { - typedef Named_function_parameters, Tag> Params; - return Params(std::cref(p)); - } - - template - Named_function_parameters, Tag> - operator=(const K& p) const - { - typedef Named_function_parameters, Tag> Params; - return Params(std::cref(p)); - } -}; - -template -struct Boost_parameter_compatibility_wrapper -{ - template - Named_function_parameters, Tag> - operator()(K& p) const + constexpr auto operator()(const K& p) const { - typedef Named_function_parameters, Tag> Params; - return Params(std::ref(p)); + if constexpr (ref_only) + { + if constexpr (ref_is_const) + { + using Params = Named_function_parameters, Tag>; + return Params(std::cref(p)); + } + else + { + using Params = Named_function_parameters, Tag>; + return Params(std::ref(p)); + } + } + else + { + using Params = Named_function_parameters; + return Params(p); + } } template - Named_function_parameters, Tag> - operator=(std::reference_wrapper p) const + constexpr auto operator=(const K& p) const { - typedef Named_function_parameters, Tag> Params; - return Params(std::ref(p)); + return operator()(p); } }; // define free functions and Boost_parameter_compatibility_wrapper for named parameters -#define CGAL_add_named_parameter(X, Y, Z) \ - template \ - Named_function_parameters \ - Z(const K& p) \ - { \ - typedef Named_function_parameters Params;\ - return Params(p); \ +#define CGAL_add_named_parameter(X, Y, Z) \ + template \ + constexpr auto Z(const K& p) { \ + using Params = Named_function_parameters; \ + return Params(p); \ } #define CGAL_add_named_parameter_with_compatibility(X, Y, Z) \ - inline const Boost_parameter_compatibility_wrapper Z; + inline constexpr Boost_parameter_compatibility_wrapper Z; #define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z) \ - inline const Boost_parameter_compatibility_wrapper Z; + inline constexpr Boost_parameter_compatibility_wrapper Z; #define CGAL_add_named_parameter_with_compatibility_ref_only(X, Y, Z) \ - inline const Boost_parameter_compatibility_wrapper Z; + inline constexpr Boost_parameter_compatibility_wrapper Z; #define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) \ - inline const Boost_parameter_compatibility_wrapper Z; + inline constexpr Boost_parameter_compatibility_wrapper Z; #include -#undef CGAL_add_named_parameter -#undef CGAL_add_extra_named_parameter_with_compatibility -#undef CGAL_add_named_parameter_with_compatibility -#undef CGAL_add_named_parameter_with_compatibility_cref_only -#undef CGAL_add_named_parameter_with_compatibility_ref_only - -// Version with three parameters for dynamic property maps -template -D choose_parameter(const internal_np::Param_not_found&, Dynamic_tag tag, PolygonMesh& pm) -{ - return get(tag, pm); -} - -template -const T& choose_parameter(const T& t, Dynamic_tag, PolygonMesh&) -{ - return t; -} template -struct is_default_parameter -{ - typedef typename internal_np::Lookup_named_param_def::type NP_type; - - static const bool value = std::is_same::value; - - typedef CGAL::Boolean_tag type; -}; +using is_default_parameter = Boolean_tag; } // end of parameters namespace diff --git a/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h b/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h index 22796bcb1311..838be6fdc424 100644 --- a/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h +++ b/STL_Extension/include/CGAL/STL_Extension/internal/parameters_interface.h @@ -9,6 +9,24 @@ // // Author(s) : Maxime Gimeno +#include + +#ifndef CGAL_add_named_parameter_with_compatibility +# define CGAL_add_named_parameter_with_compatibility(X, Y, Z) CGAL_add_named_parameter(X, Y, Z) +#endif + +#ifndef CGAL_add_named_parameter_with_compatibility_cref_only +# define CGAL_add_named_parameter_with_compatibility_cref_only(X, Y, Z) CGAL_add_named_parameter(X, Y, Z) +#endif + +#ifndef CGAL_add_named_parameter_with_compatibility_ref_only +# define CGAL_add_named_parameter_with_compatibility_ref_only(X, Y, Z) CGAL_add_named_parameter(X, Y, Z) +#endif + +#ifndef CGAL_add_extra_named_parameter_with_compatibility +# define CGAL_add_extra_named_parameter_with_compatibility(X, Y, Z) +#endif + // List of named parameters that we use in CGAL CGAL_add_named_parameter(vertex_point_t, vertex_point, vertex_point_map) CGAL_add_named_parameter(halfedge_index_t, halfedge_index, halfedge_index_map) @@ -456,3 +474,9 @@ CGAL_add_named_parameter(computation_policy_t, computation_policy, computation_p //List of named parameters used in Mesh_smoothing_3 CGAL_add_named_parameter(max_number_of_evaluations_t, max_number_of_evaluations, max_number_of_evaluations) + +#undef CGAL_add_named_parameter +#undef CGAL_add_named_parameter_with_compatibility +#undef CGAL_add_extra_named_parameter_with_compatibility +#undef CGAL_add_named_parameter_with_compatibility_cref_only +#undef CGAL_add_named_parameter_with_compatibility_ref_only diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index 4e6b703cb9b5..e67695eaf9ff 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -1,118 +1,197 @@ #include #include -#include +#include +#include #include +#include +#include namespace inp = CGAL::internal_np; namespace params = CGAL::parameters; template -struct A -{ - A(int v):v(v){} - int v; -}; +using Static_int = std::integral_constant; -struct B +struct Non_copyable { - B(){} - B(const B&) = delete; + int value = 0; + Non_copyable() = default; + Non_copyable(const Non_copyable&) = delete; }; template void check_same_type(T) { - static const bool b = std::is_same< A, T >::value; + static const bool b = std::is_same_v, T>; static_assert(b); assert(b); } -template -void test_values_and_types(const NamedParameters& np) +void test_values_and_types() { + auto np = params::vertex_index_map(Static_int<0>()) + .visitor(Static_int<1>()); using params::get_parameter; + using CGAL::parameter_or; // test values - assert(get_parameter(np, inp::vertex_index).v == 0); - assert(get_parameter(np, inp::visitor).v == 1); + assert(get_parameter(np, inp::vertex_index).value == 0); + assert(np.parameter(inp::vertex_index).value == 0); + assert(get_parameter(np, inp::visitor).value == 1); + assert(np.parameter(inp::visitor).value == 1); // test types check_same_type<0>(get_parameter(np, inp::vertex_index)); + check_same_type<0>(np.parameter(inp::vertex_index)); + check_same_type<0>(parameter_or(np, inp::vertex_index, Static_int<42>{})); + check_same_type<0>(parameter_or>(np, inp::vertex_index)); check_same_type<1>(get_parameter(np, inp::visitor)); + check_same_type<1>(np.parameter(inp::visitor)); + check_same_type<1>(parameter_or(np, inp::visitor, Static_int<42>{})); + check_same_type<1>(parameter_or>(np, inp::visitor)); + + auto v = parameter_or(np, inp::face_color_map, Static_int<42>{}); + assert(v.value == 42); + assert(v.value == np.parameter_or(inp::face_color_map, Static_int<42>{}).value); + + auto v2 = parameter_or>(np, inp::face_color_map); + assert(v2.value == 42); + assert(v2.value == np.parameter_or>(inp::face_color_map).value); } -template -void test_no_copyable(const NamedParameters& np) +void test_missing_parameters() { - typedef typename inp::Get_param::type NP_type; - static_assert(std::is_same >::value); + const auto np = params::default_values(); + using NamedParameters = decltype(np); + + static_assert(!NamedParameters::has_parameter(inp::vertex_index)); + static_assert(params::is_default_parameter::value); + static_assert(std::is_same_v); + + Static_int<4> fallback; + Static_int<4>& result = CGAL::parameter_or(np, inp::vertex_index, fallback); + assert(&result == &fallback); + + Static_int<4>& member_result = np.parameter_or(inp::vertex_index, fallback); + assert(&member_result == &fallback); - const A<4>& a = params::choose_parameter(params::get_parameter_reference(np, inp::edge_index), A<4>(4)); - assert(a.v==4); + auto default_constructed = CGAL::parameter_or>(np, inp::vertex_index); + assert(default_constructed.value == 42); } -template -void test_references(const NamedParameters& np) +void test_compatibility_aliases() { - typedef A<2> Default_type; - Default_type default_value(2); + auto np = params::default_values() + .seeds(Static_int<5>{}) + .time_limit(0.5) + .max_iteration_number(9); + + static_assert(!params::is_default_parameter::value); + static_assert(!params::is_default_parameter::value); + static_assert(!params::is_default_parameter::value); + assert(params::get_parameter(np, inp::seeds).value == 5); + assert(params::get_parameter(np, inp::maximum_running_time) == 0.5); + assert(params::get_parameter(np, inp::number_of_iterations) == 9); +} + +void test_no_copyable() +{ + Non_copyable b; + auto np = params::visitor(b); + using NamedParameters = decltype(np); + using NP_type = typename inp::Get_param::type; + static_assert(std::is_same_v>); + + const Static_int<4>& a = params::choose_parameter( + params::get_parameter_reference(np, inp::edge_index), Static_int<4>()); + assert(a.value == 4); +} + +void test_references() +{ + Non_copyable b; + auto v = Static_int<0>(); + auto np = params::visitor(std::ref(b)) + .vertex_point_map(b) + .vertex_index_map(v) + .face_index_map(std::cref(b)); + using NamedParameters = decltype(np); + using Default_type = Static_int<2>; + Default_type default_value; // std::reference_wrapper - typedef typename inp::Lookup_named_param_def::reference Visitor_reference_type; - static_assert(std::is_same::value); - Visitor_reference_type vis_ref = params::choose_parameter(params::get_parameter_reference(np, inp::visitor), default_value); + using Visitor_reference_type = + typename inp::Lookup_named_param_def::reference; + static_assert(std::is_same_v); + Visitor_reference_type vis_ref = + params::choose_parameter(params::get_parameter_reference(np, inp::visitor), default_value); CGAL_USE(vis_ref); + assert(&vis_ref == &b); + vis_ref.value = 42; + assert(b.value == 42); // std::reference_wrapper of const - typedef typename inp::Lookup_named_param_def::reference FIM_reference_type; - static_assert(std::is_same::value); - FIM_reference_type fim_ref = params::choose_parameter(params::get_parameter_reference(np, inp::face_index), default_value); + using FIM_reference_type = + typename inp::Lookup_named_param_def::reference; + static_assert(std::is_same_v); + FIM_reference_type fim_ref = + params::choose_parameter(params::get_parameter_reference(np, inp::face_index), default_value); CGAL_USE(fim_ref); + assert(&fim_ref == &b); // non-copyable - typedef typename inp::Lookup_named_param_def::reference VPM_reference_type; - static_assert(std::is_same::value); - VPM_reference_type vpm_ref = params::choose_parameter(params::get_parameter_reference(np, inp::vertex_point), default_value); + using VPM_reference_type = + typename inp::Lookup_named_param_def::reference; + static_assert(std::is_same_v); + VPM_reference_type vpm_ref = + params::choose_parameter(params::get_parameter_reference(np, inp::vertex_point), default_value); CGAL_USE(vpm_ref); + assert(&vpm_ref == &b); // passed by copy - typedef typename inp::Lookup_named_param_def::reference VIM_reference_type; - static_assert(std::is_same, VIM_reference_type>::value); - VIM_reference_type vim_ref = params::choose_parameter(params::get_parameter_reference(np, inp::vertex_index), default_value); + using VIM_reference_type = + typename inp::Lookup_named_param_def::reference; + static_assert(std::is_same_v, VIM_reference_type>); + VIM_reference_type vim_ref = + params::choose_parameter(params::get_parameter_reference(np, inp::vertex_index), default_value); CGAL_USE(vim_ref); + assert(&vim_ref != &v); // default - typedef typename inp::Lookup_named_param_def::reference EIM_reference_type; - static_assert(std::is_same::value); - EIM_reference_type eim_ref = params::choose_parameter(params::get_parameter_reference(np, inp::edge_index), default_value); - assert(&eim_ref==&default_value); + using EIM_reference_type = + typename inp::Lookup_named_param_def::reference; + static_assert(std::is_same_v); + EIM_reference_type eim_ref = + params::choose_parameter(params::get_parameter_reference(np, inp::edge_index), default_value); + assert(&eim_ref == &default_value); } int main() { - test_values_and_types(params::vertex_index_map(A<0>(0)).visitor(A<1>(1))); + test_values_and_types(); + + test_missing_parameters(); + + test_compatibility_aliases(); - B b; - test_no_copyable(params::visitor(b)); + test_no_copyable(); - test_references(params::visitor(std::ref(b)) - .vertex_point_map(b) - .vertex_index_map(A<0>(0)) - .face_index_map(std::reference_wrapper(b)) - ); + test_references(); // test that, in case of duplicates, the last parameter value is kept auto np = params::visitor(1).visitor(2); + static_assert(decltype(np)::has_parameter(inp::visitor)); assert(params::get_parameter(np, inp::visitor) == 2); + assert(CGAL::parameter_or(np, inp::visitor, 3) == 2); auto d = CGAL::parameters::default_values(); - static_assert(std::is_same::value); + static_assert(std::is_same_v); #ifndef CGAL_NO_DEPRECATED_CODE auto d1 = CGAL::parameters::all_default(); - static_assert(std::is_same::value); + static_assert(std::is_same_v); auto d2 = CGAL::Polygon_mesh_processing::parameters::all_default(); - static_assert(std::is_same::value); + static_assert(std::is_same_v); #endif return EXIT_SUCCESS; From e221fa65b8bafcbeb86735aebccf7184f8e9f12f Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 14:51:59 +0200 Subject: [PATCH 18/25] fix CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS --- .../include/CGAL/Named_function_parameters.h | 118 ++++++++++-------- .../STL_Extension/test_cgal_named_params.cpp | 37 ++++-- 2 files changed, 92 insertions(+), 63 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index d758e02aca7f..def3ba8390cf 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -12,16 +12,18 @@ #ifndef CGAL_NAMED_FUNCTION_PARAMETERS_H #define CGAL_NAMED_FUNCTION_PARAMETERS_H -#include +#include #ifndef CGAL_NO_STATIC_ASSERTION_TESTS #include #endif -#include #include +#include +#include #include +#include #include #include #include @@ -42,7 +44,10 @@ namespace CGAL { namespace internal_np{ -struct No_property {}; +struct No_property { + static const std::size_t number_of_parameters = 0; +}; + struct Param_not_found {}; template @@ -78,6 +83,10 @@ namespace internal_np { template struct Named_params_impl : Base { + static constexpr std::size_t number_of_parameters = 1 + Base::number_of_parameters; + using tag = Tag; + using base = Base; + typename std::conditional::value, T, std::reference_wrapper >::type v; // copy of the parameter if copyable constexpr Named_params_impl(const T& v, const Base& b) @@ -95,6 +104,9 @@ struct Named_params_impl : Base template struct Named_params_impl { + static constexpr std::size_t number_of_parameters = 1; + using tag = Tag; + typename std::conditional::value, T, std::reference_wrapper >::type v; // copy of the parameter if copyable constexpr Named_params_impl(const T& v) @@ -234,8 +246,11 @@ template struct Named_function_parameters : internal_np::Named_params_impl { - typedef internal_np::Named_params_impl base; - typedef Named_function_parameters self; + using base = internal_np::Named_params_impl; + using self = Named_function_parameters; + using tag = Tag; + + static constexpr std::size_t number_of_parameters = base::number_of_parameters; using base::parameter; using base::has_parameter; @@ -321,7 +336,7 @@ struct Named_function_parameters } // typedef for SFINAE - typedef int CGAL_Named_function_parameters_class; + using CGAL_Named_function_parameters_class = int; }; namespace parameters { @@ -398,51 +413,41 @@ using is_default_parameter = Boolean_tag -struct Tag_wrapper{}; +template +struct Wrapped_type { + static void id(); +}; -template -constexpr -bool is_tag_present(Tag_wrapper, Tag) -{ - return true; -} +template +inline constexpr auto meta = Wrapped_type::id; -template -constexpr -bool is_tag_present(Tag_wrapper, Tag) -{ - return false; -} +using Meta = std::remove_const_t)>; -template = 1>> -constexpr -bool is_tag_present(Tag_wrapper, Tag) +template +constexpr auto parameter_types() { - if (std::is_same_v) - return true; - else - return is_tag_present(Tag_wrapper(), Tag()); + std::array result{}; + if constexpr (result.size() > 0) { + result.back() = meta; + } + if constexpr (result.size() > 1) { + auto base_result = parameter_types(); + static_assert(base_result.size() + 1 == result.size()); + for(auto i = 0u; i < base_result.size(); ++i) { + result[i] = base_result[i]; + } + } + return result; } -template -struct Authorized_options_rec; - -template -struct Authorized_options_rec< Named_function_parameters, TagsAllowed...> +template +inline constexpr int find_index(const std::array& arr, Meta value) { - static constexpr bool value = - std::is_same_v || - (is_tag_present(Tag_wrapper(), Tag()) || Authorized_options_rec::value); - -}; - -template -struct Authorized_options_rec< Named_function_parameters, TagsAllowed...> -{ - static constexpr bool value = - std::is_same_v || is_tag_present(Tag_wrapper(), Tag()); -}; + for (std::size_t i = 0; i < N; ++i) { + if (arr[i] == value) return static_cast(i); + } + return -1; +} }// impl namespace @@ -451,9 +456,17 @@ constexpr bool authorized_options() { #ifndef CGAL_DISABLE_NAMED_FUNCTION_PARAMETERS_CHECKS - using NP = cpp20::remove_cvref_t; - return authorized_parameters_impl::Authorized_options_rec - ::value; + using NP = CGAL::cpp20::remove_cvref_t; + if constexpr(NP::has_parameter(internal_np::do_not_check_allowed_np_t{})) { + return true; + } + constexpr auto np_types = authorized_parameters_impl::parameter_types(); + constexpr std::array allowed_types{ authorized_parameters_impl::meta... }; + for(auto m : np_types) { + if(m == authorized_parameters_impl::meta) continue; + if(authorized_parameters_impl::find_index(allowed_types, m) == -1) return false; + } + return true; #else return true; #endif @@ -464,9 +477,14 @@ bool authorized_options() #ifdef CGAL_NDEBUG #define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) #else -#define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) \ -{ \ -using namespace ::CGAL::internal_np; \ +#define CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, ...) \ +{ \ +using namespace ::CGAL::internal_np; \ +using ::CGAL::internal_np::vertex_point_t; \ +using ::CGAL::internal_np::vertex_index_t; \ +using ::CGAL::internal_np::halfedge_index_t; \ +using ::CGAL::internal_np::edge_index_t; \ +using ::CGAL::internal_np::face_index_t; \ static_assert(::CGAL::parameters::authorized_options()); \ } #endif diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index d4705b7aaa28..3b9a549375b8 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -175,25 +175,36 @@ void test_authorized_options() auto np_permissive = np_ko.do_not_check_allowed_np(true); auto np_default = CGAL::parameters::default_values(); - - static_assert(CGAL::parameters::authorized_options(np_default)); - static_assert(CGAL::parameters::authorized_options()); + static_assert(CGAL::parameters::authorized_options(np_ok1)); - static_assert(CGAL::parameters::authorized_options()); + static_assert(CGAL::parameters::authorized_options(np_ok2)); - static_assert(!CGAL::parameters::authorized_options()); + static_assert(!CGAL::parameters::authorized_options(np_ko)); - static_assert(CGAL::parameters::authorized_options()); + static_assert(CGAL::parameters::authorized_options(np_permissive)); - static_assert(CGAL::parameters::authorized_options()); + static_assert(CGAL::parameters::authorized_options(np_ko.do_not_check_allowed_np(true))); + CGAL::internal_np::face_index_t>()); CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok1, vertex_point_t, edge_index_t, face_index_t); CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np_ok2, vertex_point_t, edge_index_t, face_index_t); From 73bb4de53598cc90300bf16e6d1bf3d84a542ada Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 15:48:47 +0200 Subject: [PATCH 19/25] NP: fix Boost_parameter_compatibility_wrapper --- .../include/CGAL/Named_function_parameters.h | 15 ++++++++------- .../test/STL_Extension/test_cgal_named_params.cpp | 9 +++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 0ac50aa8aa0d..a604e03985b5 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -343,25 +343,26 @@ template struct Boost_parameter_compatibility_wrapper { template - constexpr auto operator()(const K& p) const + constexpr auto operator()(K&& p) const { + using KK = CGAL::cpp20::remove_cvref_t; if constexpr (ref_only) { if constexpr (ref_is_const) { - using Params = Named_function_parameters, Tag>; - return Params(std::cref(p)); + using Params = Named_function_parameters, Tag>; + return Params{std::cref(std::forward(p))}; } else { - using Params = Named_function_parameters, Tag>; - return Params(std::ref(p)); + using Params = Named_function_parameters, Tag>; + return Params{std::ref(std::forward(p))}; } } else { using Params = Named_function_parameters; - return Params(p); + return Params{std::forward(p)}; } } @@ -377,7 +378,7 @@ struct Boost_parameter_compatibility_wrapper template \ constexpr auto Z(const K& p) { \ using Params = Named_function_parameters; \ - return Params(p); \ + return Params{p}; \ } #define CGAL_add_named_parameter_with_compatibility(X, Y, Z) \ diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index e67695eaf9ff..cf50398e1d42 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -13,6 +13,14 @@ namespace params = CGAL::parameters; template using Static_int = std::integral_constant; +void test_all_cgal_named_params() { + struct A{}; + A a; +#define CGAL_add_named_parameter(X, Y, Z) \ + (void)params::Z(a).Z(a); +#include +} + struct Non_copyable { int value = 0; @@ -169,6 +177,7 @@ void test_references() int main() { + test_all_cgal_named_params(); test_values_and_types(); test_missing_parameters(); From 98a1fe931cea5c0e9ae3dcec66d68d4edb5a8aa3 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 16:36:33 +0200 Subject: [PATCH 20/25] NP: fix Boost_parameter_compatibility_wrapper, again --- .../include/CGAL/Named_function_parameters.h | 6 +++--- STL_Extension/include/CGAL/type_traits.h | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index a604e03985b5..7e31d469c449 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -345,7 +345,7 @@ struct Boost_parameter_compatibility_wrapper template constexpr auto operator()(K&& p) const { - using KK = CGAL::cpp20::remove_cvref_t; + using KK = cpp20::remove_cvref_t>; if constexpr (ref_only) { if constexpr (ref_is_const) @@ -367,9 +367,9 @@ struct Boost_parameter_compatibility_wrapper } template - constexpr auto operator=(const K& p) const + constexpr auto operator=(K&& p) const { - return operator()(p); + return operator()(std::forward(p)); } }; diff --git a/STL_Extension/include/CGAL/type_traits.h b/STL_Extension/include/CGAL/type_traits.h index d28840945e43..4c894a96ff3a 100644 --- a/STL_Extension/include/CGAL/type_traits.h +++ b/STL_Extension/include/CGAL/type_traits.h @@ -15,6 +15,7 @@ #include #include +#include #include namespace CGAL { @@ -54,6 +55,21 @@ namespace cpp20 { template< class T > using remove_cvref_t = typename remove_cvref::type; + template + struct unwrap_reference { using type = T; }; + + template + struct unwrap_reference> { using type = U&; }; + + template + using unwrap_reference_t = typename unwrap_reference::type; + + template + struct unwrap_ref_decay : unwrap_reference> {}; + + template + using unwrap_ref_decay_t = typename unwrap_ref_decay::type; + } // end namespace cpp20 namespace details { From a9f990bb86583d592af896f206af200d0db8e723 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 17:54:25 +0200 Subject: [PATCH 21/25] NP: fix Boost_parameter_compatibility_wrapper --- STL_Extension/include/CGAL/Named_function_parameters.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 7e31d469c449..5610d422f7ee 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -351,12 +351,14 @@ struct Boost_parameter_compatibility_wrapper if constexpr (ref_is_const) { using Params = Named_function_parameters, Tag>; - return Params{std::cref(std::forward(p))}; + const auto& ref = cpp20::unwrap_reference_t(p); + return Params{std::cref(ref)}; } else { using Params = Named_function_parameters, Tag>; - return Params{std::ref(std::forward(p))}; + auto& ref = cpp20::unwrap_reference_t(p); + return Params{std::ref(ref)}; } } else From 41557b3241242b92ae7060d1c5f6bb68c754e87c Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Fri, 9 Oct 2026 10:24:43 +0200 Subject: [PATCH 22/25] check_headers now checks also check_headers_linked_twice --- Installation/CMakeLists.txt | 59 ++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/Installation/CMakeLists.txt b/Installation/CMakeLists.txt index dd0795da1402..4383c19b0903 100644 --- a/Installation/CMakeLists.txt +++ b/Installation/CMakeLists.txt @@ -830,6 +830,7 @@ if(CGAL_BRANCH_BUILD) find_package(GMP REQUIRED) find_package(Doxygen REQUIRED) find_package(Eigen3 REQUIRED) + include(CGAL_Eigen3_support) get_target_property(CGAL_EIGEN_3_INCLUDE_DIRS Eigen3::Eigen INTERFACE_INCLUDE_DIRECTORIES) find_package(Qt6 COMPONENTS Core Widgets OpenGL Gui REQUIRED) set(CMAKE_POLICY_DEFAULT_CMP0167 NEW) @@ -843,23 +844,18 @@ if(CGAL_BRANCH_BUILD) find_package(OpenMesh) find_package(OpenCV QUIET) - set(compile_options - "\ -${CMAKE_CXX_FLAGS} -DCGAL_EIGEN3_ENABLED -DCGAL_PROFILE \ -${Qt6Widgets_DEFINITIONS} ${Qt6OpenGL_DEFINITIONS} ${Qt6Gui_DEFINITIONS} \ -${Qt6OpenGL_EXECUTABLE_COMPILE_FLAGS} -fPIC \ -${Qt6Gui_EXECUTABLE_COMPILE_FLAGS} \ -${CGAL_3RD_PARTY_DEFINITIONS} ${CGAL_Qt6_3RD_PARTY_DEFINITIONS} \ -${CGAL_DEFINITIONS}" -std=c++17) + set(compile_options ${CMAKE_CXX_FLAGS} -DCGAL_EIGEN3_ENABLED -DCGAL_PROFILE ${Qt6Widgets_DEFINITIONS} + ${Qt6OpenGL_DEFINITIONS} ${Qt6Gui_DEFINITIONS} ${Qt6OpenGL_EXECUTABLE_COMPILE_FLAGS} -fPIC + ${Qt6Gui_EXECUTABLE_COMPILE_FLAGS} ${CGAL_3RD_PARTY_DEFINITIONS} ${CGAL_Qt6_3RD_PARTY_DEFINITIONS} + ${CGAL_DEFINITIONS} -std=c++17) message("COMPILATION OPTIONS ARE : ${compile_options}") if(NOT OpenMesh_FOUND) - set(compile_options - "${compile_options} \ --DCGAL_BOOST_GRAPH_GRAPH_TRAITS_POLYMESH_ARRAYKERNELT_H \ --DCGAL_BOOST_GRAPH_GRAPH_TRAITS_TRIMESH_ARRAYKERNELT_H \ --DCGAL_PROPERTIES_POLYMESH_ARRAYKERNELT_H \ --DCGAL_PROPERTIES_TRIMESH_ARRAYKERNELT_H") + list(APPEND compile_options + -DCGAL_BOOST_GRAPH_GRAPH_TRAITS_POLYMESH_ARRAYKERNELT_H + -DCGAL_BOOST_GRAPH_GRAPH_TRAITS_TRIMESH_ARRAYKERNELT_H + -DCGAL_PROPERTIES_POLYMESH_ARRAYKERNELT_H + -DCGAL_PROPERTIES_TRIMESH_ARRAYKERNELT_H) message( STATUS "Skip headers \ @@ -870,17 +866,17 @@ ${CGAL_DEFINITIONS}" -std=c++17) because OpenMesh_FOUND is false.") endif() if(NOT OpenCV_FOUND) - set(compile_options "${compile_options} \ --DCGAL_CLASSIFICATION_OPENCV_RANDOM_FOREST_CLASSIFIER_H") + list(APPEND compile_options + -DCGAL_CLASSIFICATION_OPENCV_RANDOM_FOREST_CLASSIFIER_H) message(STATUS "Skip header \ \"CGAL/Classification/OpenCV_random_forest_classifier.h\" \ because OpenCV_FOUND is false") endif() if(LEDA_FOUND) - set(compile_options "${compile_options} -DCGAL_USE_LEDA") + list(APPEND compile_options -DCGAL_USE_LEDA) endif() if(NOT IPE_FOUND) - set(compile_options "${compile_options} -DCGAL_IPELET_BASE_H=1") + list(APPEND compile_options -DCGAL_IPELET_BASE_H=1) message(STATUS "Skip header \"CGAL/CGAL_ipelet_base.h\" \ because IPE_FOUND is false.") endif() @@ -929,7 +925,6 @@ You must disable CGAL_ENABLE_CHECK_HEADERS.") ${GMP_INCLUDE_DIR} ${Qt6OpenGL_INCLUDE_DIRS} ${Qt6Widgets_INCLUDE_DIRS} - ${Qt6Gui_DEFINITIONS} ${CGAL_3RD_PARTY_INCLUDE_DIRS} ${CGAL_Qt6_3RD_PARTY_INCLUDE_DIRS}) list(APPEND include_options "-I${incdir}") @@ -1062,6 +1057,7 @@ ${CMAKE_CURRENT_SOURCE_DIR}/../${package}/include/${header}" endif() # end the if on list_of_documented_headers endforeach() # look on headers add_custom_target(check_pkg_${package}_headers + COMMENT "Check headers for package ${package}" DEPENDS ${check_pkg_headers_depends}) add_custom_command( OUTPUT ${CGAL_BINARY_DIR}/package_info/${package}/included_headers @@ -1078,6 +1074,7 @@ ${CMAKE_CURRENT_SOURCE_DIR}/../${package}/include/${header}" ${check_pkg_headers_depends}) add_custom_target( pkg_${package}_deps + COMMENT "Compute dependencies for package ${package}" DEPENDS ${CGAL_BINARY_DIR}/package_info/${package}/dependencies) list(APPEND check_pkg_target_list check_pkg_${package}_headers) list(APPEND packages_deps @@ -1095,26 +1092,28 @@ ${CMAKE_CURRENT_SOURCE_DIR}/../${package}/include/${header}" file(COPY_FILE ${CGAL_BINARY_DIR}/test_headers.cpp ${CGAL_BINARY_DIR}/test_headers2.cpp) file(APPEND ${CGAL_BINARY_DIR}/test_headers.cpp "int main(){}\n") - add_custom_target(check_headers DEPENDS ${check_pkg_target_list}) + # New check that headers can be included in different compilation units, that can be linked + # together. That checks for missing `inline` keywords. + add_executable(check_headers_linked_twice + ${CGAL_BINARY_DIR}/test_headers.cpp + ${CGAL_BINARY_DIR}/test_headers2.cpp) + target_link_libraries(check_headers_linked_twice PRIVATE CGAL::CGAL CGAL::CGAL_Qt6 Qt6::Widgets CGAL::Eigen3_support) + target_compile_options(check_headers_linked_twice PRIVATE ${compile_options} ${include_options}) + if(VTK_FOUND) + target_link_libraries(check_headers_linked_twice PRIVATE VTK::IOImage) + endif() + + add_custom_target(check_headers DEPENDS ${check_pkg_target_list} check_headers_linked_twice) string(REPLACE " " ";" compile_options ${compile_options}) string(REPLACE " " ";" include_options ${include_options_str}) add_custom_command( TARGET check_headers + COMMENT "Check that all headers can be included in the same translation unit" POST_BUILD COMMAND ${CMAKE_CXX_COMPILER} ${compile_options} ${include_options} -x c++ -fsyntax-only ${CGAL_BINARY_DIR}/test_headers.cpp) - # New check that headers can be included in different compilation units, that can be linked - # together. That checks for missing `inline` keywords. - add_executable(check_headers_linked_twice - ${CGAL_BINARY_DIR}/test_headers.cpp - ${CGAL_BINARY_DIR}/test_headers2.cpp) - target_link_libraries(check_headers_linked_twice PRIVATE CGAL::CGAL CGAL::CGAL_Qt6 Qt6::Widgets) - if(VTK_FOUND) - target_link_libraries(check_headers_linked_twice PRIVATE VTK::IOImage) - endif() - target_compile_options(check_headers_linked_twice PRIVATE ${compile_options} ${include_options}) add_custom_target(packages_dependencies DEPENDS ${packages_deps}) message( From ae3e09dae86941ba2537a53922777d500ebafe52 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Fri, 9 Oct 2026 10:25:13 +0200 Subject: [PATCH 23/25] test and fix ref_only and cref_only with more cases --- .../include/CGAL/Named_function_parameters.h | 12 ++++++--- .../STL_Extension/test_cgal_named_params.cpp | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/STL_Extension/include/CGAL/Named_function_parameters.h b/STL_Extension/include/CGAL/Named_function_parameters.h index 5610d422f7ee..e89472498a48 100644 --- a/STL_Extension/include/CGAL/Named_function_parameters.h +++ b/STL_Extension/include/CGAL/Named_function_parameters.h @@ -242,6 +242,12 @@ struct Named_function_parameters constexpr auto parameter(...) const { return internal_np::Param_not_found(); } static constexpr bool has_parameter(...) { return false; } + template + constexpr decltype(auto) parameter_ref([[maybe_unused]] Query_tag tag) const { + static_assert(has_parameter(Query_tag()), "Parameter not found"); + return internal_np::get_reference(parameter(tag)); + } + template constexpr decltype(auto) parameter_or([[maybe_unused]] Query_tag tag, [[maybe_unused]] D&& default_value) const { if constexpr (has_parameter(Query_tag())) { @@ -345,18 +351,18 @@ struct Boost_parameter_compatibility_wrapper template constexpr auto operator()(K&& p) const { - using KK = cpp20::remove_cvref_t>; if constexpr (ref_only) { + using Pointed_type = std::remove_reference_t>; if constexpr (ref_is_const) { - using Params = Named_function_parameters, Tag>; + using Params = Named_function_parameters, Tag>; const auto& ref = cpp20::unwrap_reference_t(p); return Params{std::cref(ref)}; } else { - using Params = Named_function_parameters, Tag>; + using Params = Named_function_parameters, Tag>; auto& ref = cpp20::unwrap_reference_t(p); return Params{std::ref(ref)}; } diff --git a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp index cf50398e1d42..95959f8d3fa5 100644 --- a/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp +++ b/STL_Extension/test/STL_Extension/test_cgal_named_params.cpp @@ -175,6 +175,29 @@ void test_references() assert(&eim_ref == &default_value); } + +void test_ref_only_parameters() +{ + int i = 42; + auto np_ref_only_i = params::weights(i).weights(i); + auto& ref_i = np_ref_only_i.parameter_ref(CGAL::internal_np::weights_param_t{}); + assert(&ref_i == &i); + + const int ci = 43; + auto np_cref_only_ci = params::image(ci).image(ci); + auto& ref_ci = np_cref_only_ci.parameter_ref(CGAL::internal_np::image_3_param_t{}); + assert(&ref_ci == &ci); + + auto np_ref_only_ci = params::weights(ci).weights(ci); + auto& ref_ci2 = np_ref_only_ci.parameter_ref(CGAL::internal_np::weights_param_t{}); + assert(&ref_ci2 == &ci); + + auto np_cref_only_2 = params::image(2*3).image(2*3); + auto& ref_2 = np_cref_only_2.parameter_ref(CGAL::internal_np::image_3_param_t{}); + static_assert(std::is_const_v>); + assert(np_cref_only_2.parameter_ref(CGAL::internal_np::image_3_param_t{}) == 6); +} + int main() { test_all_cgal_named_params(); @@ -188,6 +211,8 @@ int main() test_references(); + test_ref_only_parameters(); + // test that, in case of duplicates, the last parameter value is kept auto np = params::visitor(1).visitor(2); static_assert(decltype(np)::has_parameter(inp::visitor)); From 5c0c0d4afdb0826f380c1332217227053aa98624 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 8 Oct 2026 16:50:29 +0200 Subject: [PATCH 24/25] KSP: fix checks with CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS --- .../include/CGAL/Kinetic_space_partition_3.h | 3 ++- .../include/CGAL/Kinetic_surface_reconstruction_3.h | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h b/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h index d5aff68c351a..5d5c66bc100e 100644 --- a/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h +++ b/Kinetic_space_partition/include/CGAL/Kinetic_space_partition_3.h @@ -403,7 +403,8 @@ class Kinetic_space_partition_3 { const PolygonRange& polygons, const NamedParameters& np = CGAL::parameters::default_values()) { - CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t); + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t, debug_t, verbose_t, reorient_bbox_t, + bbox_dilation_ratio_t, max_octree_depth_t, max_octree_node_size_t); using NP_helper = Point_set_processing_3_np_helper; using PointMap = typename NP_helper::Point_map; diff --git a/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h b/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h index 5423d24848e8..7901d3e08913 100644 --- a/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h +++ b/Kinetic_surface_reconstruction/include/CGAL/Kinetic_surface_reconstruction_3.h @@ -165,7 +165,7 @@ class Kinetic_surface_reconstruction_3 { Kinetic_surface_reconstruction_3(Point_range& points, const NamedParameters& np = CGAL::parameters::default_values()) : m_max_distance_to_plane(-1), m_points(points), m_ground_polygon_index(-1), m_kinetic_partition(np), m_bbox_diagonal(-1), m_lcc(m_lcc_from_kinetic_partition) { - CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t); + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, point_t, normal_t, verbose_t, debug_t); m_verbose = parameters::choose_parameter(parameters::get_parameter(np, internal_np::verbose), false); m_debug = parameters::choose_parameter(parameters::get_parameter(np, internal_np::debug), false); @@ -557,7 +557,8 @@ class Kinetic_surface_reconstruction_3 { */ template void partition(std::size_t k, const CGAL_NP_CLASS& np = parameters::default_values()) { - CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, reorient_bbox_t, bbox_dilation_ratio_t); + CGAL_CHECK_AUTHORIZED_NAMED_PARAMETERS(np, reorient_bbox_t, bbox_dilation_ratio_t, point_t, normal_t, verbose_t, + debug_t, max_octree_depth_t, max_octree_node_size_t); if (m_regions.empty()) return; From 1e194496ca92486ac173d24ee50a92843afb1712 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Fri, 9 Oct 2026 13:37:47 +0200 Subject: [PATCH 25/25] fix -fsyntax-only --- Installation/CMakeLists.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Installation/CMakeLists.txt b/Installation/CMakeLists.txt index 4383c19b0903..62339554756b 100644 --- a/Installation/CMakeLists.txt +++ b/Installation/CMakeLists.txt @@ -848,7 +848,7 @@ if(CGAL_BRANCH_BUILD) ${Qt6OpenGL_DEFINITIONS} ${Qt6Gui_DEFINITIONS} ${Qt6OpenGL_EXECUTABLE_COMPILE_FLAGS} -fPIC ${Qt6Gui_EXECUTABLE_COMPILE_FLAGS} ${CGAL_3RD_PARTY_DEFINITIONS} ${CGAL_Qt6_3RD_PARTY_DEFINITIONS} ${CGAL_DEFINITIONS} -std=c++17) - message("COMPILATION OPTIONS ARE : ${compile_options}") + message("COMPILATION OPTIONS ARE: ${compile_options}") if(NOT OpenMesh_FOUND) list(APPEND compile_options @@ -1105,14 +1105,13 @@ ${CMAKE_CURRENT_SOURCE_DIR}/../${package}/include/${header}" add_custom_target(check_headers DEPENDS ${check_pkg_target_list} check_headers_linked_twice) - string(REPLACE " " ";" compile_options ${compile_options}) string(REPLACE " " ";" include_options ${include_options_str}) add_custom_command( TARGET check_headers COMMENT "Check that all headers can be included in the same translation unit" POST_BUILD COMMAND ${CMAKE_CXX_COMPILER} ${compile_options} ${include_options} -x - c++ -fsyntax-only ${CGAL_BINARY_DIR}/test_headers.cpp) + c++ -fsyntax-only "${CGAL_BINARY_DIR}/test_headers.cpp") add_custom_target(packages_dependencies DEPENDS ${packages_deps})