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

v2 merge followup #1040

Open
2 of 4 tasks
sbillig opened this issue Jan 18, 2025 · 7 comments
Open
2 of 4 tasks

v2 merge followup #1040

sbillig opened this issue Jan 18, 2025 · 7 comments

Comments

@sbillig
Copy link
Collaborator

sbillig commented Jan 18, 2025

FYI @g-r-a-n-t @micahscopes @Y-Nak, to avoid duplicate work, here are the minor things I'm planning to do now that v2 has been merged:

@g-r-a-n-t Do you want to put up a basic stdlib pr with parts of #986?
@Y-Nak iirc you expressed some interest in working on the std lib, do you want to take that on?

@g-r-a-n-t
Copy link
Member

g-r-a-n-t commented Jan 18, 2025

A few things:

  • I think we should name this ingot "core" and not add evm stuff to it.
  • The core library should reside at library/core in the repository.
  • For core development purposes, it is better if the driver loads the core library from the filesystem rather than statically loading it in the executable, due to the cargo rebuild triggered by the latter.
  • There should be a --core flag on the cli for setting the core library.
  • There should be a fe home directory (env var: FE_HOME, default: ~/.fe)
  • If the core flag is not set, then the driver will check for a core ingot set in $FE_HOME/config.toml.
  • If it is not set in the home config, then it will try to load it from $FE_HOME/library/core.
  • If that fails, then it will write to $FE_HOME/library/core.

@sbillig
Copy link
Collaborator Author

sbillig commented Jan 19, 2025

@g-r-a-n-t I mostly agree with that list. However, I think it might be better to statically load the core library from the executable rather than the filesystem by default, and allow the user to override this by providing a --core argument (to the fe cli, or fe-language-server), or by setting it in the project or user-level config file. (We should also allow them to set a custom prelude file via similar means).

Using a guaranteed-good copy of the core lib by default avoids potentially confusing or incorrect behavior if the fs copy is modified (intentionally, accidentally, or nefariously).

@sbillig
Copy link
Collaborator Author

sbillig commented Jan 19, 2025

The rust-embed crate (which the language server is/was using to load the old stdlib) will load files from disk in debug builds and from the executable in release builds, meaning working on the core lib wouldn't require rebuilds. https://crates.io/crates/rust-embed

@Y-Nak
Copy link
Member

Y-Nak commented Jan 19, 2025

I’d be happy to work on specific modules of std/core, as I want to check features around HKTs and diagnostics. Does "Bare-bones" include implementing these features as well? In any cases, please feel free to proceed as you feel, I'll ask you before starting working on a specific feature.

@g-r-a-n-t
Copy link
Member

@sbillig that sounds good. So maybe we just start with a PR that contains the following:

  • CLI --core arg and default static loading.
  • Salsa wiring for local and core ingots.
  • A dummy core ingot with a few definitions just to confirm things work.

I'd be happy to pair program on this Monday or work alone. We can go over it on the weekly call and hopefully have it merged.

@Y-Nak I want to confirm something about v2 that came up recently in a meeting. I think it was mentioned at some point that an IngotId cannot be used in multiple InputIngot external_ingot sets. Is this the case? It seems that we should be able to intern InputIngots once and use their IngotIds in the external_ingot sets of multiple InputIngots given that interning takes place in topological order, but maybe I'm missing something.

@Y-Nak
Copy link
Member

Y-Nak commented Jan 19, 2025

@g-r-a-n-t It can be used without any issues. As long as the Ingot dependencies do not form a cycle, it should work fine in v2.

@sbillig
Copy link
Collaborator Author

sbillig commented Jan 20, 2025

@Y-Nak

I’d be happy to work on specific modules of std/core, as I want to check features around HKTs and diagnostics. Does "Bare-bones" include implementing these features as well? In any cases, please feel free to proceed as you feel, I'll ask you before starting working on a specific feature.

I was just thinking I'd put in a few simple things to start, like Option, Result, todo(), etc. At this point I'm just trying to get the scaffolding up basically, so it's easy to add more.

@g-r-a-n-t

@sbillig that sounds good. So maybe we just start with a PR that contains the following:

  • CLI --core arg and default static loading.
  • Salsa wiring for local and core ingots.
  • A dummy core ingot with a few definitions just to confirm things work.

I'd be happy to pair program on this Monday or work alone. We can go over it on the weekly call and hopefully have it merged.

Sounds great. Feel free to get started and I'll check in and see where I can help.

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

No branches or pull requests

3 participants