tracing: simplify level event macros#3441
Open
hds wants to merge 1 commit into
Open
Conversation
hds
force-pushed
the
hds/simplify-level-event-macros
branch
2 times, most recently
from
cd5fd4f to
2ee3d9c
Compare
The level event macros (e.g. `trace!`, `info!`) call into the `event!` macro with the level set appropriately. However, these macros have become complex, covering many different cases and reaching aroudn 250 lines of patterns. As discovered in #3437, there were many cases where these level event macros didn't correctly handle patterns which the `event!` macro does handle. This change simplifies the event macros to delegate more parsing to the common `event!` macro patterns. Rather than matching all the possible field patterns, the level event macros only match the directives (name, target, and parent or some combination thereof). The remainder of the macro invocation is matched as a token tree (`tt`) and passed as is to the `event!` macro. This reduces the patterns to only the 8 combinations of directives (including no directives at all), reducing the previous 250 lines of patterns for each macro down to around 25 lines instead. Additionally, an unqualified use of `Some` in the `valueset` macro has been fixed, as this affected all event macros (`event!` and all the level event macros). With these changes, the comprehensive checks introduced in #3437 now all pass and so the job can be fully enabled to fail on CI.
hds
force-pushed
the
hds/simplify-level-event-macros
branch
from
2ee3d9c to
78d2af6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The level event macros (e.g.
trace!,info!) call into theevent!macro with the level set appropriately. However, these macros have
become complex, covering many different cases and reaching aroudn 250
lines of patterns.
As discovered in #3437, there were many cases where these level event
macros didn't correctly handle patterns which the
event!macro doeshandle.
Solution
This change simplifies the event macros to delegate more parsing to the
common
event!macro patterns. Rather than matching all the possiblefield patterns, the level event macros only match the directives (name,
target, and parent or some combination thereof). The remainder of the
macro invocation is matched as a token tree (
tt) and passed as is tothe
event!macro.This reduces the patterns to only the 8 combinations of directives
(including no directives at all), reducing the previous 250 lines of
patterns for each macro down to around 25 lines instead.
Additionally, an unqualified use of
Somein thevaluesetmacro hasbeen fixed, as this affected all event macros (
event!and all thelevel event macros).
With these changes, the comprehensive checks introduced in #3437 now all
pass and so the job can be fully enabled to fail on CI.