flux-rs / flux

Refinement Types for Rust
MIT License
645 stars 20 forks source link

Improve error message when a field is accessed in an opaque struct #835

Open nilehmann opened 3 weeks ago

nilehmann commented 3 weeks ago

The error message could explain why the field of an opaque struct cannot be accessed and suggest using #[flux::trusted].

Example of someone confused about it

Relevant pointers:

vrindisbacher commented 3 days ago

@nilehmann, I can take this if you'd like?

nilehmann commented 3 days ago

@vrindisbacher that'd be great! One extra pointer. We use rustc's infrastructure for reporting errors which is documented here https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-structs.html

vrindisbacher commented 2 days ago

@nilehmann, how does this look?

File:

#![allow(unused)]
#![allow(dead_code)]

#[flux_rs::opaque]
struct MyStruct {
    inner: u32,
}

impl MyStruct {
    pub fn inner(&self) -> u32 {
        self.inner
    }
}

Output:

error[E0999]: cannot access fields of opaque struct `MyStruct`.
  --> ../opaque_struct.rs:13:9
   |
13 |         self.inner
   |         ^^^^^^^^^^
-Ztrack-diagnostics: created at crates/flux-refineck/src/lib.rs:108:78
   |
help: If you'd like to use fields of `MyStruct` in methods, use `flux::trusted` instead of `flux::opaque`.
  --> ../opaque_struct.rs:7:1
   |
7  | struct MyStruct {
   | ^^^^^^^^^^^^^^^

error: aborting due to 1 previous error
nilehmann commented 3 hours ago

@vrindisbacher, this wasn't clear in the issue description, but the way to solve a cannot access fields of opaque struct is to trust the function accessing the field. In your example above you'd need to trust fn inner:

#[flux_rs::opaque]
struct MyStruct {
    inner: u32,
}

impl MyStruct {
   #[flux::trusted]
    pub fn inner(&self) -> u32 {
        self.inner
    }
}

If you mark a struct as opaque, the only thing you can do in untrusted code is "pass it around". The expectation is that the module defining the opaque struct will define a trusted API, and clients will use it without accessing the fields and only through the API.

Not sure what's the best way to convey this in the error message.

vrindisbacher commented 2 hours ago

Ah I see. Perhaps, the best way to do this is to have a help span over the function suggesting they add trusted and then point them to some documentation (maybe in the flux book) about use of opaque?

nilehmann commented 20 minutes ago

Pointing to documentation would be cool, we would have to write it though XD

So ideally, we could have something like

error[E0999]: cannot access fields of opaque struct `MyStruct`.
  --> ../opaque_struct.rs:13:9
   |
13 |         self.inner
   |         ^^^^^^^^^^
-Ztrack-diagnostics: created at crates/flux-refineck/src/lib.rs:108:78
   |
help: try annotating this method with `#[flux::trusted]`
  --> ../opaque_struct.rs:7:1
   |
12 | pub fn inner(&self) -> u32 {
   |        ^^^^^
 note: fields of opaque structs can only be accessed inside trusted code
 note: to know more about opaque http://flux-rs.github.io/....
vrindisbacher commented 18 minutes ago

Ya that's exactly what I was thinking. I'm happy to write the documentation as part of this if you want.