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

Visit all attributes for feature collection #53397

Merged
merged 1 commit into from
Aug 17, 2018

Conversation

varkor
Copy link
Member

@varkor varkor commented Aug 15, 2018

Previously feature attributes were just collected on item-like "things" as well as exported macros and crate attributes. This ignored some places feature attributes could be specified, such as on enum variants.

Fixes #53391.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 15, 2018

📌 Commit 23d10ec has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2018
@nikomatsakis
Copy link
Contributor

@varkor this is kind of a regression, I suppose?

@varkor
Copy link
Member Author

varkor commented Aug 15, 2018

@nikomatsakis: this change potentially permits more features than before (for example the future_atomic_orderings feature in #53391). It shouldn't disallow any that were previously allowed.

(I also imagine the current behaviour didn't miss many feature attributes, because most would have appeared in tests. future_atomic_orderings only didn't because it was deliberately perma-unstable.)

@nikomatsakis
Copy link
Contributor

@varkor sorry, to be clear, what I meant is:

This PR is fixing a regression where we were previously getting errors about "unrecognized feature" when indeed the feature was legitimate?

In particular I was wondering if we want p=1

@varkor
Copy link
Member Author

varkor commented Aug 16, 2018

@nikomatsakis: oh, sorry, yes that is the case. p=1 seems reasonable.

@bors p=1

@bors
Copy link
Contributor

bors commented Aug 16, 2018

⌛ Testing commit 23d10ec with merge f7d31968fb2cc3a847ae5a8034b350d3981be744...

@eddyb
Copy link
Member

eddyb commented Aug 16, 2018

@bors retry (higher priory PR had a spurious failure)

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Cloning into 'rust-lang/rust'...
travis_time:end:0b30505b:start=1534453940520227768,finish=1534453945826607108,duration=5306379340
$ cd rust-lang/rust
$ git checkout -qf f7d31968fb2cc3a847ae5a8034b350d3981be744
fatal: reference is not a tree: f7d31968fb2cc3a847ae5a8034b350d3981be744
The command "git checkout -qf f7d31968fb2cc3a847ae5a8034b350d3981be744" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Aug 16, 2018

⌛ Testing commit 23d10ec with merge 81ba448...

bors added a commit that referenced this pull request Aug 16, 2018
…omatsakis

Visit all attributes for feature collection

Previously feature attributes were just collected on item-like "things" as well as exported macros and crate attributes. This ignored some places feature attributes could be specified, such as on enum variants.

Fixes #53391.
@bors
Copy link
Contributor

bors commented Aug 17, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 81ba448 to master...

@bors bors merged commit 23d10ec into rust-lang:master Aug 17, 2018
@varkor varkor deleted the feature-collector-expand-visitor branch August 17, 2018 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants