Skip to content
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.

Sign up for GitHub

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

Jump to bottom

mir_framework: Problem with exports used by external tools #120130

Closed
jubnzv opened this issue Jan 19, 2024 · 2 comments · Fixed by #120158
Closed

mir_framework: Problem with exports used by external tools #120130

jubnzv opened this issue Jan 19, 2024 · 2 comments · Fixed by #120158
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jubnzv
Copy link
Contributor

jubnzv commented Jan 19, 2024

c16d3f3 has made it impossible to import some structures in rustc_mir_framework when implementing dataflow analyses in external tools.

When implementing AnalysisDomain for a custom dataflow problem, we can no longer import the Forward or Backward direction. Here is the source code of such an implementation. Forward is needed to set the Direction type of the AnalysisDomain trait. Previously, it was available under the pub use export, but for now it is private.

The default implementation of the AnalysisDomain::Direction is Forward, so I can simply use the default value as a workaround. However, if I need to write a backward analysis, I must rewrite some parts of the framework in our source code because they cannot be imported.

There are other tools in the ecosystem that use that API, e.g.:

These changes will make problems if they decide to update to the recent toolchain.

Could you please take a look at this matter? Are these changes intentional? Could we make some of the exports public again?
cc @nnethercote
@jubnzv jubnzv added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jan 19, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 19, 2024
@Noratrieb
Copy link
Member

Feel free to send in a PR to make it public again. Please add a comment on the items saying they're used by external tools.

@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jan 19, 2024
@jubnzv
Copy link
Contributor Author

jubnzv commented Jan 19, 2024

I will double-check which of the removed imports might be beneficial for external tools and will send a PR if there are no objections.

@jubnzv jubnzv mentioned this issue Jan 19, 2024
Merged
5 tasks
jubnzv added a commit to jubnzv/rust that referenced this issue Jan 20, 2024
@jubnzv
rustc_mir_dataflow: Add exports for external tools
1605f47 
Closes rust-lang#120130
@jubnzv jubnzv mentioned this issue Jan 20, 2024
Merged
jubnzv added a commit to jubnzv/rust that referenced this issue Jan 20, 2024
@jubnzv
rustc_mir_dataflow: Add exports for external tools
ecbe92f 
Added back previously available exports:

* Forward/Backward: used when implementing `AnalysisDomain`
* Engine: used in user's code to solve the dataflow problem
* SwitchIntEdgeEffects: used when implementing functions of the `Analysis` trait
* graphviz: potentially useful for debugging purposes

Closes rust-lang#120130
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 21, 2024
@matthiaskrgr
Rollup merge of rust-lang#120158 - jubnzv:120130-mirdf-exports, r=nne…
d3172f2 
…thercote

`rustc_mir_dataflow`: Restore removed exports

Added back previously available exports:

* `Forward`/`Backward`: used when implementing `AnalysisDomain`
* `Engine`: used in user's code to solve the dataflow problem
* `SwitchIntEdgeEffects`: used when implementing functions of the `Analysis` trait
* `graphviz`: potentially useful for debugging purposes

Closes rust-lang#120130
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 21, 2024
@matthiaskrgr
Rollup merge of rust-lang#120158 - jubnzv:120130-mirdf-exports, r=nne…
e59a6fe 
…thercote

`rustc_mir_dataflow`: Restore removed exports

Added back previously available exports:

* `Forward`/`Backward`: used when implementing `AnalysisDomain`
* `Engine`: used in user's code to solve the dataflow problem
* `SwitchIntEdgeEffects`: used when implementing functions of the `Analysis` trait
* `graphviz`: potentially useful for debugging purposes

Closes rust-lang#120130
@bors bors closed this as completed in 270f151 Jan 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 22, 2024
@rust-timer
Unrolled build for rust-lang#120158
279f499 
Rollup merge of rust-lang#120158 - jubnzv:120130-mirdf-exports, r=nnethercote

`rustc_mir_dataflow`: Restore removed exports

Added back previously available exports:

* `Forward`/`Backward`: used when implementing `AnalysisDomain`
* `Engine`: used in user's code to solve the dataflow problem
* `SwitchIntEdgeEffects`: used when implementing functions of the `Analysis` trait
* `graphviz`: potentially useful for debugging purposes

Closes rust-lang#120130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rustc_mir_dataflow: Restore removed exports jubnzv/rust
3 participants
@jubnzv @rustbot @Noratrieb