Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the constructor of salsa::Id to const fn #440

Merged
merged 2 commits into from
Apr 24, 2023

Conversation

Y-Nak
Copy link
Contributor

@Y-Nak Y-Nak commented Apr 12, 2023

This change is motivated to allow interned structs to have default const instances and prefill them in db initialization.

A specific example is managing symbols as a salsa interned struct in a compiler implementation. By declaring keywords as const symbols, we can handle keywords without going through their internal representation as rustc does (ref: rustc_span::symbol).
Conceptually, it'd look like the one below.

#[salsa::interned]
pub struct Symbol {
    data: String,
}

const SELF_SYM: Symbol = Symbol(salsa::Id::from_u32(1))
...

/// This function is called in db initialization.
fn prefill(db: &dyn HirDb) {
       Symbol::new(db, "self".to_string());
       ...
}

@netlify
Copy link

netlify bot commented Apr 12, 2023

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit cbafc30
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/6443a60bb8d2fb00088f0b43

@nikomatsakis
Copy link
Member

bors r+

makes sense!

bors bot added a commit that referenced this pull request Apr 22, 2023
440: Change the constructor of `salsa::Id` to const fn r=nikomatsakis a=Y-Nak

This change is motivated to allow interned structs to have default const instances and prefill them in db initialization.

A specific example is managing symbols as a salsa interned struct in a compiler implementation. By declaring keywords as `const` symbols, we can handle keywords without going through their internal representation as rustc does (ref: [rustc_span::symbol](https://github.com/rust-lang/rust/blob/master/compiler/rustc_span/src/symbol.rs)). 
Conceptually, it'd look like the one below.

```rust
#[salsa::interned]
pub struct Symbol {
    data: String,
}

const SELF_SYM: Symbol = Symbol(salsa::Id::from_u32(1))
...

/// This function is called in db initialization.
fn prefill(db: &dyn HirDb) {
       Symbol::new(db, "self".to_string());
       ...
}
```

Co-authored-by: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 22, 2023

Build failed:

  • Test (stable, false)

@Y-Nak
Copy link
Contributor Author

Y-Nak commented Apr 22, 2023

The build failure originated from the trybuild test failure. This is because of the change of the error message emitted from rustc. I fixed it in the latest commit.
Ref: rust-lang/rust#108324

@xffxff
Copy link
Member

xffxff commented Apr 24, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 24, 2023

Build succeeded:

@bors bors bot merged commit d4a94fb into salsa-rs:master Apr 24, 2023
@Y-Nak Y-Nak deleted the const-fn-id-constructor branch April 24, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants