Builder pattern gives "cannot return value referencing temporary value"

help
1

I know this type of question has been asked a thousand times in here, but I still can't work out why it happens here.

Building an interface in ICED.

Working code to build a button instance:

pub(crate) fn view(&self) -> Container<Message, Theme, Renderer> {
  let w = Length::from(10.0);
  let h = Length::from(10.0);
  let b_one = make_button(w, h, "1");
  container(b_one)   // returns here

}

fn make_button(width: Length, height: Length, name: &str) -> Element<Message> {
    let container = Container::new(name)
        .align_x(Horizontal::Center)
        .align_y(Vertical::Center)
        .clip(false);

    Button::new(container)
        .width(width)
        .height(height)
        .style(|_theme, status| {
            crate::ui::window::get_style(status)
        })
        .on_press(Message::Char(name.to_string())))
        .into()
}

But if I add a builder like so

struct Builder {
    w : Length,
    h : Length,
}
impl Builder {
    fn make(&self) -> Element<Message> {
        make_button(self.w, self.h, "1")
    }
}

and use it to to make the button

let b_one = Builder{w, h}.make();
etc

Then I get the well know cannot return value referencing temporary value.

2

I'm not seeing the source for make_button. At what line does the error occur, or actually, please just post the entire message from running cargo build or cargo check in the terminal -- this is always the right thing.

2 Likes
3

unsure if this is related, but if Length is a small scalar type (eg. u64), it should probably implement Copy

4

If Message has a lifetime (Message<'_>),

  • On the working code, add #![deny(elided_lifetimes_in_paths)] and fix all the errors. You'll have to understand lifetime elision in function signatures to know what the signatures that now throw errors meant.

  • While fixing errors, don't do so blindly, but make sure the lifetime in Message makes sense -- it either matches an input lifetime on a parameter that it is borrowing from, or if the Message<'_> is created from a literal "str", that would be Message<'static>

  • In particular this may be the fix

    fn make(&self) -> Element<Message<'static>> {

Speculative playground. Change make to return Element<Message<'_>> again to see the error.

3 Likes
5

Sorry I copied the wrong snippet. Have corrected that.

6

Length is from iced::core and does implement copy (comments removed)

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum Length {
    Fill,
    FillPortion(u16),
    Shrink,
    Fixed(f32),
}
7

Is the issue that Builder {w, h} is temporary?

What happens if you assign it to a variable?:

let builder = Builder{w, h};
let b_one = builder.make();

Now b_one can reference builder, and you need to not drop builder while b_one is still live/in scope.

I played with this in a playground and it doesn't seem so. I think quinedot is onto something (usually a safe bet :slight_smile:)

As jumpnbrownweasel says

That will help figure out the real problem.

1 Like
8

You're suspicions are correct introducing a variable did not help.

I also did as @quinedot suggested, but went into a deep rabbit hole of pain. :frowning:

9

Output from cargo check

 ~  RustroverProjects  MyCalculator  cargo check                                                                             101   master  
    Checking my_calculator v0.1.0 (/home/trevor/RustroverProjects/MyCalculator)
error[E0515]: cannot return value referencing temporary value
   --> src/ui/window.rs:194:9
    |
130 |           let b_one = Builder{w, h}.make();
    |                       ------------- temporary value created here
...
194 | /         container(col_all)
195 | |             .width(Length::Fill)
196 | |             .height(Length::Fill)
197 | |             .style(move |_theme, _status| {
...   |
210 | |             })
211 | |             .padding(10)
    | |________________________^ returns a value referencing data owned by the current function

For more information about this error, try `rustc --explain E0515`.
error: could not compile `my_calculator` (bin "my_calculator") due to 1 previous error
10

This is not a major issue for me, I was just trying to make the code more flexible and straight forward. I just am really puzzled that two methods that return what I see as exactly the same item behave so differently, with one working fine and one not compiling.

11

Is just this change possible?

fn make(&self) -> Element<Message<'static>> {

I don't think we've seen the definition of Message (or Element) yet.

12

Message is just a simplish Enum following the Iced patter

use iced::widget::text_editor::Action;

#[derive(Debug, Clone)]
pub enum Message {
    WindowResized(u32, u32),
    Char(String),
    EditorAction(Action),
    Func(String),
    Move(i32),
    BackSpace,
    Clear,
    Evaluate,
    ToggleMode,
}

Element is a generic Widget wrapper from Iced and is defined as

#[allow(missing_debug_implementations)]
pub struct Element<'a, Message, Theme, Renderer> {
    widget: Box<dyn Widget<Message, Theme, Renderer> + 'a>,
}

