Skip to content

Fix ambiguous partial specialization when appending a sum to a pack#295

Merged
Bronek merged 2 commits into
mainfrom
bronek/P1/pack_append_partial_spec
Jul 13, 2026
Merged

Fix ambiguous partial specialization when appending a sum to a pack#295
Bronek merged 2 commits into
mainfrom
bronek/P1/pack_append_partial_spec

Conversation

@Bronek

@Bronek Bronek commented Jul 13, 2026

Copy link
Copy Markdown
Member

Two defects in the same pack::append overload set.

Fix ambiguous partial specialization when appending a sum to a pack

_pack_append's two specializations both matched a sum and neither subsumed the other, so naming append_type<sum> was ill-formed: gcc hard-errored even inside a requires-expression, while clang answered false. They now exclude each other, and the sum case defines no type, making the rejection a clean substitution failure on both compilers. Drops _append<sum> = delete, which append's trailing return type made unreachable.

Reject in_place_type as a deduced pack::append argument

The tag selects the element type; it is never an element. With a type that has no default constructor the in-place overload dropped out on its is_constructible_v conjunct and the deduced-argument overload picked the call up, silently appending the tag itself, so append(in_place_type<T>) meant "construct a T" or "append the tag" depending on a property of T. The deduced-argument overloads now carry the guard as_pack already had, leaving the in-place overload as the only candidate for a tag.

Closes #282
Closes #283


Stack: P1 of 13 — the release-0.1 bugfix queue, merging bottom-up into main; this PR is the base of the stack. Every stack boundary was validated with a gcc build and the full test suite on top of main, and the aggregate branch ran the full CI matrix green (#289).

Bronek added 2 commits July 13, 2026 09:21
`_pack_append`'s two specializations both matched a sum and neither subsumed the
other, so naming `append_type<sum>` was ill-formed: gcc hard-errored even inside
a requires-expression, while clang answered false. They now exclude each other,
and the sum case defines no `type`, making the rejection a clean substitution
failure on both compilers. Drops `_append<sum> = delete`, which `append`'s
trailing return type made unreachable.

Closes #282

Assisted-by: Claude:claude-opus-4-8[1m]
The tag selects the element type; it is never an element. With a type that has no
default constructor the in-place overload dropped out on its `is_constructible_v`
conjunct and the deduced-argument overload picked the call up, silently appending
the tag itself, so `append(in_place_type<T>)` meant "construct a T" or "append the
tag" depending on a property of T. The deduced-argument overloads now carry the
guard `as_pack` already had, leaving the in-place overload as the only candidate
for a tag.

Closes #283

Assisted-by: Claude:claude-opus-4-8[1m]
@augmentcode

augmentcode Bot commented Jul 13, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Fixes two constraint/overload-resolution defects in fn::pack::append to make failures consistent and prevent accidental tag appends.

Changes:

  • Makes detail::_pack_append specializations mutually exclusive for sum vs non-sum types, avoiding ambiguous partial specialization when querying append_type<sum>.
  • Defines the sum-matching _pack_append specialization without a type, so append_type<sum> fails via clean substitution (rather than hard-error/ambiguity) across compilers.
  • Removes the unreachable pack_impl::_append<sum> = delete overload (the trailing return type already prevented it from participating).
  • Adds a not some_in_place_type<Arg> guard to the deduced-argument pack::append(Arg) overloads so std::in_place_type can’t be appended as an element.
  • Extends pack tests with constraint-focused static_assert coverage for sums and in_place_type behavior.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread tests/fn/pack.cpp
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Bronek

Bronek commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@Bronek Bronek merged commit c6be2ab into main Jul 13, 2026
64 checks passed
Bronek added a commit that referenced this pull request Jul 13, 2026
Take #295's corrected pack.append expectations into the restructured
constraints section: it fixes #282 and #283, which this branch had pinned
as GAP comments, and makes the sum negative probes portable at last. The
#280 noexcept tripwires stay - that issue is still open.

Assisted-by: Claude:claude-opus-4-8[1m]
Bronek added a commit that referenced this pull request Jul 13, 2026
The GAP note it replaces described an overload that no longer exists: #295
dropped `_append<sum> = delete` as unreachable, and made naming
append_type<sum> a clean substitution failure on both compilers, which the
note said no portable probe could ask.

Assisted-by: Claude:claude-opus-4-8[1m]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant