Skip to content

Attribute for checking of trivial encoding and decoding#7575

Open
xunilrj wants to merge 56 commits intomasterfrom
xunilrj/trivial-checks
Open

Attribute for checking of trivial encoding and decoding#7575
xunilrj wants to merge 56 commits intomasterfrom
xunilrj/trivial-checks

Conversation

@xunilrj
Copy link
Copy Markdown
Contributor

@xunilrj xunilrj commented Mar 17, 2026

Description

Closes #7567.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@cursor
Copy link
Copy Markdown

cursor Bot commented Mar 17, 2026

PR Summary

Medium Risk
Introduces new compiler-enforced attribute handling and new stdlib wrapper types around ABI encoding/decoding, which can affect compilation results and diagnostics for user programs that opt into the checks.

Overview
Adds a new #[require(...)] attribute (wired through sway-ast and the attribute validator) to enforce trivial ABI decode requirements on annotated struct/enum declarations and ABI interface functions.

The compiler now runs a new post-typecheck declaration check that evaluates std::codec::is_decode_trivial::<T>() for referenced types (including nested fields/tuple/array members), and emits a new CompileError::TrivialCheckFailed diagnostic with targeted fix suggestions (e.g. use u64, TrivialBool, TrivialEnum, or replace Vec/String).

Stdlib codec.sw gains TrivialBool, TrivialEnum<T>, and an EnumCodecValues table mechanism (auto-generated for enums during semantic analysis) to allow enums/bools to be treated as trivially decodable with explicit validity checks. Documentation is updated with a new book page on trivial encoding, and an e2e snapshot test is added to assert the new error output for #[require(trivially_decodable = "true")].

Reviewed by Cursor Bugbot for commit a09b65c. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for all 4 issues found in the latest run.

  • ✅ Fixed: Debug statement accidentally committed to production
    • Removed the stray dbg!(&t.decls_to_check) call from compile_to_asm so compilation no longer emits unintended stderr output.
  • ✅ Fixed: Misplaced test: non-trivially-decodable struct in should_pass
    • Updated the attribute_require should-pass program to use trivially decodable field types (u64 and nested u64 struct) so it now matches its expected passing placement.
  • ✅ Fixed: Attribute value required but silently ignored in check
    • The require check now reads arg.value and only enforces trivially_decodable when the value is explicitly true ("true" or true).
  • ✅ Fixed: Require attribute check skipped for predicates and contracts
    • Threaded decls_to_check through predicate and contract compilation paths so #[require(trivially_decodable = "true")] is validated there too.

Create PR

Or push these changes by commenting:

@cursor push 73287ee042
Preview (73287ee042)
diff --git a/sway-core/src/ir_generation.rs b/sway-core/src/ir_generation.rs
--- a/sway-core/src/ir_generation.rs
+++ b/sway-core/src/ir_generation.rs
@@ -399,6 +399,7 @@
             &mut panicking_fn_cache,
             &test_fns,
             &mut compiled_fn_cache,
