-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[EventPipe] Fix folding constant warning #120375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EventPipe] Fix folding constant warning #120375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a folding constant warning in the EventPipe native code by replacing magic numbers with properly defined constants. The changes improve code maintainability by eliminating hardcoded values and making the code more readable.
Key changes:
- Defines new constants for EventPipe user events configuration
- Replaces magic numbers with named constants in session management code
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/native/eventpipe/ep-types-forward.h | Adds new constant definitions for user events capacity and activity ID extension size |
src/native/eventpipe/ep-session.c | Replaces magic numbers with the newly defined constants |
I upgraded my mac m1 to Tahoe 26.0.1 and Xcode 26.0.1 (17A400) % sw_vers
ProductName: macOS
ProductVersion: 26.0.1
BuildVersion: 25A362 but I can't seem to reproduce the build errors @vcsjones hit in #120007 (comment) from a clean repo. I think moving those vars to defines should resolve the warnings but as I can't repro the compiler warnings, I couldn't double check. |
ad28043
to
2b1a794
Compare
Hm. I still see the warnings in the build in Anyway, I don't see the warnings in the build with this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting the constant into a macro or enum should resolve the warning. Reduce the scope and put it directly in the source file instead of the more widely used header.
Fixes #120007