Skip to content

Fix release build compilation error#7329

Merged
fkouteib merged 1 commit intoanza-xyz:masterfrom
fkouteib:compile_error
Aug 5, 2025
Merged

Fix release build compilation error#7329
fkouteib merged 1 commit intoanza-xyz:masterfrom
fkouteib:compile_error

Conversation

@fkouteib
Copy link
Copy Markdown

@fkouteib fkouteib commented Aug 5, 2025

Problem

"./cargo build --release" fails with compilation error over a cfg guard added in this PR.

[error[E0599]:] no method named is_empty found for struct Batches in the current scope
--> core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs:200:33
|
200 | self.common.batches.is_empty(),
| ^^^^^^^^ method not found in Batches<Tx>
|
::: core/src/banking_stage/transaction_scheduler/scheduler_common.rs:21:1
|
21 | pub struct Batches {
| ---------------------- method is_empty not found for this struct

Summary of Changes

Add cfg guard to the method invokations because the compiler doesn't ignore/skip checks inside debug_assert!() on release build. Alternatively, I can remove the cfg guard on the method definition.

// Check transactions against filter, remove from container if it fails.
chunked_pops(container, &mut self.prio_graph, &mut window_budget);

#[cfg(debug_assertions)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't this implied by debug_assert???

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That's what I assumed too, but the build error and docs say otherwise. From Rust docs, "The result of expanding debug_assert! is always type checked."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

that's annoying

Copy link
Copy Markdown

@steviez steviez Sep 3, 2025

Choose a reason for hiding this comment

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

Isn't this implied by debug_assert???
...
That's what I assumed too

git blame led me here and same, would not have expected to have to add the extra markup:
https://doc.rust-lang.org/src/core/macros/mod.rs.html#306-312

To clarify, no actions being requested by me, I'm just sharing in your surprise a month late 😆

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.0%. Comparing base (e5eb344) to head (165995a).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7329   +/-   ##
=======================================
  Coverage    83.0%    83.0%           
=======================================
  Files         801      801           
  Lines      362242   362244    +2     
=======================================
+ Hits       300789   300802   +13     
+ Misses      61453    61442   -11     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

fine workaround for an annoying aspect of rust's debug_assert.

@fkouteib fkouteib merged commit b82d3bb into anza-xyz:master Aug 5, 2025
41 checks passed
@fkouteib fkouteib deleted the compile_error branch August 5, 2025 20:37
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