Skip to content

Attributes on generic formals#34764

Merged
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:attrs-on-generic-formals
Oct 1, 2016
Merged

Attributes on generic formals#34764
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:attrs-on-generic-formals

Conversation

@pnkfelix

Copy link
Copy Markdown
Contributor

First step for #34761

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @sfackler

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

@eddyb

eddyb commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

r? @eddyb

@bors r+

@bors

bors commented Jul 11, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 337073b has been approved by eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned sfackler Jul 11, 2016
@durka

durka commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

plugin-breaking, no?

@eddyb

eddyb commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

@bors r-

Ah, yes, of course. Thanks @durka. @Manishearth r=me for the next breaking batch or whatnot.

Comment thread src/libsyntax/parse/parser.rs Outdated

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.

Nit: s/parsing/parse

@bors

bors commented Jul 20, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #34113) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix pnkfelix force-pushed the attrs-on-generic-formals branch from d554c5b to 2605680 Compare July 22, 2016 15:33
Comment thread src/libsyntax/visit.rs Outdated

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.

Why deref here, but not with the bounds?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Its necessary (here and below) because &bounds is a &Vec while &attrs is a &ThinVec.

Right now, you cannot iterate over a &ThinVec directly; you need to deref the ThinVec to turn it into a [T] slice that you can then iterate over.

@dns2utf8

Copy link
Copy Markdown
Contributor

I wonder why you used deref with the walk_list! macros, but the old code does not.

@eddyb

eddyb commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

ping @Manishearth

@Manishearth

Copy link
Copy Markdown
Member

Oh, you need this merged? Cool, I'll start up a batch.

(In the future, for high-pri or generally blocking/bitrotty things, please let me know that that's the case and I'll immediately initiate a breaking batch)

@bors

bors commented Aug 12, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #35091) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix

pnkfelix commented Sep 9, 2016

Copy link
Copy Markdown
Contributor Author

ping @Manishearth ; did you end up not starting up that batch?

@pnkfelix

pnkfelix commented Sep 9, 2016

Copy link
Copy Markdown
Contributor Author

or maybe I should resolve the merge conflicts. I'll do that now. :)

@Manishearth

Copy link
Copy Markdown
Member

The batch happened, but this PR wasn't in it, not sure why :/

@pnkfelix pnkfelix force-pushed the attrs-on-generic-formals branch from 2605680 to 115b74f Compare September 9, 2016 12:12
@bors

bors commented Sep 10, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #36332) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix pnkfelix force-pushed the attrs-on-generic-formals branch from 115b74f to 3a9b7be Compare September 23, 2016 14:03
I am using `ThinAttributes` rather than a vector for attributes
attached to generics, since I expect almost all lifetime and types
parameters to not carry any attributes.
@pnkfelix

Copy link
Copy Markdown
Contributor Author

ping @Manishearth

@Manishearth

Copy link
Copy Markdown
Member

It's in the list. Traveling right now, can't shepherd the rollup

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.

9 participants