feat: allow multiple languages for compilers #128
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattsse
requested changes
May 27, 2024
src/compilers/solc/mod.rs
Outdated
Comment on lines
28
to
33
| #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] | ||
| #[non_exhaustive] | ||
| pub enum SolcLanguages { | ||
| Solidity, | ||
| Yul, | ||
| } |
src/compilers/solc/mod.rs
Outdated
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] | ||
| #[non_exhaustive] | ||
| pub enum SolcLanguages { |
There was a problem hiding this comment.
nit:
Suggested change
| pub enum SolcLanguages { | |
| pub enum SolcLanguage { |
src/compilers/mod.rs
Outdated
| @@ -136,45 +206,31 @@ impl<E> Default for CompilerOutput<E> { | |||
| } | |||
| } | |||
|
|
|||
| pub trait Language: Hash + Eq + Clone + Debug { | |||
There was a problem hiding this comment.
should this also be Display and I think this should be 'static
src/compilers/solc/mod.rs
Outdated
| self | ||
| } | ||
| #[derive(Debug, Clone, Serialize)] | ||
| pub struct SolcVerionedInput { |
src/compilers/solc/mod.rs
Outdated
|
|
||
| type Input = SolcInput; | ||
| impl Compiler for SolcRegistry { |
There was a problem hiding this comment.
hmm, it's a bit weird that we implement compiler on registry, maybe we can rename this? so it's more obvious that this is compiler?
There was a problem hiding this comment.
yeah, not sure how to name it
perhaps MultiSolc or just SolcCompiler?
There was a problem hiding this comment.
solc compiler = Sol Compiler Compiler 😀
klkvr
changed the title
mattsse
requested changes
May 30, 2024
mattsse
approved these changes
May 31, 2024
| let mut solc = match self { | ||
| Self::Specific(solc) => solc.clone(), | ||
|
|
||
| #[cfg(feature = "svm-solc")] |
There was a problem hiding this comment.
I think we can consider removing this feature
Merged
klkvr
added a commit
that referenced
this pull request
Jun 5, 2024
ref foundry-rs/foundry#8062 ref https://t.me/foundry_support/54144 introduced in #128
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes
Compilertrait to also have associatedLanguagetype which is supposed to be an enum with languages supported by the compiler. AlsoCompilerVersionManagertrait is removed and compiler now should contain full logic for version resolution.For version management we have to keep track of all different languages and versions to compile them with, thus I've rewritten some of the graph logic to use
HashMap<C::Language, HashMap<Version, Sources>>during compilation.Now, instead of representing a binary wrapper,
Compileris more like binary manager which resolves and installs binaries dynamically depending on requestedlanguageandversion.It's mostly ready, I just need to add logic for compilation with locked compiler version.