+            decls_to_check,
         ),
         ty::TyProgramKind::Contract {
             entry_function,
@@ -409,6 +410,7 @@
             abi_entries,
             namespace,
             declarations,
+            decls_to_check,
             &logged_types,
             &messages_types,
             panic_occurrences,

diff --git a/sway-core/src/ir_generation/compile.rs b/sway-core/src/ir_generation/compile.rs
--- a/sway-core/src/ir_generation/compile.rs
+++ b/sway-core/src/ir_generation/compile.rs
@@ -1,9 +1,19 @@
 use crate::{
-    Engines, PanicOccurrences, PanickingCallOccurrences, TypeInfo, decl_engine::{DeclEngineGet, DeclId, DeclRefFunction}, ir_generation::{
-        KeyedTyFunctionDecl, PanickingFunctionCache, convert::convert_resolved_typeid_no_span
-    }, language::{
-        Visibility, ty::{self, StructDecl, TyDecl}
-    }, metadata::MetadataManager, namespace::ResolvedDeclaration, semantic_analysis::namespace, transform::AttributeKind, type_system::TypeId, types::{LogId, MessageId}
+    decl_engine::{DeclEngineGet, DeclId, DeclRefFunction},
+    ir_generation::{
+        convert::convert_resolved_typeid_no_span, KeyedTyFunctionDecl, PanickingFunctionCache,
+    },
+    language::{
+        ty::{self, StructDecl, TyDecl},
+        Visibility,
+    },
+    metadata::MetadataManager,
+    namespace::ResolvedDeclaration,
+    semantic_analysis::namespace,
+    transform::AttributeKind,
+    type_system::TypeId,
+    types::{LogId, MessageId},
+    Engines, PanicOccurrences, PanickingCallOccurrences, TypeInfo,
 };
 
 use super::{
@@ -13,7 +23,7 @@
     CompiledFunctionCache,
 };
 
-use sway_ast::attribute::REQUIRE_ARG_NAME_TRIVIALLY_DECODABLE;
+use sway_ast::{attribute::REQUIRE_ARG_NAME_TRIVIALLY_DECODABLE, Literal};
 use sway_error::{error::CompileError, handler::Handler};
 use sway_ir::{metadata::combine as md_combine, *};
 use sway_types::{Ident, Span, Spanned};
@@ -104,6 +114,7 @@
     panicking_fn_cache: &mut PanickingFunctionCache,
     test_fns: &[(Arc<ty::TyFunctionDecl>, DeclRefFunction)],
     compiled_fn_cache: &mut CompiledFunctionCache,
+    decls_to_check: &[TyDecl],
 ) -> Result<Module, Vec<CompileError>> {
     let module = Module::new(context, Kind::Predicate);
 
@@ -138,7 +149,7 @@
         panicking_fn_cache,
         None,
         compiled_fn_cache,
-        &[],
+        decls_to_check,
     )?;
     compile_tests(
         engines,
@@ -164,6 +175,7 @@
     abi_entries: &[DeclId<ty::TyFunctionDecl>],
     namespace: &namespace::Namespace,
     declarations: &[ty::TyDecl],
+    decls_to_check: &[TyDecl],
     logged_types_map: &HashMap<TypeId, LogId>,
     messages_types_map: &HashMap<TypeId, MessageId>,
     panic_occurrences: &mut PanicOccurrences,
@@ -217,7 +229,7 @@
             panicking_fn_cache,
             None,
             compiled_fn_cache,
-            &[],
+            decls_to_check,
         )?;
     } else {
         // In the case of the encoding v0, we need to compile individual ABI entries
@@ -602,7 +614,18 @@
                 for (_, atts) in atts {
                     for att in atts.iter() {
                         for arg in att.args.iter() {
-                            if arg.name.as_str() == REQUIRE_ARG_NAME_TRIVIALLY_DECODABLE && !is_type_trivially_decodable(*decl_id) {
+                            let requires_trivially_decodable = matches!(
+                                &arg.value,
+                                Some(Literal::String(val)) if val.parsed == "true"
+                            ) || matches!(
+                                &arg.value,
+                                Some(Literal::Bool(val)) if val.kind.into()
+                            );
+
+                            if arg.name.as_str() == REQUIRE_ARG_NAME_TRIVIALLY_DECODABLE
+                                && requires_trivially_decodable
+                                && !is_type_trivially_decodable(*decl_id)
+                            {
                                 let mut infos = vec![];
                                 let mut helps = vec![];
                                 let mut bottom_helps = BTreeSet::new();

diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs
--- a/sway-core/src/lib.rs
+++ b/sway-core/src/lib.rs
@@ -1188,10 +1188,6 @@
         experimental,
     )?;
 
-    if let Ok(t) = ast_res.typed.as_ref() {
-        dbg!(&t.decls_to_check);
-    }
-
     ast_to_asm(handler, engines, &ast_res, build_config, experimental)
 }
 

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/another_file.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/another_file.sw
--- a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/another_file.sw
+++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/another_file.sw
@@ -1,5 +1,5 @@
 library;
 
 pub struct InnerStruct {
-    pub a: bool,
-}
\ No newline at end of file
+    pub a: u64,
+}

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/main.sw
--- a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/main.sw
+++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes/attribute_require/src/main.sw
@@ -5,21 +5,15 @@
 
 #[require(trivially_decodable = "true")]
 struct MyStruct {
-    a: bool,
+    a: u64,
     b: InnerStruct,
-    c: SomeEnum,
-    d: Vec<u64>,
+    c: u64,
+    d: u64,
 }
 
-enum SomeEnum {
-    A: ()
-}
-
 fn main(s: MyStruct) {
     __log(s.a);
     __log(s.b.a);
     __log(s.c);
     __log(s.d);
-    let a = SomeEnum::A;
-    __log(a);
 }

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread sway-core/src/lib.rs Outdated
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/ir_generation.rs Outdated
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Mar 17, 2026

Merging this PR will not alter performance

✅ 25 untouched benchmarks


Comparing xunilrj/trivial-checks (a09b65c) with master (e12e611)

Open in CodSpeed

Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/language/ty/declaration/declaration.rs Outdated
Comment thread sway-core/src/ir_generation/function.rs Outdated
Comment thread sway-lib-std/src/codec.sw
Comment thread sway-ast/src/attribute.rs
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/language/ty/declaration/declaration.rs Outdated
Comment thread sway-core/src/ir_generation/const_eval.rs Outdated
@ironcev ironcev changed the title Attribute for checking trivially of encoding and decoding Attribute for checking of trivially encoding and decoding Mar 26, 2026
@ironcev ironcev changed the title Attribute for checking of trivially encoding and decoding Attribute for checking of trivial encoding and decoding Mar 26, 2026
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

There are 11 total unresolved issues (including 8 from previous reviews).

Autofix Details

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Debug eprintln! left in production code path
    • I removed the unconditional eprintln! block from type_check_finalize for EnumVariantsValues so no debug output is emitted during normal compilation.
  • ✅ Fixed: EnumVariantsValues intrinsic hardcodes true for all variants
    • I replaced the hardcoded true with per-variant triviality computation by comparing each variant type’s encoding and runtime memory representations.
  • ✅ Fixed: Type check hardcodes literal 3, panics otherwise
    • I removed the literal-3 match and todo!() panic path, corrected the expected argument count to 1, and return a proper intrinsic type error for non-enum type arguments.

Create PR

Or push these changes by commenting:

@cursor push 64daa5eea7
Preview (64daa5eea7)
diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs
--- a/sway-core/src/ir_generation/function.rs
+++ b/sway-core/src/ir_generation/function.rs
@@ -2477,7 +2477,7 @@
                         let elems = decl
                             .variants
                             .iter()
-                            .map(|_variant| {
+                            .map(|variant| {
                                 // let codec = namespace.module_from_absolute_path(&[
                                 //     BaseIdent::new_no_span("std".to_string()),
                                 //     BaseIdent::new_no_span("codec".to_string()),
@@ -2526,9 +2526,27 @@
 
                                 // let value = r.get_content(context).as_bool().unwrap();
                                 // dbg!(value);
-                                ConstantContent::new_bool(context, true)
+                                let variant_type_info =
+                                    engines.te().get(variant.type_argument.type_id);
+                                let encoding_representation =
+                                    get_encoding_representation(engines, &variant_type_info);
+                                let runtime_type = convert_resolved_type_id(
+                                    self.engines,
+                                    context,
+                                    md_mgr,
+                                    self.module,
+                                    Some(self),
+                                    variant.type_argument.type_id,
+                                    &variant.type_argument.span,
+                                )?;
+                                let runtime_representation =
+                                    Some(get_runtime_representation(context, runtime_type));
+
+                                let is_decode_trivial =
+                                    encoding_representation == runtime_representation;
+                                Ok(ConstantContent::new_bool(context, is_decode_trivial))
                             })
-                            .collect::<Vec<_>>();
+                            .collect::<Result<Vec<_>, CompileError>>()?;
 
                         let elems_len = ConstantContent::new_uint(context, 64, elems.len() as u64);
                         let elems_len = Constant::unique(context, elems_len);

diff --git a/sway-core/src/language/ty/expression/expression_variant.rs b/sway-core/src/language/ty/expression/expression_variant.rs
--- a/sway-core/src/language/ty/expression/expression_variant.rs
+++ b/sway-core/src/language/ty/expression/expression_variant.rs
@@ -1330,13 +1330,6 @@
                     for expr in kind.arguments.iter_mut() {
                         expr.type_check_finalize(handler, ctx)?;
                     }
-
-                    if let sway_ast::Intrinsic::EnumVariantsValues = kind.kind {
-                        eprintln!(
-                            "type_check_finalize: {:?}",
-                            ctx.engines.help_out(&kind.type_arguments)
-                        );
-                    }
                 }
                 TyExpressionVariant::AbiName(_) => {
                     todo!("")

diff --git a/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs b/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs
--- a/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs
+++ b/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs
@@ -187,7 +187,7 @@
     if arguments.len() != 1 {
         return Err(handler.emit_err(CompileError::IntrinsicIncorrectNumArgs {
             name: kind.to_string(),
-            expected: 0,
+            expected: 1,
             span,
         }));
     }
@@ -198,11 +198,6 @@
         ty::TyExpression::type_check(handler, ctx, &arguments[0])?
     };
 
-    let _value_id = match first_argument_typed_expr.expression {
-        ty::TyExpressionVariant::Literal(Literal::U64(3)) => 3,
-        _ => todo!(),
-    };
-
     let arguments = vec![first_argument_typed_expr];
 
     if type_arguments.len() != 1 {
@@ -233,12 +228,13 @@
     let return_type = ctx.engines.te().insert_slice(ctx.engines, elem_type);
 
     match &*ctx.engines.te().get(arg) {
-        TypeInfo::UnknownGeneric { .. } => {}
-        TypeInfo::Enum(_) => {
-            todo!();
-        }
+        TypeInfo::UnknownGeneric { .. } | TypeInfo::Enum(_) => {}
         _ => {
-            todo!()
+            return Err(handler.emit_err(CompileError::IntrinsicUnsupportedArgType {
+                name: kind.to_string(),
+                span: span.clone(),
+                hint: "Type argument must be an enum type.".to_string(),
+            }));
         }
     };

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread sway-core/src/language/ty/expression/expression_variant.rs Outdated
Comment thread sway-core/src/ir_generation/function.rs Outdated
Comment thread sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs Outdated
Comment thread sway-lib-std/src/codec.sw Outdated
Comment thread sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs Outdated
Comment thread sway-lib-std/src/codec.sw
Comment thread sway-core/src/type_system/monomorphization.rs Outdated
@xunilrj xunilrj force-pushed the xunilrj/trivial-checks branch from 65ab635 to 8b042ca Compare April 1, 2026 12:53
@xunilrj xunilrj temporarily deployed to fuel-sway-bot April 1, 2026 12:53 — with GitHub Actions Inactive
@xunilrj xunilrj temporarily deployed to fuel-sway-bot April 1, 2026 13:08 — with GitHub Actions Inactive
Comment thread sway-core/src/ir_generation/compile.rs Outdated
@xunilrj xunilrj force-pushed the xunilrj/trivial-checks branch from b4ce718 to bd80a03 Compare April 20, 2026 00:52
@xunilrj xunilrj marked this pull request as ready for review April 20, 2026 01:53
@xunilrj xunilrj requested review from a team as code owners April 20, 2026 01:53
@xunilrj xunilrj self-assigned this Apr 20, 2026
@cursor cursor Bot temporarily deployed to fuel-sway-bot April 20, 2026 10:43 Inactive
Comment thread sway-core/src/ir_generation/compile.rs Outdated
Comment thread sway-core/src/lib.rs Outdated
@FuelLabs FuelLabs deleted a comment from cursor Bot Apr 20, 2026
@xunilrj xunilrj force-pushed the xunilrj/trivial-checks branch from 8cc3fab to 03bb4ff Compare April 20, 2026 15:18
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 20, 2026

I addressed both new BugBot findings in the latest code:

  • HashMap unwrap() panic (HIGH): Replaced *table.get(&field_fullname).unwrap() with a proper match that returns a CompileError::Internal for missing keys, consistent with the pattern the PR author already applied to the top-level type lookup earlier in the same function.

  • todo!() panic (MEDIUM): Replaced the todo!() in push_help_if_non_trivially_decodable_type with an empty match arm (_ => {}), so unexpected type variants in the type_decl_span branch are silently skipped instead of crashing the compiler. The other todo!() in lib.rs was already fixed by the PR author.

Open in Web Open in Cursor 

@cursor cursor Bot temporarily deployed to fuel-sway-bot April 20, 2026 15:25 Inactive
Comment thread sway-core/src/lib.rs Outdated
@xunilrj xunilrj force-pushed the xunilrj/trivial-checks branch from aa76235 to 1a7152d Compare April 21, 2026 11:51
@xunilrj
Copy link
Copy Markdown
Contributor Author

xunilrj commented Apr 21, 2026

@cursor BugBot run

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 21, 2026

The PR author's force push (1a7152d47 remove redundant manager) fixed the shadowed MetadataManager bug but dropped my earlier fixes for the two remaining issues. I re-applied them:

  • table.get(&field_fullname).unwrap() panic (HIGH): Replaced with a proper match returning CompileError::Internal for missing keys, consistent with the existing pattern for the top-level type lookup at line 714.
  • todo!() in push_help_if_non_trivially_decodable_type (MEDIUM): Replaced with _ => {} so unexpected type variants are silently skipped instead of crashing the compiler.
Open in Web Open in Cursor 

- Replace table.get(&field_fullname).unwrap() with proper match/error
  handling, consistent with the pattern already used for the top-level
  type lookup. Prevents compiler panic when a field type's
  is_decode_trivial expression cannot be evaluated to a constant bool.

- Replace todo!() in push_help_if_non_trivially_decodable_type with an
  empty match arm for unhandled type_decl_span cases, since no specific
  help message is needed for unexpected types and the compiler should not
  crash.

Co-authored-by: Daniel Frederico Lins Leite <xunilrj@users.noreply.github.com>
@cursor cursor Bot deployed to fuel-sway-bot April 21, 2026 17:32 Active
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a09b65c. Configure here.

Comment thread sway-lib-std/src/codec.sw
1 => true,
_ => __revert(INVALID_BOOL_REVERT),
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TrivialBool methods missing pub visibility modifier

High Severity

TrivialBool's is_valid and unwrap methods are missing the pub keyword, making them inaccessible to external users. The documentation explicitly shows users calling .unwrap() on TrivialBool values. The equivalent methods on TrivialEnum are correctly marked pub. Without pub, the entire purpose of TrivialBool as a user-facing wrapper type is defeated.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a09b65c. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler attribute to guarantee that types are trivially encodable/decodable

2 participants