Add unified rfl::Settings::with<>() API for format-specific settings by Centimo · Pull Request #662 · getml/reflect-cpp

Centimo · 2026-05-09T18:43:34Z

Summary

Replaces ad-hoc with_<field>() accessors on csv::Settings and
parquet::Settings with a single uniform with<>() API injected by
the RFL_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 const field replaced.

The old per-field setters (with_delimiter(), with_compression(),
etc.) are kept as [[deprecated]] shims pointing at the new API, so
existing user code keeps compiling with a clear migration warning.

Motivation

The previous design required a hand-written with_*() per field,
growing linearly with each new option. The new macro replaces that
boilerplate with a single line and is consistent across formats.

Const fields

All data members in csv::Settings and parquet::Settings are now
declared const. Settings are intended to be configured once and
consumed; preventing accidental in-place mutation makes that intent
explicit at the type level. with<>() returns a fresh copy, so the
configurable surface is unchanged. The macro's PTM overload carries
a requires const_member_of<FieldPtr, Derived> clause, so a call
with a non-const field fails at the call site, not deep inside the
implementation.

Implementation notes

field_index_v<T, FieldPtr> recovers the field index in a
consteval context. On GCC/Clang it uses the existing fake-object
approach (&(fake_object<T>().*FieldPtr) vs structured-binding
addresses). MSVC refuses to constant-fold a PTM-deref through an
extern fake object (C2131), so on MSVC a local default-initialised
T{} is constructed inside consteval instead. Both paths share
a single base helper, field_extractor<T, n>::get_ptr<I>(obj).
field_index_by_name_v<T, Name> does the analogous lookup for the
string-literal overload, walking field names through the existing
__PRETTY_FUNCTION__ parser.
get_fake_object.hpp is unchanged from upstream (static const
wrapper member); the temporary extern const variant tried during
development was rolled back once the local-object MSVC path made
it unnecessary.

Tests

tests/cli/test_settings_macro.cpp covers 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. CI: all targets green except known-
unrelated infra failures (vcpkg/conan).

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.

gemini-code-assist

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.

gemini-code-assist · 2026-05-09T18:46:58Z

+  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)));


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)));

MSVC refused to constant-fold &(get_fake_object<T>().*FieldPtr) inside a consteval context (the extern-const fake-object pattern is well-defined on GCC/Clang but not on MSVC). The PTM-to-index lookup was the only call site that did this PTM-deref through the fake object. Reshape field_extractor around a single base function that takes the source object: get_ptr<I>(const T& obj). field_index_from_ptm_impl now constructs a local T{} and compares &(obj.*FieldPtr) against get_ith_field_ptr<T, Is>(obj), so no extern-const symbol is dereferenced. get_ith_field_from_fake_object becomes a thin wrapper over get_ith_field_ptr passing get_fake_object<T>(), preserving its public signature and behaviour for name-lookup callers. Also revert get_fake_object.hpp to its pre-PR static-member form (the extern-const variant was added for a fake-object PTM-deref path that no longer exists), and enforce that Settings fields named via with<"name">() are const.

The previous static_assert in settings_with_replaced_by_name extracted the field type via get_ith_field_from_fake_object, which calls get_ptr with a const T& source. Structured-binding `const auto& [...]` over a const object always yields const-qualified references, so the deduced field type was always `const FieldT` regardless of how the field was declared. The assert was vacuously true and never fired. Reshape get_ptr around `auto& [...]` over a cv-templated source object, so field references inherit the source's cv. Calls through get_fake_object<T>() (const T&) keep producing const FieldT* — same behaviour for name-lookup and PTM-index callers. A new helper ith_field_is_const<T, I>() builds a non-const local T and inspects the deduced field type, correctly distinguishing const from non-const fields. settings_with_replaced_by_name now uses it. Verified: a settings struct with a non-const field now triggers `Fields in Settings structs must be const.` at the with<"name">() call site.

