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.

Sign up for GitHub

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

Jump to bottom

Deny #[cfg] and #[cfg_attr] on generic parameters. #51283

Merged
merged 1 commit into from
Jun 8, 2018
Merged

Deny #[cfg] and #[cfg_attr] on generic parameters. #51283

merged 1 commit into from
Jun 8, 2018

Conversation

kennytm
Copy link
Member

@kennytm kennytm commented Jun 1, 2018

Fix #51279.

Attributes on generic parameters are not expanded, meaning #[cfg], #[cfg_attr] and attribute proc macros are entirely ignored on them.

This PR makes using the first two attributes an error, because if they are correctly expanded will affect the AST and change code behavior.

I'm beta-nominating this, because generic parameter attributes are stabilizing in 1.27, and if we did not reserve their usage, we may never be able to repurpose the meaning of these attributes in the Rust 2015 edition.

@kennytm kennytm added beta-nominated Nominated for backporting to the compiler in the beta channel. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jun 1, 2018
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Jun 1, 2018
@Centril
Copy link
Contributor

Centril commented Jun 1, 2018

💯 👍

Nominating this for the next lang meeting if it hasn't been resolved until then :)

@TimNN
Copy link
Contributor

TimNN commented Jun 5, 2018

Ping from triage, @petrochenkov. This PR is waiting for your review.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 7, 2018

📌 Commit c9cb806 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 Jun 7, 2018
@nikomatsakis nikomatsakis self-assigned this Jun 7, 2018
@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 7, 2018
@pietroalbini
Copy link
Member

@bors p=1 (needs to be backported into beta)

@nikomatsakis
Copy link
Contributor

This seems like a no brainer. Approving also for beta backport.

@bors
Copy link
Contributor

bors commented Jun 7, 2018

⌛ Testing commit c9cb806 with merge d08d9e1a1f4be50a2000dea5f998ae613c690150...

@bors
Copy link
Contributor

bors commented Jun 7, 2018

💔 Test failed - status-travis

@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. 
Attempting to download s3://rust-lang-ci-sccache2/docker/570b1eebd40957db13b865983fa3a1da70d89ccfc44bf30bd0a3ac2b1f43a812fa3a57d2f957e731fd2ed84fe9ccfd4f4971319584c1c14ff83fda5559506e08
[00:01:26] Attempting with retry: curl -f -L -C - -o /tmp/rustci_docker_cache https://s3-us-west-1.amazonaws.com/rust-lang-ci-sccache2/docker/570b1eebd40957db13b865983fa3a1da70d89ccfc44bf30bd0a3ac2b1f43a812fa3a57d2f957e731fd2ed84fe9ccfd4f4971319584c1c14ff83fda5559506e08
[00:01:26]   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
[00:01:26]                                  Dload  Upload   Total   Spent    Left  Speed
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

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 bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 7, 2018
@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. 
Attempting to download s3://rust-lang-ci-sccache2/docker/570b1eebd40957db13b865983fa3a1da70d89ccfc44bf30bd0a3ac2b1f43a812fa3a57d2f957e731fd2ed84fe9ccfd4f4971319584c1c14ff83fda5559506e08
[00:01:26] Attempting with retry: curl -f -L -C - -o /tmp/rustci_docker_cache https://s3-us-west-1.amazonaws.com/rust-lang-ci-sccache2/docker/570b1eebd40957db13b865983fa3a1da70d89ccfc44bf30bd0a3ac2b1f43a812fa3a57d2f957e731fd2ed84fe9ccfd4f4971319584c1c14ff83fda5559506e08
[00:01:26]   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
[00:01:26]                                  Dload  Upload   Total   Spent    Left  Speed
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

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)

@kennytm
Copy link
Member Author

kennytm commented Jun 7, 2018

@bors retry #40474

@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 Jun 7, 2018
@pietroalbini pietroalbini mentioned this pull request Jun 7, 2018
Open
@bors
Copy link
Contributor

bors commented Jun 7, 2018

⌛ Testing commit c9cb806 with merge 56e5871eac206e05cf0d3db3799a0ffb994042f5...

@bors
Copy link
Contributor

