transpile: replace --reorganize-definitions/--disable-refactoring with --emit-refactor-annotations/--refactor#1640
transpile: replace --reorganize-definitions/--disable-refactoring with --emit-refactor-annotations/--refactor#1640
--reorganize-definitions/--disable-refactoring with --emit-refactor-annotations/--refactor#1640Conversation
… with `--emit-refactor-annotations`/`--refactor` Previously, `--reorganize-definitions` would emit annotations and run the refactor, with `--disable-refactoring` needed to only emit annotations. This simplifies things to additive flags. `--emit-refactor-annotations` only emits the annotations. `--refactor` implies `--emit-refactor-annotations` and runs the default refactor transforms.
| let json = json!({ | ||
| "lib_rs_file": file_name, | ||
| "reorganize_definitions": tcfg.reorganize_definitions, | ||
| "reorganize_definitions": tcfg.emit_refactor_annotations, |
There was a problem hiding this comment.
@ahomescu, is this the correct change here? I'm not familiar with how cross-checks are supposed to work.
There was a problem hiding this comment.
--reorganize-definitions does not just enable annotations, it also changes the structure of the emitted code to include modules and cross-module imports (see generate_submodule_imports and the block labelled "Header Reorganization: Submodule Item Stores" in translator/mod.rs, which only has an effect when individual files' definition sets are tracked, a side effect of the reorganize_definitions bool). So while we should probably clarify the name, the new name shouldn't suggest that the flag is merely additive.
We could maybe split a separate boolean out to control the transpiler's submodule/header behavior, or use a name that clarifies that this is tied together with the reorganize-definitions annotations.
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
--reorganize-definitionsdoes not just enable annotations, it also changes the structure of the emitted code to include modules and cross-module imports (seegenerate_submodule_importsand the block labelled "Header Reorganization: Submodule Item Stores" intranslator/mod.rs, which only has an effect when individual files' definition sets are tracked, a side effect of thereorganize_definitionsbool). So while we should probably clarify the name, the new name shouldn't suggest that the flag is merely additive.
Ahh. Why isn't this done always, then? I thought the reorganize_definitions transform is the one that replace duplicate definitions with cross-module imports.
We could maybe split a separate boolean out to control the transpiler's submodule/header behavior, or use a name that clarifies that this is tied together with the reorganize-definitions annotations.
What's a good name for this?
2 participants
Previously,
--reorganize-definitionswould emit annotations and run the refactor, with--disable-refactoringneeded to only emit annotations. This simplifies things to additive flags.--emit-refactor-annotationsonly emits the annotations.--refactorimplies--emit-refactor-annotationsand runs the default refactor transforms.