Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Complete removal of #[main] attribute from compiler #93753

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

jeremyBanks
Copy link
Contributor

@jeremyBanks jeremyBanks commented Feb 8, 2022

resolves #93786


The #[main] attribute was mostly removed from the language in #84217, but not completely. It is still recognized as a builtin attribute by the compiler, but it has no effect. However, this no-op attribute is no longer gated by #[feature(main)] (which no longer exists), so it's possible to include it in code on stable without any errors, which seems unintentional. For example, the following code is accepted (playground link).

#[main]
fn main() {
    println!("hello world");
}

Aside from that oddity, the existence of this attribute causes code like the following to fail (playground link). According #84062 (comment), the removal of #[main] was expected to eliminate this conflict (previously reported as #62127).

use tokio::main;

#[main]
fn main() {
    println!("hello world");
}
error[E0659]: `main` is ambiguous
 --> src/main.rs:3:3
  |
3 | #[main]
  |   ^^^^ ambiguous name
  |
  = note: ambiguous because of a name conflict with a builtin attribute
  = note: `main` could refer to a built-in attribute

This error message can be confusing, as the mostly-removed #[main] attribute is not mentioned in any documentation.

Since the current availability of #[main] on stable seems unintentional, and to needlessly block use of the main identifier in the attribute namespace, this PR finishes removing the #[main] attribute as described in #29634 (comment) by deleting it from builtin_attrs.rs, and adds two test cases to ensure that the attribute is no longer accepted and no longer conflicts with other attributes imported as main.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 8, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2022
@Mark-Simulacrum
Copy link
Member

Cc @petrochenkov

@petrochenkov
Copy link
Contributor

Looks like I just missed that main wasn't removed from the list of built-in attributes in #84217.
I'm going to mark this as beta nominated since it's a stable-to-stable regression.

@petrochenkov petrochenkov added beta-nominated Nominated for backporting to the compiler in the beta channel. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Feb 8, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 8, 2022
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 8, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Feb 8, 2022

If I understand correctly, the main effect of this PR is to make code start erroring that is using the previously-built-in no-op #[main]. Any such code must inherently have that attribute by accident (since the attribute cannot be serving a purpose for any code that is using it).

I don't think I would beta backport this without at least a crater run, since we don't know how many crates out there might be using #[main] by accident.

It also looks like something that would be nearly trivial to put through a future-incompatibility warning cycle (which I know is essentially the opposite direction from a beta backport, but the nature of this problem seems like it deserves that treatment: i.e. its not an urgent matter to fix this).

The only argument I can see against a warning cycle is that its just extra tech debt hanging around in the compiler. Versus this PR, which immediately eliminates that tech debt. So this is sort of a microcosm of an ongoing prioritization question: should we prioritize the needs of an unknown but likely small set of Rust users over the needs of rustc contributors...

@jeremyBanks jeremyBanks changed the title Remove obsolete no-op #[main] attribute from compiler Finishing removing #[main] attribute from compiler Feb 8, 2022
@jeremyBanks jeremyBanks changed the title Finishing removing #[main] attribute from compiler Complete removal of #[main] attribute from compiler Feb 8, 2022
@petrochenkov
Copy link
Contributor

I suggest backporting it sooner so it gets into the beta crater run, and reverting later if there are any regressions in non-nightly crates. If there's no time for that, then land it on nightly and wait for the next beta crater run.
What I suggest against is overthinking and spending more time on this.

@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 9, 2022

📌 Commit 475e4ee has been approved by petrochenkov

@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 Feb 9, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 9, 2022
…henkov

Complete removal of #[main] attribute from compiler

resolves rust-lang#93786

---

The `#[main]` attribute was mostly removed from the language in rust-lang#84217, but not completely. It is still recognized as a builtin attribute by the compiler, but it has no effect. However, this no-op attribute is no longer gated by `#[feature(main)]` (which no longer exists), so it's possible to include it in code *on stable* without any errors, which seems unintentional. For example, the following code is accepted ([playground link](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&code=%23%5Bmain%5D%0Afn%20main()%20%7B%0A%20%20%20%20println!(%22hello%20world%22)%3B%0A%7D%0A)).

```rust
#[main]
fn main() {
    println!("hello world");
}
```

Aside from that oddity, the existence of this attribute causes code like the following to fail ([playground link](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&code=use%20tokio%3A%3Amain%3B%0A%0A%23%5Bmain%5D%0Afn%20main()%20%7B%0A%20%20%20%20println!(%22hello%20world%22)%3B%0A%7D%0A)). According rust-lang#84062 (comment), the removal of `#[main]` was expected to eliminate this conflict (previously reported as rust-lang#62127).

```rust
use tokio::main;

#[main]
fn main() {
    println!("hello world");
}
```

```
error[E0659]: `main` is ambiguous
 --> src/main.rs:3:3
  |
3 | #[main]
  |   ^^^^ ambiguous name
  |
  = note: ambiguous because of a name conflict with a builtin attribute
  = note: `main` could refer to a built-in attribute
```

[This error message can be confusing](https://stackoverflow.com/q/71024443/1114), as the mostly-removed `#[main]` attribute is not mentioned in any documentation.

Since the current availability of `#[main]` on stable seems unintentional, and to needlessly block use of the `main` identifier in the attribute namespace, this PR finishes removing the `#[main]` attribute as described in rust-lang#29634 (comment) by deleting it from `builtin_attrs.rs`, and adds two test cases to ensure that the attribute is no longer accepted and no longer conflicts with other attributes imported as `main`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 10, 2022
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#91443 (Better suggestions when user tries to collect into an unsized `[_]`)
 - rust-lang#91504 (`#[used(linker)]` attribute)
 - rust-lang#93503 (debuginfo: Fix DW_AT_containing_type vtable debuginfo regression)
 - rust-lang#93753 (Complete removal of #[main] attribute from compiler)
 - rust-lang#93799 (Fix typo in `std::fmt` docs)
 - rust-lang#93813 (Make a few cleanup MIR passes public)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 84c2804 into rust-lang:master Feb 10, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 10, 2022
@jeremyBanks jeremyBanks deleted the main-conflict branch February 10, 2022 05:10
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 10, 2022
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.60.0, 1.59.0 Feb 11, 2022
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 11, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2022
…ulacrum

[beta] backports

This backports:

*  Complete removal of #[main] attribute from compiler rust-lang#93753
*  Resolve lifetimes for const generic defaults rust-lang#93669
*  backport llvm fix for issue 91671. rust-lang#93426
*  Fix invalid special casing of the unreachable! macro rust-lang#93179
*  Fix hashing for windows paths containing a CurDir component rust-lang#93697

r? `@Mark-Simulacrum`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[main] attribute was unintentionally stabilized (as no-op) in 1.53.0
8 participants