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

#[link_args] was accidentally stabilized; were other attributes as well? #43106

Closed
pnkfelix opened this issue Jul 7, 2017 · 4 comments
Closed

#[link_args] was accidentally stabilized; were other attributes as well? #43106

pnkfelix opened this issue Jul 7, 2017 · 4 comments
Labels
A-attributes Area: #[attributes(..)] A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Jul 7, 2017

A comment in the tracking issue for #[link_args] points out that the attribute was accidentally stabilized (quite a long time ago).

I think it will be easy to fix the particular case of #[link_args]; the infrastructure here is pretty nice. But it does raise the question: Were other attributes accidentally stabilized at the same time?

This issue has two goals: 1. correct the #[link_args] feature gate, and 2. double-check whether other attributes in the BUILTIN_ATTRIBUTES array need similar treatment.
@retep998
Copy link
Member

retep998 commented Jul 7, 2017

In particular, try to check by using the attribute in places where it was not intended to be used. As an example #[link_args] is still unstable when used on an extern block (which is even tested), but if you do a crate level #![link_args] then it is stable.

@pnkfelix pnkfelix mentioned this issue Jul 7, 2017
Merged
@pnkfelix
Copy link
Member Author

pnkfelix commented Jul 7, 2017 •
edited
Loading

(So PR #43109 is intended to address item (1.) in the description, but I want to keep this issue open until we have double-checked (2.) and potentially added corresponding unit tests.)

@pnkfelix
Copy link
Member Author

Another detail that I had not digested before about this bug: We are even emitting a lint claiming that the link_args attribute is unused when it appears in these other contexts that elude the current feature gate.

This led me to think that this bug was not so bad, since I inferred from the emission of the unused_attribute warning that the directive to link_args was being ignored in those other contexts.

However, as @retep998 states, the link_args directive is included in the linker invocation when link_args occurs elsewhere, despite the compiler claiming that the occurrence of the attribute is unused.

So the compiler is not merely failing to stop us from putting in a useless occurrence of an unstable attribue in such cases; it is in fact actively lying regarding the effect of the occurrence of the attribute.

This was referenced Jul 10, 2017
Closed
Closed
bors added a commit that referenced this issue Jul 10, 2017
@bors
Auto merge of #43109 - pnkfelix:fix-link_args-gate, r=nikomatsakis
eb9dfb8 
Fix feature gate for `#[link_args(..)]` attribute

Fix feature gate for `#[link_args(..)]` attribute so that it will fire regardless of context of attribute.

See also #29596 and #43106
@pnkfelix pnkfelix mentioned this issue Jul 10, 2017
Closed
pnkfelix added a commit to pnkfelix/rust that referenced this issue Jul 11, 2017
@pnkfelix
Slew of feature gating tests for issue rust-lang#43106.
39b8aaf
@pnkfelix pnkfelix mentioned this issue Jul 11, 2017
Merged
bors added a commit that referenced this issue Jul 19, 2017
@bors
Auto merge of #43168 - pnkfelix:check-attr-gating, r=aturon
88e2c39 
Slew of builtin-attribute gating tests

Slew of builtin-attribute "gating" tests for issue #43106.

Some stray observations:

 * I don't know if its a good thing that so many attributes allow inputs which are silently discarded. (I  made heavy use of that in writing my tests, but that was more out of curiosity than necessity.)
 * The difference between crate-level and non-crate-level behavior is quite significant in some cases. Definitely worth making sure one has tests for both cases. (Not as clear whether it was worthwhile trying the various other AST forms like `fn f()` vs `struct S;`)
 * `#[no_builtins]` and `#[no_mangle]` occur twice on the `BUILTIN_ATTRIBUTES` list. Thats almost certainly a bug. (Filed as #43148)
 * We are maximally liberal in what we allow for `#[test]` and `#[bench]` when one compiles without `--test`.
 * We allow `#[no_mangle]` on arbitrary AST nodes, but only warn about potential misuse on `fn`
 * We allow `#[cold]`, `#[must_use]`, `#[windows_subsystem]`, and `#[no_builtins]` on arbitrary AST nodes. I don't know off-hand what the semantics are for e.g. a `#[cold] type T = ...;`
 * We allow crate-level `#![inline]`. That's probably a bug since its otherwise restricted to `fn` items
@Mark-Simulacrum Mark-Simulacrum added the A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. label Jul 19, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 28, 2017
@jonas-schievink jonas-schievink added A-attributes Area: #[attributes(..)] T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 7, 2020
@petrochenkov
Copy link
Contributor

#[link_args] is feature-gated now.

Regarding the "were other attributes as well?" part, the list of built-in attributes is currently in src\librustc_feature\builtin_attrs.rs for anyone to audit.
Looks like the audit was already done since 2017 because the accidentally stabilized attributes are either unstabilized or marked with FIXMEs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-attributes Area: #[attributes(..)] A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@pnkfelix @retep998 @jonas-schievink @Mark-Simulacrum @petrochenkov