Skip to content

Split detail into logical parts - #355

Merged
jbcoe merged 6 commits into
mainfrom
jbcoe-split-detail-logical-parts
Sep 13, 2026
Merged

Split detail into logical parts#355
jbcoe merged 6 commits into
mainfrom
jbcoe-split-detail-logical-parts

Conversation

@jbcoe

@jbcoe jbcoe commented Sep 12, 2026

Copy link
Copy Markdown
Owner

No description provided.

@codecov

codecov Bot commented Sep 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.24590% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.79%. Comparing base (ca5bf7d) to head (ba3ce6c).

Files with missing lines Patch % Lines
operator_thunks.hh 87.80% 0 Missing and 5 partials ⚠️
member_function_thunks.hh 71.42% 0 Missing and 4 partials ⚠️
Additional details and impacted files 
@@                       Coverage Diff                        @@
##           jbcoe-cleanup-mangling-tests     #355      +/-   ##
================================================================
- Coverage                         73.05%   72.79%   -0.27%     
================================================================
  Files                                10       12       +2     
  Lines                               835      838       +3     
  Branches                            218      221       +3     
================================================================
  Hits                                610      610              
  Misses                               22       22              
- Partials                            203      206       +3
Flag Coverage Δ
consteval 100.00% <ø> (ø)
runtime 72.39% <85.24%> (-0.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@philipcraig philipcraig mentioned this pull request Sep 12, 2026
Merged
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 6f6dfd7 to ad20403 Compare September 12, 2026 15:11
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch 2 times, most recently from cda5eb4 to 53526aa Compare September 12, 2026 15:27
@jbcoe
jbcoe added this pull request to stack #359 September 12, 2026 15:29
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 53526aa to f78871d Compare September 12, 2026 15:57
@jbcoe
jbcoe removed this pull request from stack #359 September 12, 2026 16:32
@jbcoe
jbcoe added this pull request to stack #362 September 12, 2026 16:41
@jbcoe
jbcoe marked this pull request as ready for review September 12, 2026 16:53
@jbcoe
jbcoe requested a review from philipcraig September 12, 2026 16:57
@jbcoe jbcoe mentioned this pull request Sep 12, 2026
Draft
philipcraig
philipcraig reviewed Sep 12, 2026
View reviewed changes

@philipcraig philipcraig left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review of the whole stack (#336 → #355) as one diff against main, at f78871d. Inline comments are on this PR because it holds the final form of every file. Two points fall outside this PR's diff:

  • scripts/consteval_coverage.py only instruments protocol.hh, which keeps 3 of its 21 consteval sites after the split. The consteval coverage job now measures almost nothing, and Codecov reports a pass because it compares percentages. The script needs to iterate over the new headers.
  • name_mangling.h:255 encodes every operator* as de (unary dereference). A binary operator* is ml. See the comment on conformance.hh:146.

Comment thread conformance.hh
Comment thread vtable.hh
Comment thread protocol_wrappers.hh
Comment thread operator_thunks.hh
Comment thread protocol_traits.hh
Comment thread vtable.hh
Comment thread conformance.hh Outdated
Comment thread operator_thunks.hh
@philipcraig philipcraig mentioned this pull request Sep 12, 2026
Closed
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from f78871d to 824fe8c Compare September 12, 2026 19:28
This was referenced Sep 12, 2026
Merged
Merged
Closed
jbcoe pushed a commit that referenced this pull request Sep 12, 2026
Test User @jbcoe
Instrument all consteval headers in coverage script
861c2d6 
consteval_coverage.py only instrumented protocol.hh, which after the
detail.hh split keeps 3 of the ~21 original consteval sites; the rest
moved to conformance.hh, vtable.hh, member_function_thunks.hh,
operator_thunks.hh and protocol_wrappers.hh. The coverage job was
reporting a percentage of almost nothing. Instrument all six headers,
tagging each trap with a file index alongside the line number since
line numbers collide across headers.

Addresses philipcraig's review comment on the #336-#355 stack (out of
diff): consteval_coverage.py only instruments protocol.hh.
jbcoe pushed a commit that referenced this pull request Sep 12, 2026
Test User
Instrument all consteval headers in coverage script
271d803 
consteval_coverage.py only instrumented protocol.hh, which after the
detail.hh split keeps 3 of the ~21 original consteval sites; the rest
moved to conformance.hh, vtable.hh, member_function_thunks.hh,
operator_thunks.hh, protocol_wrappers.hh and name_mangling.h. The
coverage job was reporting a percentage of almost nothing. Instrument
all seven headers, tagging each trap with a file index alongside the
line number since line numbers collide across headers.

The disarmed sentinel is a single NO_PROBE_ARMED constant reused by
both the generated C++ macro defaults and the Python call sites,
rather than -1 hardcoded independently in three places. ProbePoint
carries its file_index directly instead of recomputing it via
INSTRUMENTED_HEADERS.index() in run_probe, and write_lcov groups
probe_points by header once instead of re-filtering the full list per
header.

Addresses philipcraig's review comment on the #336-#355 stack (out of
diff): consteval_coverage.py only instruments protocol.hh.
jbcoe pushed a commit that referenced this pull request Sep 12, 2026
Test User @jbcoe
Instrument all consteval headers in coverage script
a831360 
consteval_coverage.py only instrumented protocol.hh, which after the
detail.hh split keeps 3 of the ~21 original consteval sites; the rest
moved to conformance.hh, vtable.hh, member_function_thunks.hh,
operator_thunks.hh, protocol_wrappers.hh and name_mangling.h. The
coverage job was reporting a percentage of almost nothing. Instrument
all seven headers, tagging each trap with a file index alongside the
line number since line numbers collide across headers.

