Skip to content

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

Open
Centimo wants to merge 2 commits intogetml:mainfrom
Centimo:feature/settings-with-api
Open

Add unified rfl::Settings::with<>() API for format-specific settings#662
Centimo wants to merge 2 commits intogetml:mainfrom
Centimo:feature/settings-with-api

Conversation

@Centimo
Copy link
Copy Markdown
Contributor

@Centimo Centimo commented May 9, 2026

Summary

Replaces ad-hoc with_delimiter() / with_compression() / etc. accessors on csv::Settings and parquet::Settings with a single uniform with<>() accessor 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. 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::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. The with<>() accessor returns a fresh copy with one field replaced, so the configurable surface is unchanged. RFL_SETTINGS_OPS enforces this — it rejects pointers-to-non-const members, so any future field added without const is a compile error rather than silent regression.

Implementation notes

  • field_index_v<T, FieldPtr> recovers the field index in a consteval context 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.hpp switches from a static const member of a struct template to an extern const variable template (matching Boost.PFR). Without this, GCC requires the fake-object symbol at link time when evaluating &(fake.*FieldPtr) in consteval.

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. JSON 214/214 and CLI 46/47 pass locally (1 skipped is an unrelated locale test).

Centimo added 2 commits May 9, 2026 21:08
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.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread include/rfl/Settings.hpp
Comment on lines +58 to +61
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)));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant