oxc-project / oxc-project/backlog

Use `ptr::addr_of` for offset calculations in `Traverse`'s `walk_*` functions

Open
#17 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
No language data
Stars
7
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Traverse's walk_* functions are currently full of code like this:

walk_expression(
    traverser,
    (node as *mut u8).add(ancestor::OFFSET_BINARY_EXPRESSION_LEFT) as *mut Expression,
    ctx,
);

Ancestor methods contain similar code:

pub fn left(&self) -> &Expression<'a> {
    unsafe {
        &*((self.0 as *const u8).add(OFFSET_BINARY_EXPRESSION_LEFT) as *const Expression<'a>)
    }
}

Could probably use std::ptr::addr_of! and std::ptr::addr_of_mut! for these instead:

use std::ptr::addr_of_mut;

walk_expression(
    traverser,
    addr_of_mut!((*node).left),
    ctx,
);
use std::ptr::addr_of;

pub fn left(&self) -> &Expression<'a> {
    let node = self.0;
    unsafe { &*addr_of!((*node).left) }
}

Much easier to read, and can get rid of all the OFFSET_* constants.

I hadn't clocked that you can use addr_of! starting with another pointer, and deref that pointer without actually deref-ing it.

https://doc.rust-lang.org/std/ptr/macro.addr_of.html

However, I'd like to implement more transformers using upwards seeking first and run it through Miri before making any changes. Also addr_of! may affect performance (either positively or negatively).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Traverse's walk_* functions and the Ancestor methods that use OFFSET_* constants. First understand the proposed addr_of! and addr_of_mut! patterns, then implement more transformers using upwards seeking and run the result through Miri. Done means the offset calculations are replaced where appropriate, the OFFSET_* constants can be removed, and performance effects have been checked.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.