Skip to content

Commit

Permalink
[move] Add error constants to source language (duplicate of #16659) (…
Browse files Browse the repository at this point in the history
…#16842)

## Description 

NB: This is a duplicate of the already-approved #16659 since those
changes were accidentally landed into the lower commit.

This PR adds support for "clever assertions" to the Move source language
for Move 2024 (NB: no changes to the bytecode format!).

Clever assertions allow you to use non-u64 consts as abort codes, as
long as they are annotated with the new `#[error]` attribute:

```move
module std::AModule {
    #[error]
    const EIsThree: vector<u8> = b"EIsThree";

    public fun double_except_three(x: u64): u64 {
        assert!(x != 3, EIsThree);
        x * x
    }
}
```

Note that outside of abort context, constants annotated `#[error]` will
still be typechecked normally, e.g., the following is a valid program in
this setting
```move
#[error]
const ENotFound: bool = true;

fun f() {
    // usage in arg1 is as a bool and in arg2 as a u64
    assert!(ENotFound, ENotFound);
}
```

## Assertions with No Abort Codes

This also adds support for assertions with no abort codes:
```move
fun cool(x: u64) {
  assert!(x == 42);
}
```

This will be compiled to a tagged `u64` abort code, constant identifier,
and constant value fields set to `0xFFFF`, and `0xFFFF` (see below).

## Compilation scheme

Non-u64 constants used in an abort context, and that have been annotated
with `#[error(code = <n>)]` will turn into a tagged abort code (a `u64`)
with the following layout
```
    // |<tagbit>|<reserved>|<line number>|<identifier index>|<constant index>|
    //   1-bit    15-bits      16-bits        16-bits          16-bits
``` 

i.e., we tag these special abort codes, and then bit-pack source line
number, then the index in the constant pool that holds the string name
of the constant that was used, and then the index in the constant pool
of the actual constant value. Note, that in some cases the index of the
constant name into the constant pool and the index into the constant
pool for the constants value may be the same, e.g., if you have
something like
```move
#[error]
const MyError: vector<u8> = b"MyError";
```

In the case of "naked assertions" with no abort code, we simply set the
tag bit, and line number parts of the bitfield, and set the error code,
identifier, and constant value indices to their max value (`0xFFFF`). If
the line number is greater than `u16::MAX` then the value is clamped at
that value.

## Test Plan 

Added additional tests for these behaviors. 

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [X] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

## Description 

Describe the changes or additions included in this PR.

## Test Plan 

How did you test the new or updated feature?

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
tzakian authored Mar 25, 2024
1 parent f1aee58 commit defc1b1
Show file tree
Hide file tree
Showing 73 changed files with 1,266 additions and 105 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions crates/move-cli/tests/sandbox_tests/clever_errors/Move.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "PackageBasics"
edition = "development"

[addresses]
std = "0x1"

[dependencies]
MoveStdlib = { local = "../../../../move-stdlib" }
79 changes: 79 additions & 0 deletions crates/move-cli/tests/sandbox_tests/clever_errors/args.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
Command `test -t 1`:
INCLUDING DEPENDENCY MoveStdlib
BUILDING PackageBasics
Running Move unit tests
[ PASS ] 0x1::AModule::double_three
[ PASS ] 0x1::AModule::double_two
[ PASS ] 0x1::AModuleTests::double_one_one
[ PASS ] 0x1::AModuleTests::double_three
[ PASS ] 0x1::AModuleTests::double_three_location_based_valid
[ PASS ] 0x1::AModuleTests::double_zero_zero
[ FAIL ] 0x1::AModuleTestsShouldAllFail::double_three_const_based_different_module_fail
[ FAIL ] 0x1::AModuleTestsShouldAllFail::double_three_location_based_invalid
[ FAIL ] 0x1::AModuleTestsShouldAllFail::double_three_should_fail
[ FAIL ] 0x1::AModuleTestsShouldAllFail::double_three_should_fail_named_const

Test failures:

Failures in 0x1::AModuleTestsShouldAllFail:

┌── double_three_const_based_different_module_fail ──────
│ error[E11001]: test failure
│ ┌─ ./sources/AModule.move:10:9
│ │
│ 9 │ public fun double_except_three(x: u64): u64 {
│ │ ------------------- In this function in 0x1::AModule
│ 10 │ assert!(x != 3, EIsThree);
│ │ ^^^^^^^^^^^^^^^^^^^^^^^^^ Test did not error as expected. Expected test to abort with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::BModule but instead it aborted with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModule rooted here
└──────────────────


┌── double_three_location_based_invalid ──────
│ error[E11001]: test failure
│ ┌─ ./sources/AModule.move:10:9
│ │
│ 9 │ public fun double_except_three(x: u64): u64 {
│ │ ------------------- In this function in 0x1::AModule
│ 10 │ assert!(x != 3, EIsThree);
│ │ ^^^^^^^^^^^^^^^^^^^^^^^^^ Test did not error as expected. Expected test to abort with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModuleTestsShouldAllFail but instead it aborted with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModule rooted here
└──────────────────


┌── double_three_should_fail ──────
│ error[E11001]: test failure
│ ┌─ ./sources/AModule.move:10:9
│ │
│ 9 │ public fun double_except_three(x: u64): u64 {
│ │ ------------------- In this function in 0x1::AModule
│ 10 │ assert!(x != 3, EIsThree);
│ │ ^^^^^^^^^^^^^^^^^^^^^^^^^ Test did not abort with expected code. Expected test to abort with code 0, but instead it aborted with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModule rooted here
└──────────────────


┌── double_three_should_fail_named_const ──────
│ error[E11001]: test failure
│ ┌─ ./sources/AModule.move:10:9
│ │
│ 9 │ public fun double_except_three(x: u64): u64 {
│ │ ------------------- In this function in 0x1::AModule
│ 10 │ assert!(x != 3, EIsThree);
│ │ ^^^^^^^^^^^^^^^^^^^^^^^^^ Test did not error as expected. Expected test to abort with error constant 'ENotFound' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModule but instead it aborted with error constant 'EIsThree' originating in the module 0000000000000000000000000000000000000000000000000000000000000001::AModule rooted here
└──────────────────

Test result: FAILED. Total tests: 10; passed: 6; failed: 4
warning[W10007]: issue with attribute value
┌─ tests/AModuleTestsShouldAllFail.move:8:24
8 │ #[expected_failure(abort_code = 0)]
│ ^^^^^^^^^^ - Replace value with constant from expected module or add `location=...` attribute.
│ │
│ WARNING: passes for an abort from any module.

1 change: 1 addition & 0 deletions crates/move-cli/tests/sandbox_tests/clever_errors/args.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test -t 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
module std::AModule {

#[error]
const EIsThree: vector<u8> = b"EIsThree";

#[error]
const ENotFound: vector<u8> = b"Element not found";

public fun double_except_three(x: u64): u64 {
assert!(x != 3, EIsThree);
x * x
}

public fun double_except_four(x: u64): u64 {
assert!(x != 4);
x * x
}

public fun dont_find() {
abort ENotFound
}

#[test]
fun double_two() {
assert!(double_except_three(4) == 16, 0)
}

#[test]
#[expected_failure(abort_code = EIsThree)]
fun double_three() {
double_except_three(3);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module std::BModule {
#[allow(unused_const)]
#[error]
const EIsThree: vector<u8> = b"EIsThree";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#[test_only]
module std::AModuleTests {
use std::AModule;

#[test]
fun double_zero_zero() {
assert!(AModule::double_except_three(0) == 0, 0)
}

#[test]
#[expected_failure(abort_code = std::AModule::EIsThree)]
fun double_three() {
AModule::double_except_three(3);
}

#[test]
#[expected_failure(abort_code = std::AModule::EIsThree, location = std::AModule)]
fun double_three_location_based_valid() {
AModule::double_except_three(3);
}

#[test]
fun double_one_one() {
assert!(AModule::double_except_three(1) == 1, 0)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// All tests in this module should fail to pass (marked
// as test failures).
#[test_only]
module std::AModuleTestsShouldAllFail {
use std::AModule;

#[test]
#[expected_failure(abort_code = 0)]
fun double_three_should_fail() {
AModule::double_except_three(3);
}

#[test]
#[expected_failure(abort_code = std::AModule::ENotFound)]
fun double_three_should_fail_named_const() {
AModule::double_except_three(3);
}

#[test]
#[expected_failure(abort_code = std::AModule::EIsThree, location = Self)]
fun double_three_location_based_invalid() {
AModule::double_except_three(3);
}

#[test]
#[expected_failure(abort_code = std::BModule::EIsThree)]
fun double_three_const_based_different_module_fail() {
AModule::double_except_three(3);
}
}
3 changes: 3 additions & 0 deletions crates/move-command-line-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ dirs-next.workspace = true
vfs.workspace = true

move-core-types.workspace = true

[dev-dependencies]
proptest.workspace = true
173 changes: 173 additions & 0 deletions crates/move-command-line-common/src/error_bitset.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// Copyright (c) The Diem Core Contributors
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

const BITSET_VALUE_UNAVAILABLE: u16 = u16::MAX;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct ErrorBitset {
// |<tagbit>|<reserved>|<line number>|<identifier index>|<constant index>|
// 1-bit 15-bits 16-bits 16-bits 16-bits
pub bits: u64,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ErrorBitsetField {
Tag,
#[allow(dead_code)]
Reserved,
LineNumber,
Identifier,
Constant,
}

pub struct ErrorBitsetBuilder {
line_number: u16,
identifier_index: Option<u16>,
constant_index: Option<u16>,
}

impl ErrorBitsetField {
const TAG_MASK: u64 = 0x8000_0000_0000_0000;
const RESERVED_AREA_MASK: u64 = 0x7fff_0000_0000_0000;
const LINE_NUMBER_MASK: u64 = 0x0000_ffff_0000_0000;
const IDENTIFIER_INDEX_MASK: u64 = 0x0000_0000_ffff_0000;
const CONSTANT_INDEX_MASK: u64 = 0x0000_0000_0000_ffff;

const TAG_SHIFT: u64 = Self::RESERVED_AREA_SHIFT + 15;
const RESERVED_AREA_SHIFT: u64 = Self::LINE_NUMBER_SHIFT + 16;
const LINE_NUMBER_SHIFT: u64 = Self::IDENTIFIER_INDEX_SHIFT + 16;
const IDENTIFIER_INDEX_SHIFT: u64 = Self::CONSTANT_INDEX_SHIFT + 16;
const CONSTANT_INDEX_SHIFT: u64 = 0;

const fn mask(&self) -> u64 {
match self {
Self::Tag => Self::TAG_MASK,
Self::Reserved => Self::RESERVED_AREA_MASK,
Self::LineNumber => Self::LINE_NUMBER_MASK,
Self::Identifier => Self::IDENTIFIER_INDEX_MASK,
Self::Constant => Self::CONSTANT_INDEX_MASK,
}
}

const fn shift(&self) -> u64 {
match self {
Self::Tag => Self::TAG_SHIFT,
Self::Reserved => Self::RESERVED_AREA_SHIFT,
Self::LineNumber => Self::LINE_NUMBER_SHIFT,
Self::Identifier => Self::IDENTIFIER_INDEX_SHIFT,
Self::Constant => Self::CONSTANT_INDEX_SHIFT,
}
}

const fn get_bits(&self, bits: u64) -> u16 {
((bits & self.mask()) >> self.shift()) as u16
}
}

impl ErrorBitsetBuilder {
pub fn new(line_number: u16) -> Self {
Self {
line_number,
identifier_index: None,
constant_index: None,
}
}

pub fn with_identifier_index(&mut self, identifier_index: u16) {
self.identifier_index = Some(identifier_index);
}

pub fn with_constant_index(&mut self, constant_index: u16) {
self.constant_index = Some(constant_index);
}

pub fn build(self) -> ErrorBitset {
ErrorBitset::new(
self.line_number,
self.identifier_index.unwrap_or(BITSET_VALUE_UNAVAILABLE),
self.constant_index.unwrap_or(BITSET_VALUE_UNAVAILABLE),
)
}
}

impl ErrorBitset {
pub(crate) const fn new(line_number: u16, identifier_index: u16, constant_index: u16) -> Self {
use ErrorBitsetField as E;
let mut bits = 0u64;
bits |= 1u64 << E::Tag.shift();
bits |= (line_number as u64) << E::LineNumber.shift();
bits |= (identifier_index as u64) << E::Identifier.shift();
bits |= (constant_index as u64) << E::Constant.shift();
Self { bits }
}

pub const fn from_u64(bits: u64) -> Option<Self> {
if Self::is_tagged_error(bits) {
Some(Self { bits })
} else {
None
}
}

pub const fn is_tagged_error(bits: u64) -> bool {
ErrorBitsetField::Tag.get_bits(bits) == 1
}

const fn sentinel(v: u16) -> Option<u16> {
if v == BITSET_VALUE_UNAVAILABLE {
None
} else {
Some(v)
}
}

pub const fn line_number(&self) -> Option<u16> {
Self::sentinel(ErrorBitsetField::LineNumber.get_bits(self.bits))
}

pub const fn identifier_index(&self) -> Option<u16> {
Self::sentinel(ErrorBitsetField::Identifier.get_bits(self.bits))
}

pub const fn constant_index(&self) -> Option<u16> {
Self::sentinel(ErrorBitsetField::Constant.get_bits(self.bits))
}
}

#[cfg(test)]
mod tests {
use super::{ErrorBitset, ErrorBitsetBuilder};
use proptest::prelude::*;
use proptest::proptest;

proptest! {
#[test]
fn test_error_bitset(line_number in 0..u16::MAX, identifier_index in 0..u16::MAX, constant_index in 0..u16::MAX) {
let error_bitset = ErrorBitset::new(line_number, identifier_index, constant_index);
prop_assert_eq!(error_bitset.line_number(), Some(line_number));
prop_assert_eq!(error_bitset.identifier_index(), Some(identifier_index));
prop_assert_eq!(error_bitset.constant_index(), Some(constant_index));
}
}

proptest! {
#[test]
fn test_error_bitset_builder(line_number in 0..u16::MAX, identifier_index in 0..u16::MAX, constant_index in 0..u16::MAX) {
let error_bitset = ErrorBitset::new(line_number, identifier_index, constant_index);
let mut error_bitset_builder = ErrorBitsetBuilder::new(line_number);
error_bitset_builder.with_identifier_index(identifier_index);
error_bitset_builder.with_constant_index(constant_index);
let error_bitset_built = error_bitset_builder.build();
prop_assert_eq!(error_bitset.line_number(), Some(line_number));
prop_assert_eq!(error_bitset.identifier_index(), Some(identifier_index));
prop_assert_eq!(error_bitset.constant_index(), Some(constant_index));

prop_assert_eq!(error_bitset_built.line_number(), Some(line_number));
prop_assert_eq!(error_bitset_built.identifier_index(), Some(identifier_index));
prop_assert_eq!(error_bitset_built.constant_index(), Some(constant_index));

prop_assert_eq!(error_bitset.bits, error_bitset_built.bits);
}
}
}
1 change: 1 addition & 0 deletions crates/move-command-line-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
pub mod address;
pub mod character_sets;
pub mod env;
pub mod error_bitset;
pub mod files;
pub mod interactive;
pub mod parser;
Expand Down
Loading

0 comments on commit defc1b1

Please sign in to comment.