rust-lang / project-stable-mir

Define compiler intermediate representation usable by external tools
Other
57 stars 12 forks source link

Provide proper encapsulation of StableMIR APIs #56

Open celinval opened 9 months ago

celinval commented 9 months ago

The current crate structure of StableMIR is making it very hard to properly encapsulate our stable structures. All the internal details of stable items have to somehow be exposed in order to allow rustc_smir to build them.

For example, the definition of Ty exposes its internal representation:

pub struct Ty(pub usize);

There is no mechanism for us today to ensure that this usize corresponds to an index inside StableMIR. The Context trait is also exposed, even though we expect users to never invoke them directly.

The main problems I see with the lack of encapsulation are:

  1. Usability. It's confusing for users to know exactly what they can rely on, and what they shouldn't.
  2. Defining the boundaries of what is semantically versioned.
  3. APIs are very fragile. Users can easily make a mistake that will break a type invariant, which will result in one of the following: a. Their code will trigger an ICE b. The API may return inconsistent result c. We will have to constantly check for these invariants ourselves as much as possible and return Result for everything we do.

We should investigate what is the best mechanism to fix this.

### Tasks
- [ ] Enable proper encapsulation (merge crates?)
- [ ] Audit the StableMIR code and change the visibility of items that shouldn't be public
celinval commented 9 months ago

One possible solution is to revert the crate split that was done before. I believe the main motivation was to ensure we don't use internal constructs inside the Stable APIs. But I think it is easier for us to ensure that this doesn't happen, than ensuring that our users know which things are meant to be public only to the compiler.

I also think it is much easier to make things public later, than the opposite direction.

celinval commented 7 months ago

We have other structures that have the same issue. Not just things that wrap usize.

lolbinarycat commented 2 months ago

why not define the types in rustc_smir, then reexport them in stable_mir? that way they can have proper private fields.

generally speaking, types should be defined alongside their constructor.