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

Use GHC Name to represent resolved type constructors #2407

Merged
merged 19 commits into from
Oct 24, 2024

Conversation

facundominguez
Copy link
Collaborator

@facundominguez facundominguez commented Oct 24, 2024

This PR creates a type LHName that is used to represent unresolved names, or resolved names that correspond to Haskell entities (GHC Name), or names for entities only known to LH (like type aliases), or local names (like parameters of type aliases). While LHNames can be used to name all things in LH, this PR only uses them for type constructors.

At first, I tried to parameterize every type in LH by the type of the name, but it was a lot of work to update all the functions, and multiple passes to update the types would have been necessary as the renaming is implemented piece wise. I opted instead to insert the LHNames in the AST without any attempt at changing the type parameters. Keeping track of the state of name resolution in the type of AST expressions can be done separately.

The commit 41fad95 introduces the type LHName.

The commit 3035033 implements the serialization and resolution of these names using generic traversals.

The commit 7c3c213 introduces the type LHName in the AST to represent names of type constructors. This commit doesn't change the behavior of the code, because whenever LH used Symbols for type constructors, I now insert an LHName, and the LHName contains the old Symbol for all purposes.

The next 9 commits modify bit by bit the usage sites of the old Symbols, and replace them with uses of the resolved names inside LHName. This has been valuable in practice to detect quickly when things broke.

Making use of GHC Name for type constructors required using LHNames for type constructors in embed directives, which I had skipped at first, and it is now implemented in 178aee8.

See how we need to qualify less in the LHAssumptions modules in the update in df2a21d. All type constructors are picked from the names in scope only, and most importantly, these choices are recorded when serializing the specs to interface files. Note how the test that we had originally identified for this flaw is now passing (commit 6857a42)!

@facundominguez
Copy link
Collaborator Author

bot
top


matchTyCon :: Env -> Located LHName -> Lookup Ghc.TyCon
matchTyCon env lc@(Loc _ _ c0) = unsafePerformIO $ do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using unsafePerformIO here to lookup the TyThing of a Name. I think this is a benign use of unsafePerformIO as the result doesn't depend of when exactly this is called, as long ghc hasn't been somehow finalized.

Alternatively, it would be possible to look for the TyThing in advance, in a monadic context. The TyThing would need to be stored somewhere until it is needed and it interferes a bit with the generic traversals as it has no Data instance.

@facundominguez facundominguez merged commit 7fb9dc3 into develop Oct 24, 2024
4 checks passed
@facundominguez facundominguez deleted the fd/ghc-names branch October 24, 2024 18:08
@ranjitjhala
Copy link
Member

Thanks @facundominguez -- this is a really big deal!!!

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