Two diagnostics moved from deep inside the library to the with<>() call: - Propagate const_member_of into the macro's PTM overload. A call like .with<&T::non_const_field>(value) now fails overload resolution at the call site with a clear "no matching function" diagnostic citing the unsatisfied requires-clause, instead of a deep concept failure inside settings_with_replaced. - Add a static_assert in field_index_from_ptm_impl that T is default-constructible. The local T{} construction in consteval already required this; the assert produces an actionable message instead of a generic constant-expression failure.

The alias strips cv from the deduced field type, producing a value type suitable as a function parameter or local variable. The old name read as "the field's type" — misleading, since callers cannot use it to inspect the original field's const-ness. Rename to field_value_type_at(_t) so the name matches what the alias actually yields. No behavioural change. Two call sites in Settings.hpp updated; nothing else used the alias.

liuzicheng1987 · 2026-05-10T11:02:31Z

Hi @Centimo, I think this is certainly a step in the right direction, but I am always hesitant about API-breaking changes like this, because they always tend to frustrate people.

As a compromise, could you reinsert the old functions, add a comment in the code that they are deprecated and will be removed at a later date? Since the markdown documentation only includes the new style, most people will start using that. When we add new functions it will only be in the new style.

What do you think about that?

The local-T{} path in field_index_from_ptm_impl requires every field of T to be a constexpr literal type. libstdc++ in GCC 11 does not give std::string a constexpr default constructor, so any settings struct with a std::string member fails to instantiate with "temporary of non-literal type 'const string' in a constant expression". The local object was only needed because MSVC refuses to constant-fold &(extern_fake_object.*FieldPtr). Restore the fake-object path on GCC and Clang (works for any field type), keep the local-object path under #ifdef _MSC_VER. ith_field_is_const has the same constraint, so the const-on-name-path check is also MSVC-only; on GCC/Clang the assignment into a const member through rfl::replace fails, providing a (less focussed) diagnostic.

Centimo · 2026-05-10T15:30:31Z

Hi @liuzicheng1987

As a compromise, could you reinsert the old functions?

Do you mean the old with_* functions?

liuzicheng1987 · 2026-05-10T15:33:49Z

@Centimo, yes.

I think that people use them in the productive system and when you have to change that unexpectedly, that makes people angry.

So we should keep them for the time being but add a deprecation notice, because I do agree that your approach is better.

The with<>() unification removed per-field setters from csv::Settings and parquet::Settings outright. Reintroduce them so existing user code keeps compiling, marked [[deprecated]] with a message that points at the new with<&Settings::field>(value) / with<"field">(value) API. The repeated attribute is generated by a small RFL_DEPRECATED_WITH(name) macro placed in a new internal header (rfl/internal/deprecated_with.hpp) shared by both Settings files.

Centimo · 2026-05-10T18:13:31Z

@liuzicheng1987 done

liuzicheng1987 · 2026-05-10T19:01:48Z

@Centimo, all right, thank you very much for the contribution. I am merging.

Centimo added 2 commits May 9, 2026 21:08

docs: update csv and parquet Settings examples for the new with<>() API

5c17f35

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.

gemini-code-assist Bot reviewed May 9, 2026

View reviewed changes

Centimo added 4 commits May 9, 2026 23:07

liuzicheng1987 merged commit dc1af1b into getml:main May 10, 2026
181 of 182 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unified rfl::Settings::with<>() API for format-specific settings#662

Add unified rfl::Settings::with<>() API for format-specific settings#662
liuzicheng1987 merged 8 commits into
getml:mainfrom
Centimo:feature/settings-with-api

Centimo commented May 9, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 9, 2026

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Centimo commented May 10, 2026 •

edited

Loading

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Centimo commented May 10, 2026

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Centimo commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Const fields

Implementation notes

Tests

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Centimo commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Centimo commented May 10, 2026

Uh oh!

liuzicheng1987 commented May 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Centimo commented May 9, 2026 •

edited

Loading

Centimo commented May 10, 2026 •

edited

Loading