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

Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls #54850

Merged

Conversation

matthew-russo
Copy link
Contributor

Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Previously parsing trait items only handled opening brace token and semicolon, I added a branch to the match statement that will also handle interpolated blocks.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2018
@nikomatsakis
Copy link
Contributor

Can you add some form of test, @mcr431 ?

@petrochenkov
Copy link
Contributor

Whoa, fn test() $block worked for free functions all this time?
A had no idea.

@matthew-russo
Copy link
Contributor Author

@nikomatsakis sorry about that will add a test and update pr this evening.

@matthew-russo matthew-russo force-pushed the fix-54707-trait-function-from-macro branch from dce7980 to cf60a29 Compare October 5, 2018 20:21
@matthew-russo
Copy link
Contributor Author

@nikomatsakis added a test, let me know if you'd like any more or if it should be in a different spot. first time adding a new test.


trait Foo {
def_fn!({ println!("bar"); });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks =) I think it'd be good to include in the text the other various places one might use this as well -- e.g., for a free function etc. Skimming the existing tests I did find that macros/macro-interpolation.rs and macros/macro-stmt both test the free fn case but there don't appear to be tests in impls.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be good to just have one single test dedicated to testing that $b can be used as a fn body in all relevant contexts (the other tests are kind of testing a wider variety of things)

Copy link
Contributor Author

@matthew-russo matthew-russo Oct 8, 2018

Choose a reason for hiding this comment

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

Thanks for the feeback. will update and let you know when its ready

@nikomatsakis nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2018
@matthew-russo matthew-russo force-pushed the fix-54707-trait-function-from-macro branch from cf60a29 to 3447473 Compare October 9, 2018 02:50
@matthew-russo
Copy link
Contributor Author

@nikomatsakis I made a new test file: src/test/run-pass/macros/macro-as-fn-body.rs that includes the 3 situations described in the issue -- free function, method, and default trait method. If I'm missing any or you'd like something changed let me know

@matthew-russo
Copy link
Contributor Author

matthew-russo commented Oct 9, 2018

r? @nikomatsakis

edit: not sure how to change it from S-waiting-on-author to S-waiting-on-review

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 9, 2018

📌 Commit 3447473 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 9, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 9, 2018
…m-macro, r=nikomatsakis

Fix rust-lang#54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Fix rust-lang#54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Previously parsing trait items only handled opening brace token and semicolon, I added a branch to the match statement that will also handle interpolated blocks.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 10, 2018
…m-macro, r=nikomatsakis

Fix rust-lang#54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Fix rust-lang#54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Previously parsing trait items only handled opening brace token and semicolon, I added a branch to the match statement that will also handle interpolated blocks.
bors added a commit that referenced this pull request Oct 11, 2018
Rollup of 9 pull requests

Successful merges:

 - #54747 (codegen_llvm: verify that inline assembly operands are scalars)
 - #54848 (Better Diagnostic for Trait Object Capture)
 - #54850 (Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls)
 - #54858 (second round of refactorings for universes)
 - #54862 (Implement RFC 2539: cfg_attr with multiple attributes)
 - #54869 (Fix mobile docs)
 - #54870 (Stabilize tool lints)
 - #54893 (Fix internal compiler error on malformed match arm pattern.)
 - #54904 (Stabilize the `Option::replace` method)

Failed merges:

 - #54909 ( Add chalk rules related to associated type defs)

r? @ghost
bors added a commit that referenced this pull request Oct 11, 2018
Rollup of 9 pull requests

Successful merges:

 - #54747 (codegen_llvm: verify that inline assembly operands are scalars)
 - #54848 (Better Diagnostic for Trait Object Capture)
 - #54850 (Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls)
 - #54858 (second round of refactorings for universes)
 - #54862 (Implement RFC 2539: cfg_attr with multiple attributes)
 - #54869 (Fix mobile docs)
 - #54870 (Stabilize tool lints)
 - #54893 (Fix internal compiler error on malformed match arm pattern.)
 - #54904 (Stabilize the `Option::replace` method)

Failed merges:

 - #54909 ( Add chalk rules related to associated type defs)

r? @ghost
@bors
Copy link
Contributor

bors commented Oct 11, 2018

⌛ Testing commit 3447473 with merge a534216...

bors added a commit that referenced this pull request Oct 11, 2018
…nikomatsakis

Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls

Previously parsing trait items only handled opening brace token and semicolon, I added a branch to the match statement that will also handle interpolated blocks.
@bors
Copy link
Contributor

bors commented Oct 11, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing a534216 to master...

@bors bors merged commit 3447473 into rust-lang:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants