Uh oh!
There was an error while loading. Please reload this page.
Change EraseRegions pass to use MutVisitor - #31324
Conversation
rust-highfive
commented
Jan 31, 2016
r? @Aatch (rust_highfive has picked a reviewer for you, use r? to override) |
Aatch
commented
Feb 1, 2016
Hmm, I'm all for reducing maintenance and duplication, but I worry about future development. If we want to add pass-specific functionality, this change would make that harder. Either that or require adding unrelated pass-specific methods to the visitors, which doesn't seem like a good thing. The solution to " /cc @rust-lang/compiler am I over-engineering this? |
nagisa
commented
Feb 1, 2016
Right, I don’t find use of I’ve replaced Visitor implementation on SimplifyCfg with a inherent method.
¹: The only pattern |
15ede21 to
cf3318fComparemichaelwoerister
commented
Feb 1, 2016
To give some context: I wrote |
nikomatsakis
commented
Feb 1, 2016
+1 to using a visitor, but I think I share @Aatch's concerns that we don't want to force all passes to use visitors. |
nikomatsakis
commented
Feb 1, 2016
That said, I expect the |
nagisa
commented
Feb 4, 2016
Greatly reduced scope to only contain move from custom visitor to MutVisitor for EraseRegions. Everything else will eventually come in different PRs. |
bors
commented
Feb 6, 2016
☔ The latest upstream changes (presumably #31307) made this pull request unmergeable. Please resolve the merge conflicts. |
nikomatsakis
commented
Feb 8, 2016
@bors r+ |
bors
commented
Feb 8, 2016
📌 Commit 0b3ef97 has been approved by |
bors
commented
Feb 8, 2016
Having a `MirPass` provides literally no benefits over `MutVisitor`. Moreover using `MirPass` for `EraseRegions` basically makes the programmer to fix breakage from changing repr twice – in the visitor and eraseregions. Since `MutVisitor` implements all the “walking” inside the trait, that can be reused for `EraseRegions` too, basically resulting in less code duplication.
Having a
MirPassprovides literally no benefits overMutVisitor. Moreover usingMirPassforEraseRegionsbasically makes the programmer to fix breakage from changing repr twice – in thevisitor and eraseregions. Since
MutVisitorimplements all the “walking” inside the trait, that canbe reused for
EraseRegionstoo, basically resulting in less code duplication.