The method I'm writing implements a core Iced pattern

/// The view logic of some [`Program`].
///
/// This trait allows the [`program`] builder to take any closure that
/// returns any `Into<Element<'_, Message>>`.
pub trait View<'a, State, Message, Theme> {
    /// Produces the widget of the [`Program`].
    fn view(&self, state: &'a State) -> impl Into<Element<'a, Message, Theme>>;
}

Trying

fn make(&self) -> Element<Message<'static>> {

results in a whole heap of lifetime errors

13

It looks like Message has no parameters, so indeed Message<'static> is not going to result in anything but errors.

It seems Element is the one that has an elided lifetime in this signature:

fn make(&self) -> Element<Message> {
// aka
fn make(&self) -> Element<'_, Message> {
// aka
fn make<'s>(&'s self) -> Element<'s, Message> {

Though we must be talking about this alias that has defaults for the last two parameters, if the signature for make is accurate.

I'm not sure it will work, but you could try

fn make(&self) -> Element<'static, Message> {

instead.

1 Like
14

From this error message we can see that b_one is referenced by the last expression that starts with container(col_all). Therefore, there is something between line 130 and line 194 that is creating this reference, but since you haven't shown the code for this function I don't know what it is.

Edit: But I guess this isn't so relevant, since we know that Builder::make is the culprit somehow.

