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

Debug trait and its auto implementation #7015

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Mar 11, 2025

Description

This PR implements the __dbg(...) intrinsic, which is very similar to Rust dbg!(...) macro.

Up until now, it has being the norm to use __log to debug values. Given that this is NOT the first use case for log, we have always found some issues with it: log does not work on predicates, log does not show "private" fields like Vec::capacity and others.

To solve these problems __dbg is being introduced:
1 - it will work on all program types, including predicates;
2 - it also prints the file name, line and column;
3 - All types will have an automatic implementation of Debug if possible, which can still be customized.
4 - Even raw_ptr and other non "loggable" types, have Debug impls.
5 - __dbg will be completely stripped in the release build by default. It can be turned on again if needed.

So this:

// Aggregates
let _ = __dbg((1u64, 2u64));
let _ = __dbg([1u64, 2u64]);

// Structs and Enums
let _ = __dbg(S { });
let _ = __dbg(E::None);
let _ = __dbg(E::Some(S { }));

will generate this:

[src/main.sw:19:13] = (1, 2)
[src/main.sw:20:13] = [1, 2]
[src/main.sw:23:13] = S { }
[src/main.sw:24:13] = None
[src/main.sw:25:13] = E(S { })

How does this work?

__dbg(value) intrinsic is desugared into { let f = Formatter{}; f.print_str(...); let value = value; value.fmt(f); value }.

Formatter is similar to Rust's one. The difference is that we still do not support string formatting, so the Formatter has a lot of print_* functions.

And each print function calls a "syscall". This syscall uses ecal under the hood and it follows unix write syscall schema.

// ssize_t write(int fd, const void buf[.count], size_t count);
fn syscall_write(fd: u64, buf: raw_ptr, count: u64) {
    asm(id: 1000, fd: fd, buf: buf, count: count) {
        ecal id fd buf count;
    }
}

For that to work, the VM interpreter must have its EcalState setup and interpret syscall number 1000 as write. This PR does this for forc test and our e2e test suite.

Each test in forc test will capture these calls and only print to the terminal when requested with the --log flag.

Garbage Collector and auto generated

Before, we were associating all auto-generated code with a pseudo file called "<autogenerated>.sw" that was never garbage collected.

This generated a problem inside the LSP when the auto_impl.rs ran a second time because of a collision in the "shareable type" map. When we try to solve this collision, choosing to keep the old value or to insert the new, the type inside the map points to already collected types and the compiler panics. This is a known problem.

The workaround for this is to break the auto-generated code into multiple files. Now they are named "main.autogenerated.sw", for example. We create one pseudo-file for each real file that needs one.

When we garbage collect one file, main.sw, for example, we also collect its associated auto-generated file.

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.

Copy link

codspeed-hq bot commented Mar 11, 2025

CodSpeed Performance Report

Merging #7015 will degrade performances by 34.23%

Comparing xunilrj/dbg-and-debug (f50ebfd) with master (4d18d52)

Summary

❌ 4 regressions
✅ 18 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
compile 4.9 s 6.4 s -22.9%
did_change_with_caching 520.4 ms 606.6 ms -14.21%
traverse 231.1 ms 351.3 ms -34.23%
hover 3.6 ms 4.7 ms -23.36%

@xunilrj xunilrj force-pushed the xunilrj/dbg-and-debug branch from 9122060 to efa2227 Compare March 13, 2025 18:08
@xunilrj xunilrj force-pushed the xunilrj/dbg-and-debug branch from 76a644c to d49c3c8 Compare March 14, 2025 19:21
@xunilrj xunilrj self-assigned this Mar 19, 2025
@xunilrj xunilrj force-pushed the xunilrj/dbg-and-debug branch from fa71540 to f50ebfd Compare April 9, 2025 23:29
@xunilrj xunilrj temporarily deployed to fuel-sway-bot April 9, 2025 23:29 — with GitHub Actions Inactive
@xunilrj xunilrj enabled auto-merge (squash) April 9, 2025 23:29
Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

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

Sorry for un-approving here but just wanted to check in with the degradation of the performance reported by codspeed. A 34% degrade in performance is pretty big. Is this unavoidable?

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.

7 participants