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

Added regex functionality to the dominators sub-command. #68

Merged
merged 1 commit into from
May 23, 2018

Conversation

data-pup
Copy link
Member

@data-pup data-pup commented May 23, 2018

Regex commands can now be given to the dominators sub-command :o)

This required changing the root_id member of the DominatorTree struct into a Vec<ir::Id> collection, and some minor changes to the Emit trait logic. This generally follows the same patterns as the paths command, meaning that regular expressions are enabled manually using the --regex option.

Solves #59.

@data-pup data-pup changed the title Added regex functionality to the dominators sub-command. Solves #59. Added regex functionality to the dominators sub-command. May 23, 2018
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Nice! One question below, feel free to merge this after fixing/explaining that.

Thanks!


impl Dominators {
// TODO: wasm-bindgen does not support sending Option<String> across
// the wasm ABI boundary yet.
Copy link
Member

Choose a reason for hiding this comment

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

It is unclear to me how this TODO is relevant? Is this copy-pasted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good eye! This comment was mimicking what I saw in the implementation for Paths in that file. The comment should say: // TODO: wasm-bindgen does not support sending Vec<String> across the wasm ABI boundary yet.

I refactored the subtree member in this file from an Option<String> into a Vec<String> so that it could support multiple arguments in this commit. This was definitely a copy paste error 😬

I fixed this and squashed it into the commit, so it should be good to go now :)

@data-pup data-pup merged commit 51b4112 into rustwasm:master May 23, 2018
@data-pup data-pup deleted the add-dominators-regex branch August 20, 2018 16:08
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.

2 participants