Skip to content

Commit

Permalink
refactor(traverse): revert changes to walk.rs (#5652)
Browse files Browse the repository at this point in the history
Revert the changes to `oxc_traverse` made in #5577.

Those changes may well have been good, but...

The soundness of `Traverse` is a finely balanced thing, and the pointer gymnastics are subtle, so I think it's best to be extremely conservative about changes. I last ran Miri on it with the original formulation (`as *mut _` etc) and it passed. Currently it's not possible to usefully run Miri on the transformer because it contains known unsound code (#3483). So until we're able to check soundness with Miri, I think it's best to avoid changes, as it'd be easy to trigger UB unintentionally.
  • Loading branch information
overlookmotel committed Sep 9, 2024
1 parent 2de6ea0 commit 19cdcc5
Show file tree
Hide file tree
Showing 2 changed files with 960 additions and 1,380 deletions.
23 changes: 13 additions & 10 deletions crates/oxc_traverse/scripts/lib/walk.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ export default function generateWalkFunctionsCode(types) {
clippy::missing_panics_doc,
clippy::undocumented_unsafe_blocks,
clippy::semicolon_if_nothing_returned,
clippy::ptr_as_ptr,
clippy::ref_as_ptr,
clippy::borrow_as_ptr,
clippy::cast_ptr_alignment
)]
Expand Down Expand Up @@ -108,19 +111,19 @@ function generateWalkForStruct(type, types) {
if (field.wrappers.length === 2 && field.wrappers[1] === 'Vec') {
if (field.typeNameInner === 'Statement') {
// Special case for `Option<Vec<Statement>>`
walkCode = `walk_statements(traverser, std::ptr::from_mut(field), ctx);`;
walkCode = `walk_statements(traverser, field as *mut _, ctx);`;
} else {
walkCode = `
for item in field.iter_mut() {
${fieldWalkName}(traverser, std::ptr::from_mut(item), ctx);
${fieldWalkName}(traverser, item as *mut _, ctx);
}
`.trim();
}
} else if (field.wrappers.length === 2 && field.wrappers[1] === 'Box') {
walkCode = `${fieldWalkName}(traverser, std::ptr::from_mut(&mut **field), ctx);`;
walkCode = `${fieldWalkName}(traverser, (&mut **field) as *mut _, ctx);`;
} else {
assert(field.wrappers.length === 1, `Cannot handle struct field with type ${field.typeName}`);
walkCode = `${fieldWalkName}(traverser, std::ptr::from_mut(field), ctx);`;
walkCode = `${fieldWalkName}(traverser, field as *mut _, ctx);`;
}

return `
Expand All @@ -139,7 +142,7 @@ function generateWalkForStruct(type, types) {
// Special case for `Vec<Statement>`
walkVecCode = `walk_statements(traverser, ${fieldCode}, ctx);`;
} else {
let walkCode = `${fieldWalkName}(traverser, std::ptr::from_mut(item), ctx);`,
let walkCode = `${fieldWalkName}(traverser, item as *mut _, ctx);`,
iterModifier = '';
if (field.wrappers.length === 2 && field.wrappers[1] === 'Option') {
iterModifier = '.flatten()';
Expand Down Expand Up @@ -167,7 +170,7 @@ function generateWalkForStruct(type, types) {
return `
${scopeCode}
${tagCode || retagCode}
${fieldWalkName}(traverser, std::ptr::from_mut(&mut **(${fieldCode})), ctx);
${fieldWalkName}(traverser, (&mut **(${fieldCode})) as *mut _, ctx);
`;
}

Expand Down Expand Up @@ -198,23 +201,23 @@ function generateWalkForStruct(type, types) {
}

function makeFieldCode(field) {
return `node.cast::<u8>().add(ancestor::${field.offsetVarName}).cast::<${field.typeName}>()`;
return `(node as *mut u8).add(ancestor::${field.offsetVarName}) as *mut ${field.typeName}`;
}

function generateWalkForEnum(type, types) {
const variantCodes = type.variants.map((variant) => {
const variantType = types[variant.innerTypeName];
assert(variantType, `Cannot handle enum variant with type: ${variant.type}`);

let nodeCode = '(node)';
let nodeCode = 'node';
if (variant.wrappers.length === 1 && variant.wrappers[0] === 'Box') {
nodeCode = '(&mut **node)';
} else {
assert(variant.wrappers.length === 0, `Cannot handle enum variant with type: ${variant.type}`);
}

return `${type.name}::${variant.name}(node) => ` +
`walk_${camelToSnake(variant.innerTypeName)}(traverser, std::ptr::from_mut${nodeCode}, ctx),`;
`walk_${camelToSnake(variant.innerTypeName)}(traverser, ${nodeCode} as *mut _, ctx),`;
});

const missingVariants = [];
Expand All @@ -237,7 +240,7 @@ function generateWalkForEnum(type, types) {

variantCodes.push(
`${variantMatches.join(' | ')} => ` +
`walk_${camelToSnake(inheritedTypeName)}(traverser, node.cast(), ctx),`,
`walk_${camelToSnake(inheritedTypeName)}(traverser, node as *mut _, ctx),`,
);
}

Expand Down
Loading

0 comments on commit 19cdcc5

Please sign in to comment.