Add unified rfl::Settings::with<>() API for format-specific settings#662
Add unified rfl::Settings::with<>() API for format-specific settings#662Centimo wants to merge 2 commits intogetml:mainfrom
Conversation
Introduce RFL_SETTINGS_OPS(Derived) macro that injects two with<>() overloads into a settings struct: with<&T::field>(value) (by pointer-to-member) and with<"field">(value) (by name). Both return a copy with the chosen const field replaced. Migrate csv::Settings and parquet::Settings off ad-hoc with_*() methods and onto the unified API. The old per-field with_delimiter() / with_compression() / etc. are removed; tests are updated. Implementation details: - rfl::internal::field_index_v<T, FieldPtr> recovers the index of a data member by comparing &(fake_object<T>().*FieldPtr) to the addresses of the structured-binding fields, all in a consteval context. - rfl::internal::field_index_by_name_v<T, Name> recovers the index by walking field names through the existing __PRETTY_FUNCTION__ parser. - Both indices are then mapped back to a fake-object pointer of the shape the existing get_field_name_str_lit expects (i.e. the form &fake.field, which MSVC parses correctly), so the rest of the machinery is unchanged. - get_fake_object.hpp switches from a `static const wrapper<T>` member to an `extern const fake_object_holder<T>` variable template, matching Boost.PFR. This lets GCC fold &(fake.*FieldPtr) inside a consteval function without requiring the symbol at link time. CRTP via inheritance from rfl::Settings<Derived> is intentionally avoided: an empty base class confuses num_fields (counted as a slot, but invisible to structured bindings), which would break the fake-object machinery for any settings struct that uses the macro. The macro keeps the struct a flat aggregate. Add tests/cli/test_settings_macro.cpp covering single replace, chained replace by PTM, chained replace by name, mixed PTM+name chains, and string move semantics.
Replace the removed with_delimiter() / with_compression() / etc. snippets with the unified with<&T::field>(value) form, and show the equivalent with<"field">(value) variant.
There was a problem hiding this comment.
Code Review
This pull request introduces a unified with<>() accessor for settings structs via the RFL_SETTINGS_OPS macro, supporting both pointer-to-member and string literal template arguments. This replaces individual with_field methods in CSV and Parquet settings and promotes immutability by transitioning fields to be const. The review feedback identifies that the string-literal overload of with<>() does not currently enforce the const requirement on fields, unlike the pointer-to-member version, and suggests adding a static_assert to ensure consistent enforcement of immutability.
| constexpr std::size_t I = field_index_by_name_v<T, Name>; | ||
| static_assert(I != static_cast<std::size_t>(-1), | ||
| "No field with the given name exists in T."); | ||
| return rfl::replace(_obj, rfl::make_field<Name>(std::move(_value))); |
There was a problem hiding this comment.
The with<"name">() overload doesn't enforce that fields in a Settings struct must be const, unlike the pointer-to-member overload with<&T::field>(). This could lead to mutable settings fields being used, which goes against the design goal of this PR.
To ensure consistency and enforce immutability for all Settings fields, I suggest adding a static_assert to check that the field is const.
constexpr std::size_t I = field_index_by_name_v<T, Name>;
static_assert(I != static_cast<std::size_t>(-1),
"No field with the given name exists in T.");
using FieldTypeWithCv = std::remove_pointer_t<
decltype(get_ith_field_from_fake_object<T, static_cast<int>(I)>())>;
static_assert(std::is_const_v<FieldTypeWithCv>,
"Fields in Settings structs must be const.");
return rfl::replace(_obj, rfl::make_field<Name>(std::move(_value)));
Summary
Replaces ad-hoc
with_delimiter()/with_compression()/ etc. accessors oncsv::Settingsandparquet::Settingswith a single uniformwith<>()accessor injected by theRFL_SETTINGS_OPS(Derived)macro. Two overloads are provided:with<&T::field>(value)— pointer-to-member.with<"field">(value)— field name as string literal.Both return a copy with the chosen
constfield replaced. Old per-field accessors are removed; existing tests are updated.Motivation
The previous design required hand-written
with_*()per field, growing linearly with each new option. The new macro replaces that boilerplate with a single line, makes adding fields a one-liner, and is consistent across formats.Const fields
All data members in
csv::Settingsandparquet::Settingsare now declaredconst. Settings are intended to be configured once and consumed; preventing accidental in-place mutation makes that intent explicit at the type level. Thewith<>()accessor returns a fresh copy with one field replaced, so the configurable surface is unchanged.RFL_SETTINGS_OPSenforces this — it rejects pointers-to-non-const members, so any future field added withoutconstis a compile error rather than silent regression.Implementation notes
field_index_v<T, FieldPtr>recovers the field index in aconstevalcontext by comparing&(fake_object<T>().*FieldPtr)against the addresses of the structured-binding fields. The index then feeds back into the existing fake-object name-lookup path (form&fake.field), which MSVC parses correctly — direct PTM literals do not.field_index_by_name_v<T, Name>does the analogous lookup for the string-literal overload.get_fake_object.hppswitches from astatic constmember of a struct template to anextern constvariable template (matching Boost.PFR). Without this, GCC requires the fake-object symbol at link time when evaluating&(fake.*FieldPtr)inconsteval.Tests
tests/cli/test_settings_macro.cppcovers single-field replace, chained replace by PTM, chained replace by name, mixed PTM+name chains, and string-move semantics. Existing csv/parquet tests are updated to use the new API. JSON 214/214 and CLI 46/47 pass locally (1 skipped is an unrelated locale test).