Skip to content

Grammar: Functions, methods, impl, traits, extern. - #430

Merged
matthewjasper merged 5 commits into
rust-lang:masterfrom
ehuss:grammar-fn-stuff
Oct 3, 2018
Merged

Grammar: Functions, methods, impl, traits, extern.#430
matthewjasper merged 5 commits into
rust-lang:masterfrom
ehuss:grammar-fn-stuff

Conversation

@ehuss

Copy link
Copy Markdown
Contributor

These all are intertwined with the "function" definition. Updates grammar for:

  • Functions and methods
  • Extern blocks
  • Implementation
  • Traits

@ehuss

ehuss commented Sep 24, 2018

Copy link
Copy Markdown
ContributorAuthor

Question: Would it be clearer if Method was split into two (Method | Function) and make Method something that strictly has a self parameter? I'm thinking it would be clearer.
EDIT: Although it could be complicated in the future due to arbitrary_self_types.

Also, I missed the 2015/2018 difference of trait functions taking optional named parameters. I'm thinking of documenting the 2015 behavior with a dagger (or something) that links to the section that describes that it is no longer allowed. Thoughts?

@ehussehuss mentioned this pull request Sep 25, 2018

@matthewjaspermatthewjasper left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great, left a few small comments

Comment threadsrc/items/functions.md Outdated
> _BlockWithInnerAttributes_ :\
>    `{`\
> &nbsp;&nbsp; &nbsp;&nbsp; [_InnerAttribute_]<sup>\*</sup>\
> &nbsp;&nbsp; &nbsp;&nbsp; [_Statement_]<sup>\*</sup>\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing an optional expression

Comment threadsrc/items/traits.md Outdated

## Unsafe traits

Traits that begin with the `unsafe` keyword indicate that *implementing* the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would be clearer as "Trait items that..."

Comment threadsrc/items/traits.md Outdated

Traits that begin with the `unsafe` keyword indicate that *implementing* the
trait may be [unsafe]. It is safe to use a correctly implemented unsafe trait.
The [trait implementation] must also include the `unsafe` keyword.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"include" should be "start with" or "begin with"

@matthewjasper

Copy link
Copy Markdown
Contributor

Question: Would it be clearer if Method was split into two (Method | Function) and make Method something that strictly has a self parameter? I'm thinking it would be clearer.
EDIT: Although it could be complicated in the future due to arbitrary_self_types.

I'm not sure how arbitrary self types changes anything here. It would probably be slightly better for methods to only include things with self parameters, since I think it is the more common meaning in Rust.

@ehuss

ehuss commented Oct 1, 2018

Copy link
Copy Markdown
ContributorAuthor

Thanks for taking a look. I've done some updates to fix a bunch of things and specify functions and methods separately. I added some detail about pattern limitations for parameters.

These all are intertwined with the "function" definition. Updates grammar for:
- Functions and methods
- Extern blocks
- Implementation
- Traits
- Review comments.
- Separate Function and Method in impl.
- Move _Method_ grammar to the section discussing methods.
- Be a little more explicit about `self` types.
- Fix block ending with expression for functions.
- Fix extern blocks not supporting patterns.
- Fix trait functions/methods having optional patterns, and the pattern restrictions.
@ehuss

ehuss commented Oct 2, 2018

Copy link
Copy Markdown
ContributorAuthor

I attempted to explain the 2018 edition changes for trait parameter patterns.

@matthewjasper

Copy link
Copy Markdown
Contributor

Thanks!

@matthewjasper
matthewjasper merged commit ac6a74f into rust-lang:masterOct 3, 2018
Sign up for freeto 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

@ehuss@matthewjasper