The disarmed sentinel is a single NO_PROBE_ARMED constant reused by
both the generated C++ macro defaults and the Python call sites,
rather than -1 hardcoded independently in three places. ProbePoint
carries its file_index directly instead of recomputing it via
INSTRUMENTED_HEADERS.index() in run_probe, and write_lcov groups
probe_points by header once instead of re-filtering the full list per
header.

Addresses philipcraig's review comment on the #336-#355 stack (out of
diff): consteval_coverage.py only instruments protocol.hh.
@jbcoe
jbcoe removed this pull request from stack #362 September 12, 2026 22:29
@jbcoe
jbcoe added this pull request to stack #370 September 12, 2026 22:29
@jbcoe
jbcoe requested a review from philipcraig September 12, 2026 22:29
@jbcoe

jbcoe commented Sep 12, 2026

Copy link
Copy Markdown
Owner Author

Thanks for the detailed review.

I've added some follow-up PRs to the stack and fixed other issues as they were introduced.

I'll tackle the noexcept fn_ptr challenge as follow-up (#365).

@jbcoe
jbcoe removed this pull request from stack #370 September 12, 2026 22:34
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User @jbcoe
Restore anti-slicing guard on operator_overload_set
edd68e2 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 25a27f5 to 2d2558c Compare September 13, 2026 15:31
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User @jbcoe
Restore anti-slicing guard on operator_overload_set
1fedce0 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 2d2558c to 34137bc Compare September 13, 2026 15:33
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Reject binary operator* with a clear diagnostic
67189f0 
operator_thunk only specialised the nullary R(*)() form for op_star, so
an interface with a binary operator* (e.g. `Vec operator*(double) const`)
never matched a partial specialisation and fell through to the deleted
primary template, producing an opaque error deep inside
operator_overload_set instead of a clear diagnostic. Binary operator*
support is deferred; for now, reject it explicitly in
protocol_interface_function_infos, next to the ref-qualified check, so
the error surfaces at the interface.

The arity check excludes an explicit object parameter ("deducing this")
so a unary operator* declared that way isn't misclassified as binary.

Addresses philipcraig's review comment on conformance.hh:146 in #355.
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Restore anti-slicing guard on operator_overload_set
67aee60 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 34137bc to dc098e1 Compare September 13, 2026 15:34
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Restore anti-slicing guard on operator_overload_set
283e4d8 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from dc098e1 to b418417 Compare September 13, 2026 15:37
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Restore anti-slicing guard on operator_overload_set
d69a76e 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from b418417 to a742aa1 Compare September 13, 2026 15:37
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Restore anti-slicing guard on operator_overload_set
2d91544 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from a742aa1 to 480be76 Compare September 13, 2026 15:38
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User
Restore anti-slicing guard on operator_overload_set
8c2d9b5 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 480be76 to 6b00867 Compare September 13, 2026 15:39
jbcoe pushed a commit that referenced this pull request Sep 13, 2026
Test User @jbcoe
Restore anti-slicing guard on operator_overload_set
954d72e 
e6d30c2 (#347, "Make operator_thunk generic to simplify friendship")
dropped the protected special members that stopped operator_overload_set
being sliced off through a non-view pointer, while renaming
call_operator_overload_set to the now-generic operator_overload_set.
Restore them on all four operator specialisations, and add
CallOperatorOverloadSetCannotBeDetached tests for protocol and
protocol_view alongside the existing MemberThunksCannotBeDetached ones,
covering the slicing repro from the review comment.

Addresses philipcraig's review comment on operator_thunks.hh:243 in #355.
@jbcoe
jbcoe force-pushed the jbcoe-split-detail-logical-parts branch from 6b00867 to 1e378e2 Compare September 13, 2026 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philipcraig philipcraig philipcraig approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jbcoe @philipcraig