bors commented Jun 7, 2018

💔 Test failed - status-travis

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

@bors retry (the tree was closed, PR killed by cancelbot)

@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 Jun 7, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jun 7, 2018
@kennytm
Rollup merge of rust-lang#51283 - kennytm:fix-51279-preempt-the-warni…
26a9d58 
…ng-song-and-dance, r=nikomatsakis

Deny #[cfg] and #[cfg_attr] on generic parameters.

Fix rust-lang#51279.

Attributes on generic parameters are not expanded, meaning `#[cfg]`, `#[cfg_attr]` and attribute proc macros are entirely ignored on them.

This PR makes using the first two attributes an error, because if they are correctly expanded will affect the AST and change code behavior.

I'm beta-nominating this, because generic parameter attributes are stabilizing in 1.27, and if we did not reserve their usage, we may never be able to repurpose the meaning of these attributes in the Rust 2015 edition.
@kennytm kennytm mentioned this pull request Jun 7, 2018
Merged
bors added a commit that referenced this pull request Jun 7, 2018
@bors
Auto merge of #51426 - kennytm:rollup, r=kennytm
1b4c921 
Rollup of 9 pull requests

Successful merges:

 - #51186 (Remove two redundant .nll.stderr files)
 - #51283 (Deny #[cfg] and #[cfg_attr] on generic parameters.)
 - #51368 (Fix the use of closures within #[panic_implementation])
 - #51380 (Remove dependency on fmt_macros from typeck)
 - #51389 (rustdoc: Fix missing stability and src links for inlined external macros)
 - #51399 (NLL performance boost)
 - #51407 (Update RLS and Rustfmt)
 - #51417 (Revert #49719)
 - #51420 (Tries to address the recent network issues)

Failed merges:
@bors bors merged commit c9cb806 into rust-lang:master Jun 8, 2018
@kennytm kennytm deleted the fix-51279-preempt-the-warning-song-and-dance branch June 8, 2018 02:07
@pietroalbini pietroalbini mentioned this pull request Jun 8, 2018
Merged
@kennytm kennytm removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 9, 2018
bors added a commit that referenced this pull request Jun 9, 2018
@bors
Auto merge of #51432 - pietroalbini:beta-backports, r=kennytm
c859c4f 
[beta] Rollup backports

Merged and accepted:

* #51417: Revert #49719
* #51283: Deny #[cfg] and #[cfg_attr] on generic parameters
* #51328: Do not promote union field accesses

r? @ghost
@petrochenkov
Copy link
Contributor

petrochenkov commented Jun 9, 2018 •
edited
Loading

Attributes on generic parameters are not expanded, meaning #[cfg], #[cfg_attr] and attribute proc macros are entirely ignored on them.

Sigh, I thought I asked @abonander to test at least some of this stuff in one of his recent macro expansion PRs to make sure it produces an error.

@abonander
Copy link
Contributor

@petrochenkov only #[derive]: /~https://github.com/rust-lang/rust/pull/50092/files#diff-ab68335be7737b8cc3277e2087d93f88R16

@petrochenkov petrochenkov mentioned this pull request Jun 5, 2019
Merged
Centril added a commit to Centril/rust that referenced this pull request Jun 19, 2019
@Centril
Rollup merge of rust-lang#61547 - petrochenkov:cfgen, r=Centril
6820f2e 
Support `cfg` and `cfg_attr` on generic parameters

`cfg` attributes are supported in all other positions where attributes are accepted at all.

They were previously prohibited in rust-lang#51283 because they weren't implemented correctly before that and were simply ignored.
Centril added a commit to Centril/rust that referenced this pull request Jun 19, 2019
@Centril
Rollup merge of rust-lang#61547 - petrochenkov:cfgen, r=Centril
7b3984b 
Support `cfg` and `cfg_attr` on generic parameters

`cfg` attributes are supported in all other positions where attributes are accepted at all.

They were previously prohibited in rust-lang#51283 because they weren't implemented correctly before that and were simply ignored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@kennytm @rust-highfive @Centril @TimNN @nikomatsakis @bors @pietroalbini @petrochenkov @abonander