-
Notifications
You must be signed in to change notification settings - Fork 280
Re-enable --reorganize-definitions flag
#1414
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
Conversation
ca53fc4 to
89ecc26
Compare
| "--cargo", | ||
| "--rewrite-mode", | ||
| "inplace", | ||
| "rename_unnamed", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had completely forgotten we had this, it probably would have fixed the issue from the libxml2 demo.
1a148be to
cbfd8f2
Compare
|
Fixed a couple issues where the tests were overly brittle; now in |
|
The problem seems to be that imports of needed types for SIMD get placed into submodules and then these types are not in scope for their later usage: #include <immintrin.h>
void simd_fn_codegen(__m128i i)
{
i = _mm_aesdeclast_si128(i, i);
}transpiles with #![allow(
dead_code,
non_camel_case_types,
non_snake_case,
non_upper_case_globals,
unused_assignments,
unused_mut
)]
#![register_tool(c2rust)]
#![feature(register_tool, stdsimd)]
#[c2rust::header_src = "/usr/lib/clang/21/include/mmintrin.h:1"]
pub mod mmintrin_h {
#[cfg(target_arch = "x86")]
pub use ::core::arch::x86::__m128i;
#[cfg(target_arch = "x86_64")]
pub use ::core::arch::x86_64::__m128i;
}
#[c2rust::header_src = "/usr/lib/clang/21/include/__wmmintrin_aes.h:1"]
pub mod __wmmintrin_aes_h {
#[cfg(target_arch = "x86")]
pub use ::core::arch::x86::_mm_aesdeclast_si128;
#[cfg(target_arch = "x86_64")]
pub use ::core::arch::x86_64::_mm_aesdeclast_si128;
}
#[cfg(target_arch = "x86")]
pub use ::core::arch::x86::_mm_setzero_si128;
#[cfg(target_arch = "x86_64")]
pub use ::core::arch::x86_64::_mm_setzero_si128;
#[no_mangle]
#[c2rust::src_loc = "3:1"]
pub unsafe extern "C" fn rust_simd_fn_codegen(mut i: __m128i) {
i = _mm_aesdeclast_si128(i, i);
} |
|
Ah, there's another problem here: if So it seems we need to fix this in the transpiler, not the refactorer, and may also need to alter how the transpiler is invoked by |
f09ec6a to
8e5a67c
Compare
|
The next set of errors in CI are the testsuite failing due to |
|
Now I'm seeing |
--reorganize-definitions flag--reorganize-definitions flag
f327502 to
f81ebb2
Compare
f81ebb2 to
f7c25bb
Compare
this closure is called when exporting types, and a convert_type method already exists
other exports from the submodule (for types it defines via enum/struct definitions) are already added to this set, but reexports were not. this meant that if we organized imports of std/core types into a submodule (which the transpiler itself does when `--reorganize-definitions` is passed), these imports would then be missing from the main module.
these will have `use`s added unconditionally with the stdlib path if the main module uses them directly, so in order to allow deduplication without tracing through reexports, we should have the main module import these from their original (stdlib) location, not the reexported one
computing these is impure because it can call `import_type`. ideally we would not disable this caching but instead cache both the initializer and its set of required imports.
This reverts commit 2bd339d.
Unfortunately, we have to bend over backwards to make this work without changing CLI behavior. This adds a new `--no-reorganize-definitions` flag to disable reorganizing definitions, because it would not be backwards-compatible to add an optional argument for the flag (it would eat a following argument that would previously be unassociated with the flag).
f7c25bb to
6efb84e
Compare
It's currently a no-op. We'll also want to make it run by default, which is where it seems more likely we'll run into CI/release trouble due to our complex rustc version requirements.