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

Add path suffix lookup #16326

Merged
merged 5 commits into from Aug 9, 2014
Merged

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Aug 7, 2014

Extended ast_map::Map with an iterator over all node id's that match a path suffix.

Extended pretty printer to let users choose particular items to pretty print, either by indicating an integer node-id, or by providing a path suffix.

  • Example 1: the suffix typeck::check::check_struct matches the item with the path rustc::middle::typeck::check::check_struct when compiling the rustc crate.
  • Example 2: the suffix and matches core::option::Option::and and core::result::Result::and when compiling the core crate.

Refactored pprust slightly to support the pretty printer changes.

(See individual commits for more description.)

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 7, 2014

This enhancement was inspired during my work on for non-zeroing drop.

It makes working with --pretty flowgraph so so so much better, for two reasons:

  1. you can use path suffixes instead of node-id's as input to flowgraph=... when the suffix uniquely identifies the node-id, and
  2. you can use --pretty expanded,identified=<path-suffix> to find out all of the node-id's for a given path suffix, for cases when the desired suffix does not uniquely identify the node-id.

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 7, 2014

r? anyone


trait AstMapCarrier {
/// Provides a uniform interface for re-extracting a reference to an
/// from a value that now owns it.
Copy link
Member Author

Choose a reason for hiding this comment

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

typo: missing ast_map::Map here

@huonw
Copy link
Member

huonw commented Aug 7, 2014

Will review sometime in the next day or so (if someone else doesn't get to it first).

/// match `parts`. (Requires `parts` is non-empty.)
///
/// For example, if given `parts` equal to `["bar", "quux"]`, then
/// the iterator will produce node id's such as items with paths
Copy link
Member

Choose a reason for hiding this comment

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

I can't parse this sentence? (Is there an extra 'such as'?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I meant "will produce node-ids for items with paths such as ..."

@huonw
Copy link
Member

huonw commented Aug 8, 2014

There's probably now enough pretty-printing code to warrant a driver::pretty module.

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 8, 2014

There's probably now enough pretty-printing code to warrant a driver::pretty module.

Probably true, but I am not planning to throw that refactoring into this PR. :)

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 8, 2014

r? @huonw

@huonw
Copy link
Member

huonw commented Aug 9, 2014

r=me with some tests, e.g. checking that running --pretty normal=foo on fn foo() {} fn bar() {} will only print one of them (and not crash the compiler etc.).

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 9, 2014

@huonw heh, hoped to slip the lack of tests by you. :)

But now that I think about it, it will be easy to just change/adapt one of the existing run-make/graphviz-flowgraph tests (or all of them) that have that hacky FIND_LAST_BLOCK in the Makefile.

(Though I suppose you asked for --pretty normal, not --pretty flowgraph. Still, easy enough to add a run-make test, either way.)

This is useful e.g. for tools need a node-id, such as the flowgraph
pretty printer, since it can avoids the need to first pretty-print the
whole expanded,identified input in order to find out what the node-id
actually is.

It currently only supports path suffixes thst are made up of module
names (e.g. you cannot use the type instantiation form `a::<int>::b`
or `option::Option::unwrap_or` as a path suffix for this tool, though
the tool will produce paths that have non-modulues in the portion of
the path that is not included in the suffix).

(addressed review feedback too)
…crate`.

(Groundwork for pretty-printing only selected items in an input crate.)
With this change:

  * `--pretty variant=<node-id>` will print the item associated with
    `<node-id>` (where `<node-id>` is an integer for some node-id in
    the AST, and `variant` means one of {`normal`,`expanded`,...}).

  * `--pretty variant=<path-suffix>` will print all of the items that
    match the `<path-suffix>` (where `<path-suffix>` is a suffix of a
    path, and `variant` again means one of {`normal`,`expanded`,...}).

    Example 1: the suffix `typeck::check::check_struct` matches the
    item with the path `rustc::middle::typeck::check::check_struct`
    when compiling the `rustc` crate.

    Example 2: the suffix `and` matches `core::option::Option::and`
    and `core::result::Result::and` when compiling the `core` crate.

Both of the `--pretty variant=...` modes will include the full path to
the item in a comment that follows the item.

Note that when multiple paths match, then either:

  1. all matching items are printed, in series; this is what happens in
     the usual pretty-print variants, or

  2. the compiler signals an error; this is what happens in flowgraph
     printing.

----

Some drive-by improvements:

Heavily refactored the pretty-printing glue in driver.rs, introducing
a couple local traits to avoid cut-and-pasting very code segments that
differed only in how they accessed the `Session` or the
`ast_map::Map`. (Note the previous code had three similar calls to
`print_crate` which have all been unified in this revision; the
addition of printing individual node-ids exacerbated the situation
beyond tolerance.) We may want to consider promoting some of these
traits, e.g. `SessionCarrier`, for use more generally elsewhere in the
compiler; right now I have to double check how to access the `Session`
depending on what context I am hacking in.

Refactored `PpMode` to make the data directly reflect the fundamental
difference in the categories (in terms of printing source-code with
various annotations, versus printing a control-flow graph).

(also, addressed review feedback.)
bors added a commit that referenced this pull request Aug 9, 2014
Extended `ast_map::Map` with an iterator over all node id's that match a path suffix.

Extended pretty printer to let users choose particular items to pretty print, either by indicating an integer node-id, or by providing a path suffix.

 * Example 1: the suffix `typeck::check::check_struct` matches the item with the path `rustc::middle::typeck::check::check_struct` when compiling the `rustc` crate.

 * Example 2: the suffix `and` matches `core::option::Option::and` and `core::result::Result::and` when compiling the `core` crate.

Refactored `pprust` slightly to support the pretty printer changes.

(See individual commits for more description.)
@bors bors closed this Aug 9, 2014
@bors bors merged commit db0e71f into rust-lang:master Aug 9, 2014
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.

None yet

3 participants