style(prof): fix tons of clippy lints#3235
Merged
realFlowControl merged 3 commits intoflorian/zts-global-statefrom May 8, 2025
Merged
style(prof): fix tons of clippy lints#3235realFlowControl merged 3 commits intoflorian/zts-global-statefrom
realFlowControl merged 3 commits intoflorian/zts-global-statefrom
Conversation
Also improve documentation in some places.
Benchmarks [ profiler ]Benchmark execution time: 2025-05-07 15:52:19 Comparing candidate commit bf02b95 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 29 metrics, 7 unstable metrics. |
morrisonlevi
commented
May 7, 2025
Comment on lines
+464
to
467
| // todo: this should be feature = "stack_walking_tests" but it seemed to | ||
| // cause a failure in CI to migrate it. | ||
| #[cfg(all(test, stack_walking_tests))] | ||
| mod tests { |
Collaborator
Author
There was a problem hiding this comment.
Example of the error (job):
= note: /mnt/ramdisk/cargo/release/deps/datadog_php_profiling-a15e7ad77c649749.datadog_php_profiling.b94f957b4ddf90d7-cgu.0.rcgu.o: In function `{closure#0}':
/home/circleci/datadog/profiling/src/profiling/stack_walking.rs:263: undefined reference to `zend_flf_functions'
collect2: error: ld returned 1 exit status
= note: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
= note: use the `-l` flag to specify native libraries to link
= note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-lib)
error: could not compile `datadog-php-profiling` (lib test) due to 1 previous error
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## florian/zts-global-state #3235 +/- ##
==============================================================
+ Coverage 79.26% 79.28% +0.01%
Complexity 2948 2948
==============================================================
Files 118 118
Lines 11633 11633
==============================================================
+ Hits 9221 9223 +2
+ Misses 2412 2410 -2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
realFlowControl
approved these changes
May 7, 2025
Co-authored-by: Florian Engelhardt <[email protected]>
realFlowControl
merged commit afe5209
into
florian/zts-global-state
827 of 829 checks passed
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
3 participants
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.
Description
Many clippy lints have accumulated. This fixes all of the lints on PHP 8.3/8.4 and on Rust 1.78. It also fixes many lints from Rust 1.86 and nightly. Here are some of the things fixed:
c""and converting some engine members from*const u8to*const c_char.core::ptr::eqfor certain pointer comparisons.And some non-clippy items:
/*style comments to//, which is by far more popular.Safety:comments toSAFETY:, which is mostly what we use everywhere else.There were historical reasons for not running clippy in CI. Over time think these have all been fixed, helped by advances in Rust. We should enable clippy soon for profiling. On the versions listed, the only lints on current nightly are unpredictable function pointer comparisons. I'm not sure what to do here, other than maybe allow them. But since this is nightly, it may not land in its current state anyway, so I'm not going to sweat it.
Reviewer checklist