jam1garner / binrw

A Rust crate for helping parse and rebuild binary data using ✨macro magic✨.
https://binrw.rs
MIT License
585 stars 35 forks source link

I'm strugling to get Args working #213

Closed Thermatix closed 1 year ago

Thermatix commented 1 year ago

I don't know what I'm doing wrong but I just keep getting errors , like "About How you have to use named args when using count", so I switch to named args and then I get "cannot find value count in this scope", followed by:

error[E0599]: `LumpDataBinReadArgBuilder<'_, Needed, Needed>` is not an iterator
  --> src/wad/lumps.rs:15:52
   |
15 |     #[br(if(check_size(&name, size) != 0), count = Self::lump_size(size, &name), seek_before = SeekFrom::Start(offset as u64),  restore_posit...
   |                                                    ^^^^ `LumpDataBinReadArgBuilder<'_, Needed, Needed>` is not an iterator
...
51 | #[derive(Debug, BinRead, Clone)]
   |                 -------
   |                 |
   |                 method `count` not found for this struct
   |                 doesn't satisfy `_: Iterator`
   |
   = note: the following trait bounds were not satisfied:
           `LumpDataBinReadArgBuilder<'_, Needed, Needed>: Iterator`
           which is required by `&mut LumpDataBinReadArgBuilder<'_, Needed, Needed>: Iterator`
note: the trait `Iterator` must be implemented
  --> /home/thermatix/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:73:1
   |
73 | pub trait Iterator {
   | ^^^^^^^^^^^^^^^^^^
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following trait defines an item `count`, perhaps you need to implement it:
           candidate #1: `Iterator`

I got it working once, though it wanted me to do this to pass the args:

args { inner: LumpDataBinArgData { name: &name } } // The name might be slightly off but you get the point

Which isn't in the documentation for args btw.

but then I added an extra layer enum and added the count arg (which I renamed size) and then it stopped working again.

I don't get it, I'm following the docs so why is it complaining?

Gist with code

P.S. I fully concede this issue might not be up to scratch but I'm tired so I'm liable to making mistakes

kitlith commented 1 year ago

Bringing the bits I'm referencing inline

The outermost field:

   #[br(if(size != 0), count = Self::lump_size(size, &name), seek_before = SeekFrom::Start(offset as u64),  restore_position, 
    //args { size: count, name: &name } )]
    args { inner: (count, &name) } )]
    //#[br(args_raw = (count, &name))]

    pub data: Option<LumpData>

LumpData:

#[derive(Debug, BinRead, Clone)]
#[br(little, import { size: usize, name: &str })]
pub enum LumpData {

    #[br(pre_assert(size > 1))] DeserializeLump (
        #[br( count = size)]
        //#[br(args { inner: (size: size, name: &name) })]
        #[br(args { name: &name })]
        Vec<DeserializeLump>
    ),
    #[br(pre_assert(size == 1))] Bytes (
        #[br(count = size)]
        Vec<u8>
    )

}

The first thing to note here is that #[br(count = <whatever>] is always equivilant to #[br(args { count: <whatever> })] these days, and since LumpData no longer takes an arugment named count, that's not going to work. The second thing is that you're trying to use a variable named 'count' in your arguments, which doesn't exist. (because count doesn't define a variable, but just passes an argument.

So, for the outermost field, you probably want to move from count = Self::lump_size(size, &name) to args { size: Self::lump_size(size, &name), <...> }, and/or rename 'size' back to 'count'. (you might also want to move lump_size to the LumpData enum, but I don't know your format/code.)

The second thing to note is passing arguments through a Vec. A Vec is defined as taking VecArgs, which consists of two named arguments: count and inner, where inner is of the type of the arguments taken by the inner value. (this is why count = <blah> works with Vec, because it takes an argument called count).

So, inside LumpData when deserializing the Vec<DeserializeLump> field, you either want to use #[br(count = size, args { inner: args!{ name } })] or #[br(args { count: size, inner: args! { name }})]. These are both equivalent, but I personally think the latter is clearer. (the args macro here is essentially a shortcut for defining a structure that matches the named arugments of the appropriate type.)

(A note: I'm doing this entirely from memory, I haven't actually tested the changes to make sure that they compile. Apologies if they don't.)

Thermatix commented 1 year ago

if this

The first thing to note here is that #[br(count = ] is always equivilant to #[br(args { count: })] these days

is true, why is this not in the documentation? Was it just forgotten to update it?

Amway, considering it kept wanting me to move stuff to inner I'm going to guess you're right.

ALSO thank you so very much for clearing this up for me!

EDIT:

I've updated my code and it's now working, I still have other issues but they're unrelated, I think the fix to this issue is to update the Documentation to reflect the changes because this stuff is definitely not in the documentation, or at least I couldn't find it so make it more discoverable?

kitlith commented 1 year ago

It's in the documentation for the count attribute?

https://docs.rs/binrw/0.11.2/binrw/docs/attribute/index.html#count

Specifically, where it's talking about what it "desugars" to.

Or are you talking about inner? I don't think that'd be documented on the attributes page, since it has nothing to do with attributes, but I can agree that VecArgs (which I linked before) might not be the most discoverable place for it.

Where were you looking for this information/where did you expect to find it?

Thermatix commented 1 year ago

When I click on Args, no where does it mention:

I admit though that I missed what count de-sugars to because I didn't click on it's documentation. I saw it being used in other examples and assumed it was as it was on the tin and didn't think to see if there was anything special about it.

csnover commented 1 year ago

Hi there,

Sorry about any confusion, this is obviously never the goal. There’s no way to inspect types from a proc-macro so I’m unfortunately unaware of a way to give a more helpful diagnostic than what exists.

With regards to the documentation:

If you're using count = some_value you have to use named args

This information is communicated directly by the diagnostic when you try to use count with non-named args. It should be impossible to not receive this diagnostic in this case; if you encountered a situation where the diagnostic did not appear when you tried to use count and non-named arguments, would you please file a new report with a reproducer?

When using count alongside named args you need to pass it like this: args { count: count, inner: args! { some_arg: some_value } }

This information is communicated in the “Limitations” section of the linked documentation. However, this is technically inaccurate/outdated since the desugar today still works so long as the arguments are named arguments, so I have updated it with more accurate information.

No mention of the arg! macro

This macro is mentioned in the second bullet of the start of the linked documentation. However, this bullet point was unclear and lacked an example, so I have updated the documentation to demonstrate the use of this macro. (FWIW, it is not necessary to use the macro, you can also manually construct an inner object.)

Thanks for your feedback and I hope that otherwise you have been having a positive experience using binrw.

Best,