refactor: canonicalize moved paths that refer to item left behind - #1928
Conversation
47f0497 to
2cf3f8e
Compare
2cf3f8e to
daf6325
Compare
daf6325 to
81140ff
Compare
81140ff to
1d8d52c
Compare
d45e97c to
87a82b0
Compare
87a82b0 to
ff8ac98
Compare
ff8ac98 to
6438594
Compare
6438594 to
6012d94
Compare
6012d94 to
d20e8ed
Compare
d20e8ed to
a4044e4
Compare
a4044e4 to
252f1c9
Compare
252f1c9 to
5750c0c
Compare
5750c0c to
4090222
Compare
4090222 to
0a47a51
Compare
2d41b43 to
9885cdf
Compare
9885cdf to
314a319
Compare
4055c03 to
9609486
Compare
cec27ff to
b67a574
Compare
9cbbba7 to
486931c
Compare
486931c to
4cddd79
Compare
thedataking
left a comment
There was a problem hiding this comment.
Review of stack 1953: qualifying a moved path drops its generic arguments.
| cx.hir_map().local_def_id_to_node_id(mod_hir_id) == dest_info.id | ||
| }); | ||
| if !import_covers && !defined_in_dest { | ||
| return cx.def_qpath(def_id); |
There was a problem hiding this comment.
[P2] Preserve generic arguments when qualifying a path
def_qpath(def_id) reconstructs the definition's name without the original path's generic arguments. This turns a field type Value<i32> into crate::defs::Value, causing E0107. The destination's glob import makes this a regression: the output before this PR already compiled because Value<i32> remained resolvable there. Please carry the original segment's generic arguments onto the qualified path.
This case uses a generic Rust type; it applies to code that has already been edited after transpilation.
Reproducer
The input compiles. Run c2rust-refactor reorganize_definitions --rewrite-mode alongside -- repro.rs --edition=2021 and compile the resulting repro.new to see E0107.
#![feature(register_tool)]
#![register_tool(c2rust)]
#![allow(dead_code, non_camel_case_types, unused_imports)]
pub mod defs {
pub struct Value<T> { pub val: T }
}
pub mod user {
use crate::defs::*;
#[c2rust::header_src = "/tmp/user.h:1"]
pub mod user_h {
use crate::defs::*;
pub struct config { pub thing: Value<i32> }
}
pub fn go(_: user_h::config) {}
}
fn main() {}4cddd79 to
2b8842b
Compare
2b8842b to
f037a6c
Compare
f037a6c to
8f6a4ac
Compare
Follow-up limitation found while writing the test — FIXED: items moved out of a header could not rely on bindings that stay behind in the header (e.g. the kept glob). A bare path like `thing` that resolved through the glob was not canonicalized on the way out (`is_relative_path` treats only `self::`/`super::` as relative), so the moved item failed to compile with E0412. `move_items` now runs `canonicalize_moved_decl_paths` over every moved declaration (and its `impl` block): a single-segment path to a local, non-moving item def is rewritten to its absolute path via `def_qpath`. Paths are left alone when the destination module still binds the ident to the same def — through a pre-existing import, an import moving along with the declaration (moved `use` targets are now recorded in `ModuleInfo::import_targets`, mirroring `update_module_info_items`), or because the target is defined in the destination module itself — which keeps the existing snapshots byte-identical. Paths to moved defs are still rewritten later by `update_paths` via `path_mapping`. The `reorganize_glob_import` test now uses the bare `thing` field type and verifies it becomes `crate::defs::thing`.
Follow-up limitation found while writing the test — FIXED: items moved out
of a header could not rely on bindings that stay behind in the header (e.g.
the kept glob). A bare path like
thingthat resolved through the glob wasnot canonicalized on the way out (
is_relative_pathtreats onlyself::/super::as relative), so the moved item failed to compile withE0412.
move_itemsnow runscanonicalize_moved_decl_pathsover everymoved declaration (and its
implblock): a single-segment path to a local,non-moving item def is rewritten to its absolute path via
def_qpath.Paths are left alone when the destination module still binds the ident to
the same def — through a pre-existing import, an import moving along with
the declaration (moved
usetargets are now recorded inModuleInfo::import_targets, mirroringupdate_module_info_items), orbecause the target is defined in the destination module itself — which
keeps the existing snapshots byte-identical. Paths to moved defs are still
rewritten later by
update_pathsviapath_mapping. Thereorganize_glob_importtest now uses the barethingfield type andverifies it becomes
crate::defs::thing.Stack created with GitHub Stacks CLI • Give Feedback 💬