Skip to content

/rust

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

Avoid gated feature checking unconfigured expanded items #32846

Merged
merged 4 commits into from Apr 30, 2016

Conversation

Reviewers
No reviews
Assignees

@nrc nrc

Labels
T-lang
Projects
None yet
Milestone
No milestone
5 participants
@jseyfried @nrc @nikomatsakis @bors @LeoTestard
@jseyfried
Copy link
Contributor

jseyfried commented Apr 8, 2016

Avoid gated feature checking unconfigured macro-expanded items (fixes #32840).
Unconfigured items that are not macro-expanded are already not gated feature checked.
r? @nrc

@rust-highfive rust-highfive assigned nrc Apr 8, 2016

src/librustc_driver/driver.rs Outdated
*sess.features.borrow_mut() = features;
})
})?;

This comment has been minimized.

Sign in to view
Copy link
@LeoTestard

LeoTestard Apr 8, 2016

Contributor

There is already a second feature gate checking pass just a bit later in the same function (“complete gated feature checking 2”). I'm not sure this first pass is necessary at all. I considered removing it as part of #32791 but then thought I'd do a separate PR.

This comment has been minimized.

Sign in to view
Copy link
@jseyfried

jseyfried Apr 9, 2016

Author Contributor

Good point, I'll remove it.
src/libsyntax/ext/expand.rs
let it = expand_item_multi_modifier(Annotatable::Item(it), fld);

expand_annotatable(it, fld)
expand_annotatable(Annotatable::Item(it), fld)

This comment has been minimized.

Sign in to view
Copy link
@jseyfried

jseyfried Apr 9, 2016

Author Contributor

expand_annotatable begins with a call to expand_item_multi_modifier, which is idempotent (cf #21052).
This is unrelated to the rest of the PR.

@nrc nrc added I-nominated T-lang labels Apr 12, 2016

@nrc

This comment has been minimized.

Sign in to view
Copy link
Member

nrc commented Apr 12, 2016

Code looks fine, but @rust-lang/lang should discuss.

@jseyfried jseyfried referenced this pull request Apr 22, 2016

Closed

`#[cfg_attr]` doesn't work with `path` attribute #25544

@jseyfried

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jseyfried commented Apr 22, 2016

cf #25544
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Apr 28, 2016

Finally discussed in lang-team meeting. We agree that feature-gated items which are "configured out" by should not generate warnings, whether they result from macro expansion or not. i.e., this PR seems good. :)

@nikomatsakis nikomatsakis removed the I-nominated label Apr 28, 2016

@nrc

This comment has been minimized.

Sign in to view
Copy link
Member

nrc commented Apr 28, 2016

@bors: r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 28, 2016

📌 Commit 86f069d has been approved by nrc
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 28, 2016

⌛️ Testing commit 86f069d with merge 8bd01ad...

bors added a commit that referenced this pull request Apr 28, 2016

@bors
Auto merge of #32846 - jseyfried:allow_unconfigured_gated_expanded_it… 
…ems, r=nrc

Avoid gated feature checking unconfigured expanded items

Avoid gated feature checking unconfigured macro-expanded items (fixes #32840).
Unconfigured items that are not macro-expanded are already not gated feature checked.
r? @nrc
8bd01ad
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 29, 2016

💔 Test failed - auto-win-msvc-64-opt
@jseyfried

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jseyfried commented Apr 30, 2016

@bors retry
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 30, 2016

⌛️ Testing commit 86f069d with merge c5ec369...

bors added a commit that referenced this pull request Apr 30, 2016

@bors
Auto merge of #32846 - jseyfried:allow_unconfigured_gated_expanded_it… 
…ems, r=nrc

Avoid gated feature checking unconfigured expanded items

Avoid gated feature checking unconfigured macro-expanded items (fixes #32840).
Unconfigured items that are not macro-expanded are already not gated feature checked.
r? @nrc
c5ec369
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 30, 2016

💔 Test failed - auto-mac-64-nopt-t
@jseyfried

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

jseyfried commented Apr 30, 2016

@bors retry
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Apr 30, 2016

⌛️ Testing commit 86f069d with merge b0aefff...

bors added a commit that referenced this pull request Apr 30, 2016

@bors
Auto merge of #32846 - jseyfried:allow_unconfigured_gated_expanded_it… 
…ems, r=nrc

Avoid gated feature checking unconfigured expanded items

Avoid gated feature checking unconfigured macro-expanded items (fixes #32840).
Unconfigured items that are not macro-expanded are already not gated feature checked.
r? @nrc
b0aefff
@bors

@bors bors merged commit 86f069d into rust-lang:master Apr 30, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@LeoTestard LeoTestard referenced this pull request May 29, 2016

Closed

#[main] attribute on #[test] function does not error properly #33946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.