transpile: fixes before re-enabling --reorganize-definitions
#1452
Conversation
kkysen
force-pushed
the
kkysen/reorganize-definitions-prereqs
branch
from
fc57cd9 to
8e391d8
Compare
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
@fw-immunant, this is your own code, so I'm not sure if it makes sense for you to review, but it LGTM modulo a few cleanups I added in the last commit.
|
We should probably avoid entirely disabling the caching of zero values; I think I noticed c2rust-transpile taking significantly longer after that change. I believe the rest of these fixes were ready to merge. |
|
Cleanups in the final commit look good. |
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
kkysen
force-pushed
the
kkysen/reorganize-definitions-prereqs
branch
from
8e391d8 to
ed6ed73
Compare
I removed 828c6974b from this PR for now. I'll fix that in its own PR, and I also split out the non-enable-by-default parts of |
The only hesitation I have about merging this as-is is that the codepaths it alters are not tested by CI, because they only apply when |
|
They already passed in CI before, though. And we have other refactorer code that isn't tested in CI yet, still a WIP. |
This is split off from #1451, but doesn't enable `--reorganize-definitions` by default, so it shouldn't need the fixes in #1452 yet, of which the disabling caching of zero values needs to be fixed. Also, I'm not sure if we should enable `--reorganize-definitions` by default, since `c2rust-refactor` won't be installed by default.
Most of the changes from #1414 with #1451 split out. In combination with #1451, supersedes #1414.