Skip to content

Commit

Permalink
Filter ABI supertrait methods from contract entry generation (#6402)
Browse files Browse the repository at this point in the history
## Description

This PR filters out ABI supertrait methods from being considered for
contract function entry generation.

Closes #6333.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
tritao committed Aug 14, 2024
1 parent 9b87126 commit 4c2c384
Show file tree
Hide file tree
Showing 15 changed files with 172 additions and 24 deletions.
11 changes: 11 additions & 0 deletions sway-core/src/decl_engine/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ impl<T> Into<usize> for DeclId<T> {
}
}

impl<T> DebugWithEngines for DeclId<T>
where
DeclEngine: DeclEngineIndex<T>,
T: Named + Spanned + DebugWithEngines,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: &Engines) -> fmt::Result {
let decl = engines.de().get(self);
DebugWithEngines::fmt(&decl, f, engines)
}
}

impl<T> EqWithEngines for DeclId<T>
where
DeclEngine: DeclEngineIndex<T>,
Expand Down
21 changes: 19 additions & 2 deletions sway-core/src/language/ty/ast_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,32 @@ impl TyAstNode {
})
}

pub fn contract_fns(&self, engines: &Engines) -> Vec<DeclRefFunction> {
pub fn contract_supertrait_fns(&self, engines: &Engines) -> Vec<DeclId<TyFunctionDecl>> {
let mut fns = vec![];

if let TyAstNodeContent::Declaration(TyDecl::ImplSelfOrTrait(decl)) = &self.content {
let decl = engines.de().get(&decl.decl_id);
if decl.is_impl_contract(engines.te()) {
for item in &decl.supertrait_items {
if let TyTraitItem::Fn(f) = item {
fns.push(*f.id());
}
}
}
}

fns
}

pub fn contract_fns(&self, engines: &Engines) -> Vec<DeclId<TyFunctionDecl>> {
let mut fns = vec![];

if let TyAstNodeContent::Declaration(TyDecl::ImplSelfOrTrait(decl)) = &self.content {
let decl = engines.de().get(&decl.decl_id);
if decl.is_impl_contract(engines.te()) {
for item in &decl.items {
if let TyTraitItem::Fn(f) = item {
fns.push(f.clone());
fns.push(*f.id());
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions sway-core/src/language/ty/declaration/impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct TyImplSelfOrTrait {
pub trait_name: CallPath,
pub trait_type_arguments: Vec<TypeArgument>,
pub items: Vec<TyImplItem>,
pub supertrait_items: Vec<TyImplItem>,
pub trait_decl_ref: Option<DeclRefMixedInterface>,
pub implementing_for: TypeArgument,
pub span: Span,
Expand Down Expand Up @@ -75,6 +76,7 @@ impl HashWithEngines for TyImplSelfOrTrait {
// these fields are not hashed because they aren't relevant/a
// reliable source of obj v. obj distinction
span: _,
supertrait_items: _,
} = self;
trait_name.hash(state);
impl_type_parameters.hash(state, engines);
Expand Down
14 changes: 12 additions & 2 deletions sway-core/src/language/ty/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use sway_error::handler::{ErrorEmitted, Handler};
use sway_types::Span;

use crate::{
decl_engine::{DeclEngine, DeclEngineGet, DeclRef, DeclRefFunction},
decl_engine::{DeclEngine, DeclEngineGet, DeclId, DeclRef, DeclRefFunction},
language::{ty::*, HasModule, HasSubmodules, ModName},
semantic_analysis::namespace,
transform::{self, AllowDeprecatedState},
Expand Down Expand Up @@ -123,12 +123,22 @@ impl TyModule {
pub fn contract_fns<'a: 'b, 'b>(
&'b self,
engines: &'a Engines,
) -> impl '_ + Iterator<Item = DeclRefFunction> {
) -> impl '_ + Iterator<Item = DeclId<TyFunctionDecl>> {
self.all_nodes
.iter()
.flat_map(move |node| node.contract_fns(engines))
}

/// All contract supertrait functions within this module.
pub fn contract_supertrait_fns<'a: 'b, 'b>(
&'b self,
engines: &'a Engines,
) -> impl '_ + Iterator<Item = DeclId<TyFunctionDecl>> {
self.all_nodes
.iter()
.flat_map(move |node| node.contract_supertrait_fns(engines))
}

pub(crate) fn check_deprecated(
&self,
engines: &Engines,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
asm_generation::fuel::compiler_constants::MISMATCHED_SELECTOR_REVERT_CODE,
decl_engine::{DeclEngineGet, DeclId, DeclRef},
decl_engine::{DeclEngineGet, DeclId},
engine_threading::SpannedWithEngines,
language::{
parsed::{self, AstNodeContent, Declaration, FunctionDeclarationKind},
Expand Down Expand Up @@ -570,7 +570,7 @@ where
&mut self,
engines: &Engines,
program_id: Option<ProgramId>,
contract_fns: &[DeclRef<DeclId<TyFunctionDecl>>],
contract_fns: &[DeclId<TyFunctionDecl>],
fallback_fn: Option<DeclId<TyFunctionDecl>>,
handler: &Handler,
) -> Result<TyAstNode, ErrorEmitted> {
Expand All @@ -580,7 +580,7 @@ where
let mut writes = false;

for r in contract_fns {
let decl = engines.de().get(r.id());
let decl = engines.de().get(r);

match decl.purity {
Purity::Pure => {}
Expand Down
16 changes: 11 additions & 5 deletions sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl TyImplSelfOrTrait {
implementing_for.type_id,
);

let new_items = type_check_trait_implementation(
let (new_items, supertrait_items) = type_check_trait_implementation(
handler,
ctx.by_ref(),
implementing_for.type_id,
Expand All @@ -190,6 +190,7 @@ impl TyImplSelfOrTrait {
)),
span: block_span,
items: new_items,
supertrait_items,
implementing_for,
}
}
Expand Down Expand Up @@ -240,7 +241,7 @@ impl TyImplSelfOrTrait {
None,
);

let new_items = type_check_trait_implementation(
let (new_items, supertrait_items) = type_check_trait_implementation(
handler,
ctx.by_ref(),
implementing_for.type_id,
Expand All @@ -267,6 +268,7 @@ impl TyImplSelfOrTrait {
)),
span: block_span,
items: new_items,
supertrait_items,
implementing_for,
}
}
Expand Down Expand Up @@ -447,6 +449,7 @@ impl TyImplSelfOrTrait {
trait_decl_ref: None,
span: block_span,
items: new_items,
supertrait_items: vec![],
implementing_for,
};

Expand Down Expand Up @@ -639,7 +642,7 @@ fn type_check_trait_implementation(
trait_decl_span: &Span,
block_span: &Span,
is_contract: bool,
) -> Result<Vec<TyImplItem>, ErrorEmitted> {
) -> Result<(Vec<TyImplItem>, Vec<TyImplItem>), ErrorEmitted> {
let type_engine = ctx.engines.te();
let decl_engine = ctx.engines.de();
let engines = ctx.engines();
Expand Down Expand Up @@ -923,7 +926,7 @@ fn type_check_trait_implementation(
type_mapping.extend(&trait_type_mapping);

interface_item_refs.extend(supertrait_interface_item_refs);
impld_item_refs.extend(supertrait_impld_item_refs);
impld_item_refs.extend(supertrait_impld_item_refs.clone());
let decl_mapping = DeclMapping::from_interface_and_item_and_impld_decl_refs(
interface_item_refs,
BTreeMap::new(),
Expand Down Expand Up @@ -1011,7 +1014,10 @@ fn type_check_trait_implementation(
});
}

Ok(all_items_refs)
Ok((
all_items_refs,
supertrait_impld_item_refs.values().cloned().collect(),
))
})
}

Expand Down
24 changes: 22 additions & 2 deletions sway-core/src/semantic_analysis/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use sway_types::{BaseIdent, Named};

use crate::{
decl_engine::{DeclEngineGet, DeclId},
engine_threading::DebugWithEngines,
engine_threading::{DebugWithEngines, PartialEqWithEngines, PartialEqWithEnginesContext},
language::{
parsed::*,
ty::{self, TyAstNodeContent, TyDecl},
Expand Down Expand Up @@ -350,14 +350,34 @@ impl ty::TyModule {
}
}
(TreeType::Contract, _) => {
// collect all supertrait methods
let contract_supertrait_fns = submodules
.iter()
.flat_map(|x| x.1.module.submodules_recursive())
.flat_map(|x| x.1.module.contract_supertrait_fns(engines))
.chain(
all_nodes
.iter()
.flat_map(|x| x.contract_supertrait_fns(engines)),
)
.collect::<Vec<_>>();

// collect all contract methods
let contract_fns = submodules
let mut contract_fns = submodules
.iter()
.flat_map(|x| x.1.module.submodules_recursive())
.flat_map(|x| x.1.module.contract_fns(engines))
.chain(all_nodes.iter().flat_map(|x| x.contract_fns(engines)))
.collect::<Vec<_>>();

// exclude all contract methods that are supertrait methods
let partialeq_ctx = PartialEqWithEnginesContext::new(engines);
contract_fns.retain(|method| {
contract_supertrait_fns
.iter()
.all(|si| !PartialEqWithEngines::eq(method, si, &partialeq_ctx))
});

let mut fn_generator =
auto_impl::EncodingAutoImplContext::new(&mut ctx).unwrap();
if let Ok(node) = fn_generator.generate_contract_entry(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = "core"
source = "path+from-root-6ECA11F8A7EF18BC"

[[package]]
name = "std"
source = "path+from-root-6ECA11F8A7EF18BC"
dependencies = ["core"]

[[package]]
name = "superabi_supertrait_external_call"
source = "member"
dependencies = ["std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "superabi_supertrait_external_call"
authors = ["Fuel Labs <contact@fuel.sh>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../reduced_std_libs/sway-lib-std-assert" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"configurables": [],
"functions": [
{
"attributes": null,
"inputs": [],
"name": "method",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
},
{
"attributes": null,
"inputs": [],
"name": "method1",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
}
],
"loggedTypes": [],
"messagesTypes": [],
"types": [
{
"components": null,
"type": "u64",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
contract;

trait MySuperTrait {
fn method() -> u64;
}

abi MyAbi : MySuperTrait {
//fn method() -> u64;
}

impl MySuperTrait for Contract {
fn method() -> u64 { 42 }
}

impl MyAbi for Contract {}

#[test]
fn tests() {
let caller = abi(MyAbi, CONTRACT_ID);
assert(caller.method() == 0xBAD)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
category = "run"
expected_result_new_encoding = { action = "revert", value = 123 }
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ impl MyAbi for Contract {
fn tests() {
let caller = abi(MyAbi, CONTRACT_ID);
assert(caller.method1() == 42);
assert(caller.method() == 0xBAD)

// we disallow calling supertrait methods externally
//assert(caller.method() == 0xBAD)
}
2 changes: 1 addition & 1 deletion test/src/ir_generation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Both checks against IR and ASM may be provided in the same Sway test source file
To delimit between checks against IR and those against ASM the source file may be split into sections using delimiting marker text.

* `::check-ir::` marks the beginning of the IR checks.
* `::check-ir-optimized::` marks the begingging of the optimized IR checks.
* `::check-ir-optimized::` marks the beginning of the optimized IR checks.
* `::check-asm::` marks the beginning of the ASM checks.

Optimized IR checker can be configured with `pass: <PASSNAME or o1>`. When
Expand Down
17 changes: 9 additions & 8 deletions test/src/sdk-harness/test_projects/superabi_supertrait/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ async fn method1_test() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn method_test() -> Result<()> {
let instance = get_superabi_supertrait_instance().await;
let contract_methods = instance.methods();
// contract supertrait methods are not callable externally
// #[tokio::test]
// async fn method_test() -> Result<()> {
// let instance = get_superabi_supertrait_instance().await;
// let contract_methods = instance.methods();

let response = contract_methods.method().call().await?;
assert_eq!(0xBAD, response.value);
// let response = contract_methods.method().call().await?;
// assert_eq!(0xBAD, response.value);

Ok(())
}
// Ok(())
// }

0 comments on commit 4c2c384

Please sign in to comment.