Skip to content

Commit

Permalink
correct keyword versioning (#622)
Browse files Browse the repository at this point in the history
- Split every `KeywordItem` into multiple `KeywordDefinition`s, each
having their own versioning. This is to account for things like
`fixed8x8` and `fixed256x80` being reserved/unreserved in different
versions.
- Duplicate keywords between Solidity and Yul, since the same keyword
can be reserved in different versions between the two.
- Now all keywords have correct versioning.
- Added a script that will fetch `solc` binaries and test all variants
of keywords (10777) against the actual compiler output. It has a few
hacks around parsing/skipping errors, so it is not stable enough (yet)
to run in CI/unguided, but it definitely helped running locally and
observing the output.
  • Loading branch information
OmarTawfik authored Oct 27, 2023
1 parent 1f6c8c9 commit e37d803
Show file tree
Hide file tree
Showing 29 changed files with 3,803 additions and 302 deletions.
589 changes: 586 additions & 3 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ members = [
"crates/solidity/outputs/spec",
"crates/solidity/testing/sanctuary",
"crates/solidity/testing/snapshots",
"crates/solidity/testing/solc",
"crates/solidity/testing/utils",
]

Expand Down Expand Up @@ -60,6 +61,7 @@ solidity_npm_package = { path = "crates/solidity/outputs/npm/package" }
solidity_spec = { path = "crates/solidity/outputs/spec" }
solidity_testing_sanctuary = { path = "crates/solidity/testing/sanctuary" }
solidity_testing_snapshots = { path = "crates/solidity/testing/snapshots" }
solidity_testing_solc = { path = "crates/solidity/testing/solc" }
solidity_testing_utils = { path = "crates/solidity/testing/utils" }
#
# External
Expand All @@ -85,6 +87,7 @@ proc-macro2 = { version = "1.0.53" }
quote = { version = "1.0.26" }
rayon = { version = "1.7.0" }
regex = { version = "1.9.1" }
reqwest = { version = "0.11.22", features = ["blocking"] }
schemars = { version = "0.8.12", features = ["derive", "preserve_order"] }
semver = { version = "1.0.17", features = ["serde"] }
serde = { version = "1.0.158", features = ["derive", "rc"] }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use crate::{
compiler::{
analysis::{Analysis, ItemMetadata},
versions::{VersionRange, VersionSet},
},
compiler::analysis::{Analysis, ItemMetadata},
internals::Spanned,
spanned::Item,
Identifier,
model::{spanned::Item, Identifier},
utils::{VersionRange, VersionSet},
};
use semver::Version;
use std::{collections::HashSet, fmt::Debug};
Expand Down Expand Up @@ -142,11 +139,11 @@ fn calculate_defined_in(analysis: &mut Analysis, item: &Item) -> VersionSet {
Item::Trivia { item: _ } => {
VersionSet::from_range(calculate_enablement(analysis, &None, &None))
}
Item::Keyword { item } => VersionSet::from_range(calculate_enablement(
analysis,
&item.enabled_in,
&item.disabled_in,
)),
Item::Keyword { item } => {
VersionSet::from_ranges(item.definitions.iter().map(|definition| {
calculate_enablement(analysis, &definition.enabled_in, &definition.disabled_in)
}))
}
Item::Token { item } => {
VersionSet::from_ranges(item.definitions.iter().map(|definition| {
calculate_enablement(analysis, &definition.enabled_in, &definition.disabled_in)
Expand Down
16 changes: 8 additions & 8 deletions crates/codegen/language/definition/src/compiler/analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ mod reachability;
mod references;

use crate::{
compiler::{
analysis::{
definitions::analyze_definitions, reachability::analyze_reachability,
references::analyze_references,
},
versions::VersionSet,
compiler::analysis::{
definitions::analyze_definitions, reachability::analyze_reachability,
references::analyze_references,
},
internals::{ErrorsCollection, ParseOutput, Spanned},
spanned::{Item, ItemKind, Language},
Identifier,
model::{
spanned::{Item, ItemKind, Language},
Identifier,
},
utils::VersionSet,
};
use indexmap::IndexMap;
use proc_macro2::Span;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
compiler::{analysis::Analysis, versions::VersionSet},
spanned::TriviaParser,
Identifier,
compiler::analysis::Analysis,
model::{spanned::TriviaParser, Identifier},
utils::VersionSet,
};
use std::collections::HashSet;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use crate::{
compiler::{
analysis::Analysis,
versions::{VersionRange, VersionSet},
},
compiler::analysis::Analysis,
internals::Spanned,
spanned::{
EnumItem, EnumVariant, Field, FieldKind, FragmentItem, Item, ItemKind, KeywordItem,
PrecedenceExpression, PrecedenceItem, PrecedenceOperator, PrimaryExpression, RepeatedItem,
Scanner, SeparatedItem, StructItem, TokenDefinition, TokenItem, TriviaItem, TriviaParser,
model::{
spanned::{
EnumItem, EnumVariant, Field, FieldKind, FragmentItem, Item, ItemKind,
KeywordDefinition, KeywordItem, PrecedenceExpression, PrecedenceItem,
PrecedenceOperator, PrimaryExpression, RepeatedItem, Scanner, SeparatedItem,
StructItem, TokenDefinition, TokenItem, TriviaItem, TriviaParser,
},
Identifier,
},
Identifier,
utils::{VersionRange, VersionSet},
};
use indexmap::IndexMap;
use itertools::Itertools;
Expand Down Expand Up @@ -307,15 +308,9 @@ fn check_keyword(analysis: &mut Analysis, item: &KeywordItem, enablement: &Versi
let KeywordItem {
name,
identifier,
enabled_in,
disabled_in,
reserved_in,
unreserved_in,
value: _,
definitions,
} = item;

let enablement = update_enablement(analysis, &enablement, &enabled_in, &disabled_in);

check_reference(
analysis,
Some(name),
Expand All @@ -324,7 +319,19 @@ fn check_keyword(analysis: &mut Analysis, item: &KeywordItem, enablement: &Versi
ReferenceFilter::Tokens,
);

check_version_pair(analysis, reserved_in, unreserved_in);
for definition in definitions {
let KeywordDefinition {
enabled_in,
disabled_in,
reserved_in,
unreserved_in,
value: _,
} = &**definition;

let _ = update_enablement(analysis, &enablement, &enabled_in, &disabled_in);

check_version_pair(analysis, reserved_in, unreserved_in);
}
}

fn check_token(analysis: &mut Analysis, item: &TokenItem, enablement: &VersionRange) {
Expand Down
2 changes: 1 addition & 1 deletion crates/codegen/language/definition/src/compiler/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl LanguageEmitter {
pub struct #definition_struct;

impl #definition_struct {
pub fn create() -> codegen_language_definition::Language {
pub fn create() -> codegen_language_definition::model::Language {
return #definition;
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/codegen/language/definition/src/compiler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
mod analysis;
mod emitter;
mod versions;

use crate::{
compiler::{analysis::Analysis, emitter::LanguageEmitter},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
internals::{Error, ErrorsCollection, ParseInputTokens, Result},
spanned::Language,
model::spanned::Language,
};
use proc_macro2::TokenStream;
use syn::parse::{Parse, ParseStream};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
parse_input_tokens::ParseHelpers, Error, ErrorsCollection, ParseInputTokens, Result,
Spanned,
},
Identifier,
model::Identifier,
};
use indexmap::{IndexMap, IndexSet};
use infra_utils::paths::PathExtensions;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
internals::{Spanned, WriteOutputTokens},
Identifier,
model::Identifier,
};
use indexmap::{IndexMap, IndexSet};
use proc_macro2::{Literal, TokenStream};
Expand Down
7 changes: 3 additions & 4 deletions crates/codegen/language/definition/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
mod compiler;
mod internals;
mod model;

pub use compiler::*;
pub use model::*;
pub mod compiler;
pub mod model;
pub mod utils;
25 changes: 10 additions & 15 deletions crates/codegen/language/definition/src/model/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use self::wrapper::*;
/// More information: https://github.com/rust-lang/rust/issues/54726
#[codegen_language_internal_macros::derive_internals]
mod wrapper {
use crate::Identifier;
use crate::model::Identifier;
use indexmap::{IndexMap, IndexSet};
use semver::Version;
use serde::Serialize;
Expand Down Expand Up @@ -218,6 +218,11 @@ mod wrapper {
pub name: Identifier,
pub identifier: Identifier,

pub definitions: Vec<KeywordDefinition>,
}

#[derive(Debug, Eq, PartialEq, Serialize)]
pub struct KeywordDefinition {
pub enabled_in: Option<Version>,
pub disabled_in: Option<Version>,

Expand All @@ -229,20 +234,10 @@ mod wrapper {

#[derive(Debug, Eq, PartialEq, Serialize)]
pub enum KeywordValue {
Sequence {
values: Vec<KeywordValue>,
},
Optional {
value: Box<KeywordValue>,
},
Range {
inclusive_start: usize,
inclusive_end: usize,
increment: usize,
},
Atom {
atom: String,
},
Sequence { values: Vec<KeywordValue> },
Optional { value: Box<KeywordValue> },
Choice { values: Vec<KeywordValue> },
Atom { atom: String },
}

#[derive(Debug, Eq, PartialEq, Serialize)]
Expand Down
121 changes: 121 additions & 0 deletions crates/codegen/language/definition/src/utils/keywords.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
use crate::model::KeywordValue;
use itertools::Itertools;

impl KeywordValue {
/// Collects all possible variations generated by this value.
pub fn collect_variations(&self) -> Vec<String> {
match self {
KeywordValue::Atom { atom } => {
return vec![atom.to_owned()];
}
KeywordValue::Optional { value } => {
let mut results = value.collect_variations();
results.insert(0, String::new());
return results;
}
KeywordValue::Choice { values } => {
return values
.iter()
.flat_map(|value| value.collect_variations())
.collect_vec();
}
KeywordValue::Sequence { values } => {
let matrix = values
.iter()
.map(|value| value.collect_variations())
.collect_vec();

let results_len = matrix.iter().map(|values| values.len()).product();
let mut results = (0..results_len).map(|_| String::new()).collect_vec();

let mut span = results_len;

for variations in matrix {
span /= variations.len();

for j in 0..results_len {
results[j].push_str(&variations[j / span % variations.len()]);
}
}

return results;
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_atom() {
let value = KeywordValue::Atom { atom: "foo".into() };

assert_eq!(value.collect_variations(), vec!["foo"]);
}

#[test]
fn test_optional() {
let value = KeywordValue::Optional {
value: KeywordValue::Atom { atom: "foo".into() }.into(),
};

assert_eq!(value.collect_variations(), vec!["", "foo"]);
}

#[test]
fn test_choice() {
let value = KeywordValue::Choice {
values: vec![
KeywordValue::Atom { atom: "foo".into() },
KeywordValue::Atom { atom: "bar".into() },
],
};

assert_eq!(value.collect_variations(), vec!["foo", "bar"]);
}

#[test]
fn test_sequence() {
let value = KeywordValue::Sequence {
values: vec![
KeywordValue::Atom { atom: "foo".into() },
KeywordValue::Atom { atom: "bar".into() },
],
};

assert_eq!(value.collect_variations(), vec!["foobar"]);
}

#[test]
fn test_all() {
let value = KeywordValue::Sequence {
values: vec![
KeywordValue::Atom { atom: "foo".into() },
KeywordValue::Optional {
value: KeywordValue::Sequence {
values: vec![
KeywordValue::Atom { atom: "_".into() },
KeywordValue::Choice {
values: vec![
KeywordValue::Atom { atom: "1".into() },
KeywordValue::Atom { atom: "2".into() },
KeywordValue::Atom { atom: "3".into() },
KeywordValue::Atom { atom: "4".into() },
KeywordValue::Atom { atom: "5".into() },
],
},
],
}
.into(),
},
],
};

assert_eq!(
value.collect_variations(),
vec!["foo", "foo_1", "foo_2", "foo_3", "foo_4", "foo_5",]
);
}
}
5 changes: 5 additions & 0 deletions crates/codegen/language/definition/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
mod keywords;
mod versions;

pub use keywords::*;
pub use versions::*;
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ impl VersionRange {
pub fn is_empty(&self) -> bool {
return self.inclusive_start == self.exclusive_end;
}

pub fn contains(&self, version: &Version) -> bool {
return &self.inclusive_start <= version && version < &self.exclusive_end;
}
}

impl Display for VersionRange {
Expand Down
Loading

0 comments on commit e37d803

Please sign in to comment.