Skip to content

Refactored syntax::fold #15999

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

Merged
merged 2 commits into from
Jul 31, 2014
Merged

Refactored syntax::fold #15999

merged 2 commits into from
Jul 31, 2014

Conversation

Kimundi
Copy link
Member

@Kimundi Kimundi commented Jul 26, 2014

Note: This PR is motivated by an attempt to write an custom syntax extension that tried to use syntax::fold, and that could only do so by fixing bugs in it and copying out private functions.


Refactored syntax::fold

Prior to this, the code there had a few issues:

  • Default implementations inconsistenly either had the prefix noop_ or
    not.
  • Some default methods where implemented in terms of a public noop function
    for user code to call, others where implemented directly on the trait
    and did not allow users of the trait to reuse the code.
  • Some of the default implementations where private, and thus not reusable
    for other implementors.
  • There where some bugs where default implemntations called other default
    implementations directly, rather than to the underlying Folder, with the
    result of some ast nodes never being visted even if the user implemented that
    method. (For example, the current Folder never folded struct fields)

This commit solves this situation somewhat radically by making all
fold_... functions in the module into Folder methods, and implementing
them all in terms of public noop_... functions for other implementors to
call out to.

Some public functions had to be renamed to fit the new system, so this is a
breaking change.


Also added a few trait implementations to ast types

pub trait Folder {
// Any additions to this trait should happen in form
// of a call to a public `noop_*` function that onyly calls
Copy link
Member

Choose a reason for hiding this comment

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

onyly

@ghost
Copy link

ghost commented Jul 26, 2014

@Kimundi This is great.

@lilyball
Copy link
Contributor

Commit message nitpicks: Multiple usages of the word "where" instead of "were" and at least one typo ("implemntations")

Kimundi added 2 commits July 29, 2014 12:31
Prior to this, the code there had a few issues:

- Default implementations inconsistently either had the prefix `noop_` or
  not.
- Some default methods where implemented in terms of a public noop function
  for user code to call, others where implemented directly on the trait
  and did not allow users of the trait to reuse the code.
- Some of the default implementations where private, and thus not reusable
  for other implementors.
- There where some bugs where default implementations called other default
  implementations directly, rather than to the underlying Folder, with the
  result of some AST nodes never being visited even if the user implemented that
  method. (For example, the current Folder never folded struct fields)

This commit solves this situation somewhat radically by making _all_
`fold_...` functions in the module into Folder methods, and implementing
them all in terms of public `noop_...` functions for other implementors to
call out to.

Some public functions had to be renamed to fit the new system, so this is a
breaking change.

[breaking-change]
bors added a commit that referenced this pull request Jul 31, 2014
Note: This PR is motivated by an attempt to write an custom syntax extension that tried to use `syntax::fold`, and that could only do so by fixing bugs in it and copying out private functions.

---

Refactored `syntax::fold`

Prior to this, the code there had a few issues:

- Default implementations inconsistenly either had the prefix `noop_` or
  not.
- Some default methods where implemented in terms of a public noop function
  for user code to call, others where implemented directly on the trait
  and did not allow users of the trait to reuse the code.
- Some of the default implementations where private, and thus not reusable
  for other implementors.
- There where some bugs where default implemntations called other default
  implementations directly, rather than to the underlying Folder, with the
  result of some ast nodes never being visted even if the user implemented that
  method. (For example, the current Folder never folded struct fields)

This commit solves this situation somewhat radically by making __all__
`fold_...` functions in the module into Folder methods, and implementing
them all in terms of public `noop_...` functions for other implementors to
call out to.

Some public functions had to be renamed to fit the new system, so this is a
breaking change.

---

Also added a few trait implementations to `ast` types
@bors bors closed this Jul 31, 2014
@bors bors merged commit da6070d into rust-lang:master Jul 31, 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.

5 participants