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

feat(hir): expose hir from parsing context #210

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

grandizzy
Copy link

  • access Hir from calling ParsingContext::parse_and_lower_to_hir

@grandizzy grandizzy marked this pull request as ready for review March 6, 2025 11:01
@@ -25,6 +25,7 @@ mod ast_lowering;
mod ast_passes;

mod parse;
use crate::hir::{Arena, Hir};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should maintain style of the other fns, using hir:: as a module instead of importing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in 9c654cc

@@ -100,6 +101,32 @@ pub fn parse_and_resolve(pcx: ParsingContext<'_>) -> Result<()> {
Ok(())
}

/// Parses and lowers to HIR, recursing into imports.
pub fn parse_and_lower_to_hir<'hir>(
Copy link
Member

@DaniPopes DaniPopes Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to avoid any code dup here and also avoid using Hir directly, but instead through gcx; I think what we can do here is something like this

fn parse_and_resolve() {
    let gcx = *parse_and_lower()?;
    analysis(gcx)?;
    Ok(())
}

fn parse_and_lower<'hir>(pcx: '_, hir_arena: 'hir) -> impl Deref<Target=Gcx<'hir>> {
    // all the code currently in parse_and_resolve ...
    GcxWrapper(...)
}

struct GcxWrapper(GlobalContext);
impl Deref for GcxWrapper {
    // move the unsafe here
}
impl Drop for GcxWrapper {
    // move from OnDrop closure
}

This way we can keep the drop impl and avoid a closure for using Gcx

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaniPopes would the approach in 9c654cc which doesn't introduce a new wrapper work?

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.

2 participants