rust-embedded / svd2rust

Generate Rust register maps (`struct`s) from SVD files
Apache License 2.0
707 stars 150 forks source link

Register Write Limitation #859

Open AdinAck opened 1 month ago

AdinAck commented 1 month ago

Moved from stm32-rs-nightlies

I ran into an issue when writing to a register where I wanted to return a value from the write, like this:

let val = reg.write(|w| {
    T::set(w.field())
});

But, the signature of write forbids this:

pub fn write<F, T>(&self, f: F)
    where
        F: FnOnce(&mut REG::Writer) -> &mut W<REG>,

So I'm curious, why not change the signature to:

pub fn write<F, T>(&self, f: F) -> T
    where
        F: FnOnce(&mut REG::Writer) -> T

By implementing it like this:

pub fn write<F, T>(&self, f: F) -> T
where
    F: FnOnce(&mut W<REG>) -> T,
{
    let mut writer = W {
        bits: REG::RESET_VALUE & !REG::ONE_TO_MODIFY_FIELDS_BITMAP
            | REG::ZERO_TO_MODIFY_FIELDS_BITMAP,
        _reg: marker::PhantomData,
    };
    let result = f(&mut writer);

    self.register.set(writer.bits);

    result
}

Is there a drawback to this?

burrbull commented 1 month ago

https://github.com/rust-embedded/svd2rust/pull/738#issuecomment-1624819353

AdinAck commented 1 month ago

I don't believe my proposal is related to that. I don't want to return the writer, in fact, the proposed body of write forbids T ever being W or &mut W since bits is moved out of the writer for register.set, no?

AdinAck commented 1 month ago

Ok yes I believe I set up an analogous setup in rust playground:

#[derive(Debug)]
struct Bits(u32);

#[derive(Debug)]
struct W {
    bits: Bits,
    _reg: (),
}

fn consume_bits(Bits(num): Bits) {
    println!("bits: {}", num);
}

fn write<F, T>(f: F) -> T
where
    F: FnOnce(&mut W) -> T,
{
    let mut writer = W { bits: Bits(0), _reg: () };

    let result = f(&mut writer);

    consume_bits(writer.bits);

    result
}

fn main() {
    let outside = write(|_w| 0xaa);
    // let outside = write(|w| w); <- does not compile
    println!("outisde: {:?}", outside);
}

So I don't believe this introduces any complications as mentioned in #738.

burrbull commented 21 hours ago

Sorry for waiting. Looks interesting. I think you could nominate it for next meeting: https://github.com/rust-embedded/wg/discussions/800

AdinAck commented 14 hours ago

No problem, you have a lot oh your hands. I've never done that before so I don't know the proper procedure.