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.

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

Introduce Expr use visitor #13724

Merged
merged 5 commits into from
May 1, 2014
Merged

Conversation

nikomatsakis
Copy link
Contributor

Pre-step towards issue #12624 and others: Introduce ExprUseVisitor, remove the
moves computation. ExprUseVisitor is a visitor that walks the AST for a
function and calls a delegate to inform it where borrows, copies, and moves
occur.

In this patch, I rewrite the gather_loans visitor to use ExprUseVisitor, but in
future patches, I think we could rewrite regionck, check_loans, and possibly
other passes to use it as well. This would refactor the repeated code between
those places that tries to determine where copies/moves/etc occur.

r? @alexcrichton

@alexcrichton
Copy link
Member

This looks good to me, but I'm not sure I have the relevant chops to sign off on this. I think @pnkfelix, @nrc, or @pcwalton have been running around in this part of the compiler much more than I have recently

…isitor, remove the

moves computation. ExprUseVisitor is a visitor that walks the AST for a
function and calls a delegate to inform it where borrows, copies, and moves
occur.

In this patch, I rewrite the gather_loans visitor to use ExprUseVisitor, but in
future patches, I think we could rewrite regionck, check_loans, and possibly
other passes to use it as well. This would refactor the repeated code between
those places that tries to determine where copies/moves/etc occur.
@nikomatsakis
Copy link
Contributor Author

ok r? @pnkfelix @pcwalton or @nick29581 :)

_consume_span: Span,
_cmt: mc::cmt,
_mode: ConsumeMode)
{ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe at this point the Delegate trait should not provide no-op default implementations; I'd prefer to force the Delegate implementors to have to write their own no-op implementations. This isn't like Visitor where the there is useful/necessary behavior in the default methods. At least I would wait until we have seen it implemented on other analysis.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@nikomatsakis
Copy link
Contributor Author

@pnkfelix addressed nits

bors added a commit that referenced this pull request May 1, 2014
Pre-step towards issue #12624 and others: Introduce ExprUseVisitor, remove the
moves computation. ExprUseVisitor is a visitor that walks the AST for a
function and calls a delegate to inform it where borrows, copies, and moves
occur.

In this patch, I rewrite the gather_loans visitor to use ExprUseVisitor, but in
future patches, I think we could rewrite regionck, check_loans, and possibly
other passes to use it as well. This would refactor the repeated code between
those places that tries to determine where copies/moves/etc occur.

r? @alexcrichton
@bors bors closed this May 1, 2014
@bors bors merged commit b9af043 into rust-lang:master May 1, 2014
@nikomatsakis nikomatsakis deleted the expr-use-visitor branch March 30, 2016 16:13
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 28, 2024
For example, we definitely wouldn't want to do this in libcore.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants