Skip to content

Commit

Permalink
Improve warnings on unnecessary storage annotations (FuelLabs#3414)
Browse files Browse the repository at this point in the history
Fixes FuelLabs#1183: just trying to improve the error message.

This PR also provides a fix for the following issue: the purity analyzer
did not report the case where *both* read and write storage annotations
were unused, i.e. in this case there were no warnings:

```
contract;

abi MyContract {
    #[storage(read, write)]
    fn foo() -> u64;
}

impl MyContract for Contract {
    #[storage(read, write)]
    fn foo() -> u64 {
        0
    }
}
```
  • Loading branch information
anton-trunov authored Nov 23, 2022
1 parent a7fa7d9 commit f8d22d2
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 4 deletions.
8 changes: 6 additions & 2 deletions sway-core/src/ir_generation/purity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,15 @@ pub(crate) fn check_function_purity(
// Or we have unneeded attributes.
(Some(StorageOperation::ReadsWrites), false, true) => warn(span, Reads),
(Some(StorageOperation::ReadsWrites), true, false) => warn(span, Writes),
(Some(StorageOperation::ReadsWrites), false, false) => warn(span, ReadsWrites),
(Some(StorageOperation::Reads), false, false) => warn(span, Reads),
(Some(StorageOperation::Writes), false, false) => warn(span, Writes),

// (Pure, false, false) is OK, as is (ReadsWrites, true, true).
_ => (),
// Attributes and effects are in total agreement
(None, false, false)
| (Some(StorageOperation::Reads), true, false)
| (Some(StorageOperation::Writes), false, true)
| (Some(StorageOperation::ReadsWrites), true, true) => (),
};

(reads, writes)
Expand Down
4 changes: 2 additions & 2 deletions sway-error/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ impl fmt::Display for Warning {
),
DeadStorageDeclarationForFunction { unneeded_attrib } => write!(
f,
"The '{unneeded_attrib}' storage declaration for this function is never accessed \
and can be removed."
"This function's storage attributes declaration does not match its \
actual storage access pattern: '{unneeded_attrib}' attribute(s) can be removed."
),
MatchExpressionUnreachableArm => write!(f, "This match arm is unreachable."),
UnrecognizedAttribute {attrib_name} => write!(f, "Unknown attribute: \"{attrib_name}\"."),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-B415B1E0A318C0B0'

[[package]]
name = 'std'
source = 'path+from-root-B415B1E0A318C0B0'
dependencies = ['core']

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

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
contract;

abi MyContract {
#[storage(read)]
fn foo() -> u64;
}

impl MyContract for Contract {
#[storage(read)]
fn foo() -> u64 {
0
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()This function's storage attributes declaration does not match this function's actual storage access pattern: 'read' attribute(s) can be removed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-F9F255B625A3400C'

[[package]]
name = 'std'
source = 'path+from-root-F9F255B625A3400C'
dependencies = ['core']

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

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
contract;

abi MyContract {
#[storage(read, write)]
fn foo() -> u64;
}

impl MyContract for Contract {
#[storage(read, write)]
fn foo() -> u64 {
0
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()This function's storage attributes declaration does not match this function's actual storage access pattern: 'read, write' attribute(s) can be removed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-4A37DED755D546B6'

[[package]]
name = 'std'
source = 'path+from-root-4A37DED755D546B6'
dependencies = ['core']

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

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
contract;

abi MyContract {
#[storage(write)] // Or any other storage annotation
fn foo() -> u64;
}

impl MyContract for Contract {
#[storage(write)] // Or any other storage annotation
fn foo() -> u64 {
0
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "compile"

# check: $()This function's storage attributes declaration does not match this function's actual storage access pattern: 'write' attribute(s) can be removed.

0 comments on commit f8d22d2

Please sign in to comment.