antoyo / relm

Idiomatic, GTK+-based, GUI library, inspired by Elm, written in Rust
MIT License
2.42k stars 79 forks source link

compilation errors are reported on the "widget" annotation not at the correct location #231

Closed emmanueltouzery closed 3 years ago

emmanueltouzery commented 3 years ago

I'm not sure whether that one is in fact fixable, but it is quite annoying when I hit it (which is relatively often). I get compilation errors like that:

error[E0282]: type annotations needed
   --> projectpad/src/widgets/server_item_list_item.rs:112:1
    |
112 | #[widget]
    | ^^^^^^^^^ this method call resolves to `&T`
    |
    = note: type must be known at this point

So the error is reported on the #[widget] annotation and not at the real location of the error. It can get very annoying, sometimes I had errors like "types don't match" (without any extra info) reported at this location :(

But due to macro use i'm not sure whether it's fixable in relm. If you're not aware of this issue and need a small reproduction, I'll try to produce one.

antoyo commented 3 years ago

I need a small reproduction, because it's not supposed to happen.

Thanks for reporting this issue.

emmanueltouzery commented 3 years ago

hmm I'm sorry, I spent some time trying to isolate the issue in a small reproduction but I gave up for now. I hope it's not too bad that I reproduced it in my full project :-(

if you run cargo build in the relm_231 branch of my app you get the issue in several spots:

error[E0599]: no method named `unwrap` found for struct `gdk::auto::display::Display` in the current scope
   --> projectpad/src/widgets/search_view.rs:133:1
    |
133 | #[widget]
    | ^^^^^^^^^ method not found in `gdk::auto::display::Display`

error[E0282]: type annotations needed
  |
  = note: type must be known at this point

   Compiling projectpad-cli v0.1.0 (/home/emmanuel/home/projectpad-cli/projectpad-cli)
error[E0282]: type annotations needed
   --> projectpad/src/widgets/project_poi_header.rs:169:1
    |
169 | #[widget]
    | ^^^^^^^^^ this method call resolves to `&T`
    |
    = note: type must be known at this point

error[E0282]: type annotations needed
   --> projectpad/src/widgets/server_item_list_item.rs:112:1
    |
112 | #[widget]
    | ^^^^^^^^^ this method call resolves to `&T`
    |
    = note: type must be known at this point

https://github.com/emmanueltouzery/projectpad-cli/tree/relm_231

probably it would be enough to run cargo build in the projectpad subfolder.

antoyo commented 3 years ago

Ok, it seems there's something different in your setup because, when I copy those lines:

        let hand_cursor = gdk::Cursor::new_for_display(
            &self.search_result_area.get_display().unwrap(),
            gdk::CursorType::Hand2,
        );

in the init_view() method of a small example, I get the proper error:

   |
56 |             &self.search_result_area.get_display().unwrap(),
   |                                                    ^^^^^^ method not found in `gdk::Display`

I'll try to understand what is going on.

emmanueltouzery commented 3 years ago

Yes that was my issue, I could not easily isolate the issue in a smaller project. But presumably if you try in the real project you see the issue too?

antoyo commented 3 years ago

Nope. It works in titanium:

   --> src/app/mod.rs:279:28
    |
279 |         handle_error!(self.mdel.bookmark_manager.create_tables());
    |                            ^^^^ help: a field with a similar name exists: `model`
   --> src/app/mod.rs:443:18
    |
443 |             gtk::Bx {
    |                  ^^ could not find `Bx` in `gtk`
emmanueltouzery commented 3 years ago

Yes the errors work 99% of the time, it's not like all errors in the #[widget] don't work. It's exactly these (and in a few other cases where I had such issues). I assume if you checkout & build that project of mine on that branch, you'll reproduce the error. If not I'm very confused.

antoyo commented 3 years ago

Yes, I am able to reproduce your errors in your branch.

antoyo commented 3 years ago

For some reason, if I comment the method fill_popover(), the unwrap() error is shown correctly…

emmanueltouzery commented 3 years ago

it might be a rust compiler bug and not a relm issue... but i really don't know enough to judge :(

antoyo commented 3 years ago

I don't thing it's a compiler bug.

I found out that removing the & in front of &move |btn: &gtk::ModelButton, str_val: String| { gives the proper error for the unwrap() (that, of course, generates a new error).

I'm really not sure what's going on here. Perhaps upgrading to proc-macro2 version 1 would help.

emmanueltouzery commented 3 years ago

to illustrate that this is happening to me routinely, i reproduced another case in another file right now. branch anotherone of the same repo...

error[E0609]: no field `relm` on type `widgets::project_poi_contents::Model`
  --> projectpad/src/widgets/project_poi_contents.rs:28:1
   |
28 | #[widget]
   | ^^^^^^^^^ unknown field
   |
   = note: available fields are: `db_sender`, `cur_project_item`, `project_note_contents`, `pass_popover`
antoyo commented 3 years ago

Is this error in a branch too?

I'd like to take a look at this error (and the other one that wasn't about the unwrap()) to check if there's a common pattern to reproduce this error.

emmanueltouzery commented 3 years ago

yes, I aptly named the branch anotherone :) https://github.com/emmanueltouzery/projectpad-cli/tree/anotherone

antoyo commented 3 years ago

Hum, this one is actually quite weird: a Rust syntax error is causing the other errors to not be shown at the right location.

I'll check, but this one might be a compiler bug.

Edit: Yeah, this one seems to be a bug in either rustc or syn.

Edit 2: And the previous errors about type annotations needed are also shown when removing the & in front of closures. I'll try to figure out how to fix this.

antoyo commented 3 years ago

Strangely, putting the closure (not a reference to it) in a variable and taking the reference of this variable makes the errors be shown at the good location.

antoyo commented 3 years ago

I was able to reproduce this bug (with a reference to a closure) outside of relm. I reported it there.

I'll check if the syntax error can also be reproduce.

antoyo commented 3 years ago

The other problem is also reproducted outside of relm. I reported it there.

So, I'll close this issue when this is fixed and the syn dependency is updated to the fixed version.

emmanueltouzery commented 3 years ago

ok as for myself, I'll stop reporting even if I reproduce it again. Let me just report one final case, I've already hit that one in the past, where not even the #[widget] is highlighted... the compiler reports no location whatsoever for the error.

https://github.com/emmanueltouzery/projectpad-cli/tree/no_info

forgive me for the build warnings. I'm in "rapid iteration" mode...

but in a nutshell the build error is...

error[E0308]: mismatched types

that's it, not location whatsoever.

emmanueltouzery commented 3 years ago

yeah, as you probably saw, longstanding compiler bug.. i guess it's best to close this :-(

emmanueltouzery commented 3 years ago

so, this is caused by https://github.com/rust-lang/rust/issues/43081 -- this is being worked on, for instance in https://github.com/rust-lang/rust/pull/73084 however it seems clear this won't be fixed for some time.

I guess there's no way in relm of using view! without #[widget] right? It's either both or none I guess? Because I love view! but these build errors without location are quite painful :-(

antoyo commented 3 years ago

They are painful. Sometimes, what I do is an additional impl for the widget, which won't use the attribute. The downside of that is that you won't get the assignment to the model update the view.

emmanueltouzery commented 3 years ago

For what it's worth, I've been following the upstream bugs on this, there is slow progress ongoing, and rust 1.49 (released yesterday) has fixed quite a few of these problems. I didn't check the exact examples you've studied, but I see it in my real-world app code. It's WAY BETTER now.

antoyo commented 3 years ago

Let's close this issue, then, since we cannot fix anything in relm anyway.