Since Builder contains fields that are always passed by value (not by reference), you could try an owned value for self by removing the &:

    fn make(self) -> Element<Message> {  // `self` not `&self`

I hacked together a similar situation where I get the same error that you got, and to fix it I did have to use a 'static lifetime (in addition to making self owned):

            fn make(self) -> Element<'static, Message> {

That worked in my small example, but it probably means you would have to change the lifetime of Element to 'static in other places in your code.

If you could post enough code that we can all build it, I suspect someone will give you a fix fairly quickly. Unless a problem is very simple, it doesn't work very well to try to fix things from incomplete snippets.

Playing around with this some more, I think the lifetimes in iced are so complex that when adding another abstraction like Builder you're going to have to define more of the lifetimes explicitly. Namely, explicitly use 'static for Element everywhere. make_button is returning Element with a `'static' lifetime, but it can be elided ... until you add the Builder that is.

15

Add this to the top of your src/lib.rs:

#![warn(elided_lifetimes_in_paths)]

and the compiler will warn you when using types that are borrowing data from their context, which can prevent them from being returned from functions.

2 Likes
16

Thanks @jumpnbrownweasel, A couple more 'static lifetimes did the trick, and also gave me a better understanding of what they really mean, so win - win.

Final code ended up as:

        let b_one = ButtonBuilder::new("1", w, h).make();
        let b_mult = ButtonBuilder::new("x", w, h).msg(Message::Char("*".to_string())).make();
etc....

struct ButtonBuilder<'a> {
    name : &'a str,
    w : Length,
    h : Length,
    msg : Option<Message>
}
impl <'a> ButtonBuilder<'static> {

    fn new(name: &'static str, w: Length, h: Length) -> Self {
        Self {name, w, h , msg: None}
    }

    fn msg(&mut self, msg : Message) -> &mut ButtonBuilder<'static> {
        self.msg = Some(msg);
        self
    }
    fn make(&self) -> Element<'static, Message> {
        make_button(self.w, self.h, self.name, self.msg.clone().unwrap_or(Message::Char(self.name.to_string())))
    }
}


/// Make a button in a container that centers it
fn make_button(width: Length, height: Length, name: &str, msg: Message) -> Element<Message> {
    let container = Container::new(name)
        .align_x(Horizontal::Center)
        .align_y(Vertical::Center)
        .clip(false);

    Button::new(container)
        .width(width)
        .height(height)
        .style(|_theme, status| {
            crate::ui::window::get_style(status)
        })
        .on_press(msg)
        .into()
}

The only bit I'm not completely happy with is needing to clone the Option before using it, but I can work on that.

17

ChatGPT helped me with that problem - use take()

fn make(&mut self) -> Element<'static, Message> {
    make_button(self.w, self.h, self.name, self.msg.take().unwrap_or(Message::Char(self.name.to_string())))
}
18

Glad it worked out.

On the cloning of msg, the other solution is to define make(self) instead of make(&self). This will consume self, including the msg, so you don't need clone or take, only unwrap_or_else. I would use unwrap_or_else instead of unwrap_or since the latter will create the String from the name even when it isn't needed.

Consuming self is common for builder methods in general, because often the idea is to accumulate a set of owned objects and then transfer them to the final output object when make is called. Then you don't need the builder object anymore, which is why you can consume it in make.

(ChatGPT doesn't know everything. :wink: )

Something like this (untested):

fn make(mut self) -> Element<'static, Message> {
    make_button(
        self.w,
        self.h,
        self.name,
        self.msg.unwrap_or_else(|| {
            Message::Char(self.name.to_string())
        }),
    )
}
3 Likes
19

@jumpnbrownweasel , Your suggestion to consume self led me to another round of thinking to consume self for the whole builder pattern. This removed all the borrowing and the problems as well.
Removing 'static could be important down the track if I want to translate the UI and as I now don't need to only use static string constants.

For the benefit of anybody who stumbles on this the final code is.

        let b_one = ButtonBuilder::new("1", w, h).make();
        let b_mult = ButtonBuilder::new("x", w, h).msg(Message::Char("*".to_string())).make();
etc...

struct ButtonBuilder<'a> {
    name : &'a str,
    w : Length,
    h : Length,
    msg : Option<Message>,
    colors : Option<(Color, Color)>,
}
impl <'a> ButtonBuilder<'a> {

    fn new(name: &'a str, w: Length, h: Length) -> Self {
        Self {name, w, h , msg: None, colors: None}
    }

    fn for_func(name: &'a str, w: Length, h: Length) -> Self {
        Self {name, w, h, msg: Some(Message::Func(name.to_string())), colors: None}
    }
    fn msg(mut self, msg : Message) -> Self {
        self.msg = Some(msg);
        self
    }

    fn colors(mut self, colors : (Color, Color)) -> Self {
        self.colors = Some(colors);
        self
    }

    fn make(self) -> Element<'a, Message> {
        make_button(self.w, self.h, self.name,
                    self.msg.unwrap_or_else(|| {Message::Char(self.name.to_string())}),
                    self.colors.unwrap_or_else(|| {(Color::from_rgb8(0x24, 0x24, 0x24), Color::from_rgb8(0x55, 0x55, 0x55))}))
    }
}


/// Make a button in a container that centers it
fn make_button(width: Length, height: Length, name: &str, msg: Message, colors: (Color, Color)) -> Element<Message> {
    let container = Container::new(name)
        .align_x(Horizontal::Center)
        .align_y(Vertical::Center)
        .clip(false);

    Button::new(container)
        .width(width)
        .height(height)
        .style(move |_theme, status| {
            get_style(status, colors)
        })
        .on_press(msg)
        .into()
}
5 Likes
20

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.