1
0
mirror of https://github.com/emilk/egui.git synced 2026-08-29 04:40:03 -04:00

Enable or_fun_call lint to avoid unnecessary allocations (#7754)

### What

From the [lint
description](https://rust-lang.github.io/rust-clippy/master/index.html?search=or_fu#or_fun_call):

> The function will always be called. This is only bad if it allocates
or does some non-trivial amount of work.

But also:

> If the function has side-effects, not calling it will change the
semantic of the program, but you shouldn’t rely on that.
> 
> The lint also cannot figure out whether the function you call is
actually expensive to call or not.

Still worth it to keep our happy paths clean, imo.
This commit is contained in:
Jochen Görtler
2025-12-05 10:46:34 +01:00
committed by GitHub
parent 3fcdab4ebd
commit 2dbfe3a083
19 changed files with 44 additions and 33 deletions

View File

@@ -278,6 +278,7 @@ non_zero_suggestions = "warn"
nonstandard_macro_braces = "warn" nonstandard_macro_braces = "warn"
option_as_ref_cloned = "warn" option_as_ref_cloned = "warn"
option_option = "warn" option_option = "warn"
or_fun_call = "warn"
path_buf_push_overwrite = "warn" path_buf_push_overwrite = "warn"
pathbuf_init_then_push = "warn" pathbuf_init_then_push = "warn"
precedence_bits = "warn" precedence_bits = "warn"

View File

@@ -719,11 +719,11 @@ impl GlowWinitRunning<'_> {
// vsync - don't count as frame-time: // vsync - don't count as frame-time:
frame_timer.pause(); frame_timer.pause();
profiling::scope!("swap_buffers"); profiling::scope!("swap_buffers");
let context = current_gl_context let context = current_gl_context.as_ref().ok_or_else(|| {
.as_ref() egui_glow::PainterError::from(
.ok_or(egui_glow::PainterError::from(
"failed to get current context to swap buffers".to_owned(), "failed to get current context to swap buffers".to_owned(),
))?; )
})?;
gl_surface.swap_buffers(context)?; gl_surface.swap_buffers(context)?;
frame_timer.resume(); frame_timer.resume();

View File

@@ -82,7 +82,7 @@ impl<'a> AtomKind<'a> {
) -> (Vec2, SizedAtomKind<'a>) { ) -> (Vec2, SizedAtomKind<'a>) {
match self { match self {
AtomKind::Text(text) => { AtomKind::Text(text) => {
let wrap_mode = wrap_mode.unwrap_or(ui.wrap_mode()); let wrap_mode = wrap_mode.unwrap_or_else(|| ui.wrap_mode());
let galley = text.into_galley(ui, Some(wrap_mode), available_size.x, fallback_font); let galley = text.into_galley(ui, Some(wrap_mode), available_size.x, fallback_font);
(galley.intrinsic_size(), SizedAtomKind::Text(galley)) (galley.intrinsic_size(), SizedAtomKind::Text(galley))
} }

View File

@@ -168,7 +168,7 @@ impl<'a> AtomLayout<'a> {
let fallback_font = fallback_font.unwrap_or_default(); let fallback_font = fallback_font.unwrap_or_default();
let wrap_mode = wrap_mode.unwrap_or(ui.wrap_mode()); let wrap_mode = wrap_mode.unwrap_or_else(|| ui.wrap_mode());
// If the TextWrapMode is not Extend, ensure there is some item marked as `shrink`. // If the TextWrapMode is not Extend, ensure there is some item marked as `shrink`.
// If none is found, mark the first text item as `shrink`. // If none is found, mark the first text item as `shrink`.
@@ -188,7 +188,7 @@ impl<'a> AtomLayout<'a> {
let fallback_text_color = let fallback_text_color =
fallback_text_color.unwrap_or_else(|| ui.style().visuals.text_color()); fallback_text_color.unwrap_or_else(|| ui.style().visuals.text_color());
let gap = gap.unwrap_or(ui.spacing().icon_spacing); let gap = gap.unwrap_or_else(|| ui.spacing().icon_spacing);
// The size available for the content // The size available for the content
let available_inner_size = ui.available_size() - frame.total_margin().sum(); let available_inner_size = ui.available_size() - frame.total_margin().sum();

View File

@@ -941,7 +941,7 @@ impl Panel {
PanelState::load(ctx, panel.id) PanelState::load(ctx, panel.id)
.map(get_rect_state_size) .map(get_rect_state_size)
.or(panel.default_size) .or(panel.default_size)
.unwrap_or(get_spacing_size()) .unwrap_or_else(get_spacing_size)
} }
} }

View File

@@ -465,7 +465,7 @@ impl<'a> Popup<'a> {
pub fn get_best_align(&self) -> RectAlign { pub fn get_best_align(&self) -> RectAlign {
let expected_popup_size = self let expected_popup_size = self
.get_expected_size() .get_expected_size()
.unwrap_or(vec2(self.width.unwrap_or(0.0), 0.0)); .unwrap_or_else(|| vec2(self.width.unwrap_or(0.0), 0.0));
let Some(anchor_rect) = self.anchor.rect(self.id, &self.ctx) else { let Some(anchor_rect) = self.anchor.rect(self.id, &self.ctx) else {
return self.rect_align; return self.rect_align;
@@ -473,6 +473,7 @@ impl<'a> Popup<'a> {
RectAlign::find_best_align( RectAlign::find_best_align(
#[expect(clippy::iter_on_empty_collections)] #[expect(clippy::iter_on_empty_collections)]
#[expect(clippy::or_fun_call)]
once(self.rect_align).chain( once(self.rect_align).chain(
self.alternative_aligns self.alternative_aligns
// Need the empty slice so the iters have the same type so we can unwrap_or // Need the empty slice so the iters have the same type so we can unwrap_or

View File

@@ -611,7 +611,7 @@ impl ContextImpl {
} }
let parent_id = find_accesskit_parent(&state.parent_map, builders, id) let parent_id = find_accesskit_parent(&state.parent_map, builders, id)
.unwrap_or(crate::accesskit_root_id()); .unwrap_or_else(crate::accesskit_root_id);
let parent_builder = builders.get_mut(&parent_id).unwrap(); let parent_builder = builders.get_mut(&parent_id).unwrap();
parent_builder.push_child(id.accesskit_id()); parent_builder.push_child(id.accesskit_id());

View File

@@ -433,7 +433,7 @@ impl Response {
pub fn drag_motion(&self) -> Vec2 { pub fn drag_motion(&self) -> Vec2 {
if self.dragged() { if self.dragged() {
self.ctx self.ctx
.input(|i| i.pointer.motion().unwrap_or(i.pointer.delta())) .input(|i| i.pointer.motion().unwrap_or_else(|| i.pointer.delta()))
} else { } else {
Vec2::ZERO Vec2::ZERO
} }

View File

@@ -136,7 +136,7 @@ impl Ui {
accessibility_parent, accessibility_parent,
} = ui_builder; } = ui_builder;
let layer_id = layer_id.unwrap_or(LayerId::background()); let layer_id = layer_id.unwrap_or_else(LayerId::background);
debug_assert!( debug_assert!(
id_salt.is_none(), id_salt.is_none(),
@@ -148,7 +148,7 @@ impl Ui {
let layout = layout.unwrap_or_default(); let layout = layout.unwrap_or_default();
let disabled = disabled || invisible; let disabled = disabled || invisible;
let style = style.unwrap_or_else(|| ctx.style()); let style = style.unwrap_or_else(|| ctx.style());
let sense = sense.unwrap_or(Sense::hover()); let sense = sense.unwrap_or_else(Sense::hover);
let placer = Placer::new(max_rect, layout); let placer = Placer::new(max_rect, layout);
let ui_stack = UiStack { let ui_stack = UiStack {
@@ -277,7 +277,7 @@ impl Ui {
let id_salt = id_salt.unwrap_or_else(|| Id::from("child")); let id_salt = id_salt.unwrap_or_else(|| Id::from("child"));
let max_rect = max_rect.unwrap_or_else(|| self.available_rect_before_wrap()); let max_rect = max_rect.unwrap_or_else(|| self.available_rect_before_wrap());
let mut layout = layout.unwrap_or(*self.layout()); let mut layout = layout.unwrap_or_else(|| *self.layout());
let enabled = self.enabled && !disabled && !invisible; let enabled = self.enabled && !disabled && !invisible;
if let Some(layer_id) = layer_id { if let Some(layer_id) = layer_id {
painter.set_layer_id(layer_id); painter.set_layer_id(layer_id);
@@ -287,7 +287,7 @@ impl Ui {
} }
let sizing_pass = self.sizing_pass || sizing_pass; let sizing_pass = self.sizing_pass || sizing_pass;
let style = style.unwrap_or_else(|| self.style.clone()); let style = style.unwrap_or_else(|| self.style.clone());
let sense = sense.unwrap_or(Sense::hover()); let sense = sense.unwrap_or_else(Sense::hover);
if sizing_pass { if sizing_pass {
// During the sizing pass we want widgets to use up as little space as possible, // During the sizing pass we want widgets to use up as little space as possible,

View File

@@ -681,7 +681,7 @@ pub fn paint_texture_load_result(
} }
Ok(TexturePoll::Pending { .. }) => { Ok(TexturePoll::Pending { .. }) => {
let show_loading_spinner = let show_loading_spinner =
show_loading_spinner.unwrap_or(ui.visuals().image_loading_spinners); show_loading_spinner.unwrap_or_else(|| ui.visuals().image_loading_spinners);
if show_loading_spinner { if show_loading_spinner {
Spinner::new().paint_at(ui, rect); Spinner::new().paint_at(ui, rect);
} }

View File

@@ -248,7 +248,9 @@ impl Label {
layout_job.halign = Align::LEFT; layout_job.halign = Align::LEFT;
layout_job.justify = false; layout_job.justify = false;
} else { } else {
layout_job.halign = self.halign.unwrap_or(ui.layout().horizontal_placement()); layout_job.halign = self
.halign
.unwrap_or_else(|| ui.layout().horizontal_placement());
layout_job.justify = ui.layout().horizontal_justify(); layout_job.justify = ui.layout().horizontal_justify();
} }

View File

@@ -118,7 +118,7 @@ impl Widget for ProgressBar {
let desired_width = let desired_width =
desired_width.unwrap_or_else(|| ui.available_size_before_wrap().x.at_least(96.0)); desired_width.unwrap_or_else(|| ui.available_size_before_wrap().x.at_least(96.0));
let height = desired_height.unwrap_or(ui.spacing().interact_size.y); let height = desired_height.unwrap_or_else(|| ui.spacing().interact_size.y);
let (outer_rect, response) = let (outer_rect, response) =
ui.allocate_exact_size(vec2(desired_width, height), Sense::hover()); ui.allocate_exact_size(vec2(desired_width, height), Sense::hover());

View File

@@ -496,7 +496,7 @@ impl TextEdit<'_> {
} = self; } = self;
let text_color = text_color let text_color = text_color
.or(ui.visuals().override_text_color) .or_else(|| ui.visuals().override_text_color)
// .unwrap_or_else(|| ui.style().interact(&response).text_color()); // too bright // .unwrap_or_else(|| ui.style().interact(&response).text_color()); // too bright
.unwrap_or_else(|| ui.visuals().widgets.inactive.text_color()); .unwrap_or_else(|| ui.visuals().widgets.inactive.text_color());
@@ -691,7 +691,7 @@ impl TextEdit<'_> {
if ui.is_rect_visible(rect) { if ui.is_rect_visible(rect) {
if text.as_str().is_empty() && !hint_text.is_empty() { if text.as_str().is_empty() && !hint_text.is_empty() {
let hint_text_color = ui.visuals().weak_text_color(); let hint_text_color = ui.visuals().weak_text_color();
let hint_text_font_id = hint_text_font.unwrap_or(font_id.into()); let hint_text_font_id = hint_text_font.unwrap_or_else(|| font_id.into());
let galley = if multiline { let galley = if multiline {
hint_text.into_galley( hint_text.into_galley(
ui, ui,

View File

@@ -199,8 +199,8 @@ impl AccessibilityInspectorPlugin {
} }
let label = node let label = node
.label() .label()
.or(node.value()) .or_else(|| node.value())
.unwrap_or(node.id().0.to_string()); .unwrap_or_else(|| node.id().0.to_string());
let label = format!("({:?}) {}", node.role(), label); let label = format!("({:?}) {}", node.role(), label);
// Safety: This is safe since the `accesskit::NodeId` was created from an `egui::Id`. // Safety: This is safe since the `accesskit::NodeId` was created from an `egui::Id`.

View File

@@ -43,8 +43,11 @@ impl DemoApp for FractalClockApp {
.stroke(egui::Stroke::NONE) .stroke(egui::Stroke::NONE)
.corner_radius(0) .corner_radius(0)
.show(ui, |ui| { .show(ui, |ui| {
self.fractal_clock self.fractal_clock.ui(
.ui(ui, self.mock_time.or(Some(crate::seconds_since_midnight()))); ui,
self.mock_time
.or_else(|| Some(crate::seconds_since_midnight())),
);
}); });
} }
} }

View File

@@ -479,7 +479,7 @@ impl<'a> TableBuilder<'a> {
} }
} }
let striped = striped.unwrap_or(ui.visuals().striped); let striped = striped.unwrap_or_else(|| ui.visuals().striped);
let state_id = ui.id().with(id_salt); let state_id = ui.id().with(id_salt);
@@ -548,7 +548,7 @@ impl<'a> TableBuilder<'a> {
sense, sense,
} = self; } = self;
let striped = striped.unwrap_or(ui.visuals().striped); let striped = striped.unwrap_or_else(|| ui.visuals().striped);
let state_id = ui.id().with(id_salt); let state_id = ui.id().with(id_salt);

View File

@@ -264,7 +264,8 @@ impl Display for SnapshotError {
diff, diff,
diff_path, diff_path,
} => { } => {
let diff_path = std::path::absolute(diff_path).unwrap_or(diff_path.clone()); let diff_path =
std::path::absolute(diff_path).unwrap_or_else(|_| diff_path.clone());
write!( write!(
f, f,
"'{name}' Image did not match snapshot. Diff: {diff}, {}. {HOW_TO_UPDATE_SCREENSHOTS}", "'{name}' Image did not match snapshot. Diff: {diff}, {}. {HOW_TO_UPDATE_SCREENSHOTS}",
@@ -272,7 +273,7 @@ impl Display for SnapshotError {
) )
} }
Self::OpenSnapshot { path, err } => { Self::OpenSnapshot { path, err } => {
let path = std::path::absolute(path).unwrap_or(path.clone()); let path = std::path::absolute(path).unwrap_or_else(|_| path.clone());
match err { match err {
ImageError::IoError(io) => match io.kind() { ImageError::IoError(io) => match io.kind() {
ErrorKind::NotFound => { ErrorKind::NotFound => {
@@ -310,7 +311,7 @@ impl Display for SnapshotError {
) )
} }
Self::WriteSnapshot { path, err } => { Self::WriteSnapshot { path, err } => {
let path = std::path::absolute(path).unwrap_or(path.clone()); let path = std::path::absolute(path).unwrap_or_else(|_| path.clone());
write!(f, "Error writing snapshot: {err}\nAt: {}", path.display()) write!(f, "Error writing snapshot: {err}\nAt: {}", path.display())
} }
Self::RenderError { err } => { Self::RenderError { err } => {

View File

@@ -51,7 +51,7 @@ pub fn default_wgpu_setup() -> egui_wgpu::WgpuSetup {
adapters adapters
.first() .first()
.map(|a| (*a).clone()) .map(|a| (*a).clone())
.ok_or("No adapter found".to_owned()) .ok_or_else(|| "No adapter found".to_owned())
})); }));
egui_wgpu::WgpuSetup::CreateNew(setup) egui_wgpu::WgpuSetup::CreateNew(setup)

View File

@@ -298,7 +298,8 @@ impl CubicBezierShape {
/// the number of points is determined by the tolerance. /// the number of points is determined by the tolerance.
/// the points may not be evenly distributed in the range [0.0,1.0] (t value) /// the points may not be evenly distributed in the range [0.0,1.0] (t value)
pub fn flatten(&self, tolerance: Option<f32>) -> Vec<Pos2> { pub fn flatten(&self, tolerance: Option<f32>) -> Vec<Pos2> {
let tolerance = tolerance.unwrap_or((self.points[0].x - self.points[3].x).abs() * 0.001); let tolerance =
tolerance.unwrap_or_else(|| (self.points[0].x - self.points[3].x).abs() * 0.001);
let mut result = vec![self.points[0]]; let mut result = vec![self.points[0]];
self.for_each_flattened_with_t(tolerance, &mut |p, _t| { self.for_each_flattened_with_t(tolerance, &mut |p, _t| {
result.push(p); result.push(p);
@@ -313,7 +314,8 @@ impl CubicBezierShape {
/// The result will be a vec of vec of Pos2. it will store two closed aren in different vec. /// The result will be a vec of vec of Pos2. it will store two closed aren in different vec.
/// The epsilon is used to compare a float value. /// The epsilon is used to compare a float value.
pub fn flatten_closed(&self, tolerance: Option<f32>, epsilon: Option<f32>) -> Vec<Vec<Pos2>> { pub fn flatten_closed(&self, tolerance: Option<f32>, epsilon: Option<f32>) -> Vec<Vec<Pos2>> {
let tolerance = tolerance.unwrap_or((self.points[0].x - self.points[3].x).abs() * 0.001); let tolerance =
tolerance.unwrap_or_else(|| (self.points[0].x - self.points[3].x).abs() * 0.001);
let epsilon = epsilon.unwrap_or(1.0e-5); let epsilon = epsilon.unwrap_or(1.0e-5);
let mut result = Vec::new(); let mut result = Vec::new();
let mut first_half = Vec::new(); let mut first_half = Vec::new();
@@ -519,7 +521,8 @@ impl QuadraticBezierShape {
/// the number of points is determined by the tolerance. /// the number of points is determined by the tolerance.
/// the points may not be evenly distributed in the range [0.0,1.0] (t value) /// the points may not be evenly distributed in the range [0.0,1.0] (t value)
pub fn flatten(&self, tolerance: Option<f32>) -> Vec<Pos2> { pub fn flatten(&self, tolerance: Option<f32>) -> Vec<Pos2> {
let tolerance = tolerance.unwrap_or((self.points[0].x - self.points[2].x).abs() * 0.001); let tolerance =
tolerance.unwrap_or_else(|| (self.points[0].x - self.points[2].x).abs() * 0.001);
let mut result = vec![self.points[0]]; let mut result = vec![self.points[0]];
self.for_each_flattened_with_t(tolerance, &mut |p, _t| { self.for_each_flattened_with_t(tolerance, &mut |p, _t| {
result.push(p); result.push(p);