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

Improve visibility of subsequently defined contracts #298

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion analyzer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,12 @@ impl From<Shared<ContractScope>> for ContractAttributes {

let external_contracts = scope.borrow().get_module_type_defs(|typ| {
if let Type::Contract(contract) = typ {
Some(contract.to_owned())
// Skip our own contract
if contract.name == scope.borrow().name {
None
} else {
Some(contract.to_owned())
}
} else {
None
}
Expand Down
16 changes: 9 additions & 7 deletions analyzer/src/namespace/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub struct ModuleScope {

#[derive(Clone, Debug, PartialEq)]
pub struct ContractScope {
pub name: String,
pub parent: Shared<ModuleScope>,
pub interface: Vec<String>,
pub event_defs: HashMap<String, Event>,
Expand Down Expand Up @@ -107,8 +108,9 @@ impl ModuleScope {
}

impl ContractScope {
pub fn new(parent: Shared<ModuleScope>) -> Shared<Self> {
pub fn new(name: &str, parent: Shared<ModuleScope>) -> Shared<Self> {
Rc::new(RefCell::new(ContractScope {
name: name.to_owned(),
parent,
function_defs: HashMap::new(),
event_defs: HashMap::new(),
Expand Down Expand Up @@ -352,7 +354,7 @@ mod tests {
#[test]
fn test_scope_resolution_on_first_level_block_scope() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
assert_eq!(block_scope_1, block_scope_1.borrow().function_scope());
assert_eq!(contract_scope, block_scope_1.borrow().contract_scope());
Expand All @@ -361,7 +363,7 @@ mod tests {
#[test]
fn test_scope_resolution_on_second_level_block_scope() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
let block_scope_2 =
BlockScope::from_block_scope(BlockScopeType::IfElse, Rc::clone(&block_scope_1));
Expand All @@ -372,7 +374,7 @@ mod tests {
#[test]
fn test_1st_level_def_lookup_on_1st_level_block_scope() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
block_scope_1
.borrow_mut()
Expand All @@ -387,7 +389,7 @@ mod tests {
#[test]
fn test_1st_level_def_lookup_on_2nd_level_block_scope() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
let block_scope_2 =
BlockScope::from_block_scope(BlockScopeType::IfElse, Rc::clone(&block_scope_1));
Expand All @@ -404,7 +406,7 @@ mod tests {
#[test]
fn test_2nd_level_def_lookup_on_1nd_level_block_scope_fails() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
let block_scope_2 =
BlockScope::from_block_scope(BlockScopeType::IfElse, Rc::clone(&block_scope_1));
Expand All @@ -418,7 +420,7 @@ mod tests {
#[test]
fn test_inherits_type() {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
let block_scope_1 = BlockScope::from_contract_scope("", Rc::clone(&contract_scope));
assert_eq!(
true,
Expand Down
2 changes: 1 addition & 1 deletion analyzer/src/traversal/assignments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ mod tests {
// - bar: u256[100]
fn scope() -> Shared<BlockScope> {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
contract_scope
.borrow_mut()
.add_field(
Expand Down
46 changes: 35 additions & 11 deletions analyzer/src/traversal/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ pub fn contract_def(
module_scope: Shared<ModuleScope>,
context: Shared<Context>,
stmt: &Spanned<fe::ModuleStmt>,
) -> Result<(), SemanticError> {
) -> Result<Shared<ContractScope>, SemanticError> {
if let fe::ModuleStmt::ContractDef { name, body } = &stmt.node {
let contract_scope = ContractScope::new(Rc::clone(&module_scope));
let contract_scope = ContractScope::new(name.node, Rc::clone(&module_scope));

for stmt in body.iter() {
match &stmt.node {
fe::ContractStmt::ContractField { .. } => {
contract_field(Rc::clone(&contract_scope), stmt)
// Contract fields are evaluated in the next pass together with function bodies
// so that they can use other contract types that may only be defined after the
// current contract.
Ok(())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@g-r-a-n-t since fields aren't in the public API of contracts it omitting them here doesn't interfere with the contract representation in the scope. But by delaying analysis to the next pass we can at least support referring to subsequent contracts in contract fields.

}
fe::ContractStmt::EventDef { .. } => event_def(Rc::clone(&contract_scope), stmt),
fe::ContractStmt::FuncDef { .. } => {
Expand All @@ -46,13 +49,6 @@ pub fn contract_def(
.map_err(|error| error.with_context(stmt.span))?;
}

for stmt in body.iter() {
if let fe::ContractStmt::FuncDef { .. } = &stmt.node {
functions::func_body(Rc::clone(&contract_scope), Rc::clone(&context), stmt)
.map_err(|error| error.with_context(stmt.span))?;
};
}

let contract_attributes = ContractAttributes::from(Rc::clone(&contract_scope));

contract_scope
Expand All @@ -63,10 +59,38 @@ pub fn contract_def(
name.node,
Type::Contract(Contract {
name: name.node.to_owned(),
functions: contract_attributes.public_functions.clone(),
functions: contract_attributes.public_functions,
}),
);

return Ok(contract_scope);
}

unreachable!()
}

/// Gather context information for fields and function bodies of contracts.
/// Gathering this information is deferred to allow contracts to refer to other
/// contract types that are defined after it.
pub fn contract_body(
contract_scope: Shared<ContractScope>,
context: Shared<Context>,
stmt: &Spanned<fe::ModuleStmt>,
) -> Result<(), SemanticError> {
if let fe::ModuleStmt::ContractDef { body, .. } = &stmt.node {
for stmt in body.iter() {
if let fe::ContractStmt::ContractField { .. } = &stmt.node {
contract_field(Rc::clone(&contract_scope), stmt)?;
};

if let fe::ContractStmt::FuncDef { .. } = &stmt.node {
functions::func_body(Rc::clone(&contract_scope), Rc::clone(&context), stmt)
.map_err(|error| error.with_context(stmt.span))?;
};
}

let contract_attributes = ContractAttributes::from(Rc::clone(&contract_scope));

context.borrow_mut().add_contract(stmt, contract_attributes);

return Ok(());
Expand Down
2 changes: 1 addition & 1 deletion analyzer/src/traversal/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ mod tests {

fn scope() -> Shared<BlockScope> {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
BlockScope::from_contract_scope("", contract_scope)
}

Expand Down
2 changes: 1 addition & 1 deletion analyzer/src/traversal/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ mod tests {

fn scope() -> Shared<BlockScope> {
let module_scope = ModuleScope::new();
let contract_scope = ContractScope::new(module_scope);
let contract_scope = ContractScope::new("", module_scope);
BlockScope::from_contract_scope("", contract_scope)
}

Expand Down
2 changes: 1 addition & 1 deletion analyzer/src/traversal/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ mod tests {

fn scope() -> Shared<ContractScope> {
let module_scope = ModuleScope::new();
ContractScope::new(module_scope)
ContractScope::new("", module_scope)
}

fn analyze(scope: Shared<ContractScope>, src: &str) -> Context {
Expand Down
15 changes: 14 additions & 1 deletion analyzer/src/traversal/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,33 @@ use std::rc::Rc;
pub fn module(context: Shared<Context>, module: &fe::Module) -> Result<(), SemanticError> {
let scope = ModuleScope::new();

let mut contracts = vec![];

for stmt in module.body.iter() {
match &stmt.node {
fe::ModuleStmt::TypeDef { .. } => type_def(Rc::clone(&scope), stmt)?,
fe::ModuleStmt::StructDef { name, body } => {
structs::struct_def(Rc::clone(&scope), name.node, body)?
}
fe::ModuleStmt::ContractDef { .. } => {
contracts::contract_def(Rc::clone(&scope), Rc::clone(&context), stmt)?
// Collect contract statements and the scope that we create for them. After we
// have walked all contracts once, we walk over them again for a
// more detailed inspection.
let contract_scope =
contracts::contract_def(Rc::clone(&scope), Rc::clone(&context), stmt)?;
contracts.push((stmt, contract_scope))
}
fe::ModuleStmt::FromImport { .. } => unimplemented!(),
fe::ModuleStmt::SimpleImport { .. } => unimplemented!(),
}
}

for (stmt, scope) in contracts.iter() {
if let fe::ModuleStmt::ContractDef { .. } = stmt.node {
contracts::contract_body(Rc::clone(&scope), Rc::clone(&context), stmt)?
}
}

Ok(())
}

Expand Down
4 changes: 1 addition & 3 deletions compiler/tests/fixtures/demos/uniswap.fe
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ contract UniswapV2Pair:

# if fee is on, mint liquidity equivalent to 1/6th of the growth in sqrt(k)
def _mint_fee(reserve0: u256, reserve1: u256) -> bool:
# TODO: The interfaces of subsequently defined contracts should be visible (https://github.com/ethereum/fe/issues/283)
# fee_to: address = UniswapV2Factory(factory).fee_to()
fee_to: address = address(0)
fee_to: address = UniswapV2Factory(self.factory).fee_to()
fee_on: bool = fee_to != address(0)
k_last: u256 = self.k_last # gas savings
if fee_on:
Expand Down
12 changes: 12 additions & 0 deletions compiler/tests/fixtures/features/two_contracts.fe
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
contract Foo:

other: Bar

pub def external_bar() -> u256:
return Bar(address(0)).bar()

pub def foo() -> u256:
return 42

contract Bar:

other: Foo

pub def external_foo() -> u256:
return Foo(address(0)).foo()

pub def bar() -> u256:
return 26
25 changes: 25 additions & 0 deletions newsfragments/298.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
Make subsequently defined contracts visible.

Before this change:

```
# can't see Bar
contract Foo:
...
# can see Foo
contract Bar:
...
```

With this change the restriction is lifted and the following becomes possible.

```
contract Foo:
bar: Bar
pub def external_bar() -> u256:
return self.bar.bar()
contract Bar:
foo: Foo
pub def external_foo() -> u256:
return self.foo.foo()
```