Skip to content

Conversation

@lukevalenty
Copy link
Contributor

No description provided.

@lukevalenty lukevalenty requested a review from elbeno November 16, 2025 01:48

The following diagram shows how these components relate to each other:

[source,mermaid]
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: This doesn't render properly. Pull out the mermaid source to a foo.mmd file (then github knows how to render it as well!) and use:
mermaid::foo.mmd[format="svg" config="mermaid.conf"]

Copy link
Contributor

@elbeno elbeno left a comment

Choose a reason for hiding this comment

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

Nice work. Only so much I can review right now...


| `uninitialized`
| Can remain uninitialized
| Reserved/padding fields
Copy link
Contributor

Choose a reason for hiding this comment

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

note: A primary use case for uninitialized fields is also overlapping fields so the same bit can be addressed by two fields (e.g. N fields for single bits, one field for the bitset) but the field initializations don't fight.

| `with_in<V1,V2...>`
| Set membership matcher
| Multiple valid values
|===
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Add with_predicate


**Typical development loop:**

[source,mermaid]
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: This doesn't render either - extract.

| Any matcher must match
| `match::any(m1, m2, m3)`

| `match::predicate<N>(λ)`
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Fancy λ


| `service<MsgBase>`
| Linear scan of all callbacks
| \<10 callbacks, simple dispatch
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: The \< is not rendering properly.

the following message will not call the callback:
===== Automatic simplification

The matcher system performs compile-time simplification:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Add a link to the talk.
(Also, not sure this belongs in the message library docs -- there is a match library section.)

A custom matcher must satisfy the following requirements:

1. **Type tag**: `using is_matcher = void;`
2. **Match operator**: `operator()(Event const &) const -> bool`
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: -> becomes a ligature


1. **Type tag**: `using is_matcher = void;`
2. **Match operator**: `operator()(Event const &) const -> bool`
3. **Description**: `describe() const` - returns a compile-time string
Copy link
Contributor

Choose a reason for hiding this comment

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

note: "returns a compile-time string" is correct but probably not enough info to actually go on...


// everything else is the same
For simple lambda-based matchers, use `match::predicate`:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Or even better, use field::with_predicate.

}

[[nodiscard]] constexpr static auto describe() {
return stdx::ct_format<"{} in [{}, {}]">(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Probably all of these naked stdx::ct_format calls should be replaced with STDX_CT_FORMAT which will reduce the noise of things like cts_t wrappers.

@lukevalenty
Copy link
Contributor Author

Awesome feedback, thank you! I know it's a big pr 😅

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.

4 participants