Remove #[main] attribute and add #[rustc_main] attribute. #84062

crlf0710 · 2021-04-10T16:04:16Z

This removes the #[main] attribute support from the compiler according to the decisions within #29634. For existing use cases within test harness generation, replaced it with a newly-introduced resolution based crate-level attribute #[rustc_main], which will later be used to implement RFC 1260.

Future vision: If we agree to migrate and merge the functionality of #[start] attribute into this #[rustc_main] attribute and make it also resolution-based like RFC 1260, the entry pass within rustc_passes crate will be able to be removed entirely, and whole entry function determination mechanism can happen at the ast lowering stage.

Closes #29634.
cc #28937
cc #84060

r? @petrochenkov cc @nikomatsakis

rust-highfive · 2021-04-10T16:04:19Z

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

compiler/rustc_codegen_cranelift/src/driver/jit.rs

compiler/rustc_error_codes/src/error_codes/E0137.md

petrochenkov · 2021-04-11T11:48:02Z

compiler/rustc_feature/src/builtin_attrs.rs

@@ -421,6 +421,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
        "the `#[profiler_runtime]` attribute is used to identify the `profiler_builtins` crate \
        which contains the profiler runtime and will never be stable",
    ),
+    gated!(rustc_main, AssumedUsed, template!(List: "path"), experimental!(rustc_main)),


If this attribute is not going to be stabilized, then it doesn't need a separate feature and a tracking issue.
It can use feature(rustc_attrs) instead, see the attributes defined with rustc_attr! below.

petrochenkov · 2021-04-11T11:52:29Z

src/test/ui/feature-gates/feature-gate-rustc-attrs.rs

@@ -1,5 +1,4 @@
 // Test that `#[rustc_*]` attributes are gated by `rustc_attrs` feature gate.
-


Nit: unnecessary change causing an stderr diff.

petrochenkov · 2021-04-11T11:59:54Z

@crlf0710
So the main question is - why is #[rustc_main] not just a renamed copy of #[main]?
Why do you need to invent a new logic with resolving a path in #[rustc_main(path)]?
It doesn't bring us any closer to implementing RFC 1260, it's an orthogonal feature.

petrochenkov · 2021-04-11T12:15:08Z

This PR does too many orthogonal things in general, and it's hard to track the motivation.
For example - why is the code moved around so much? Is it required for implementing the new logic, or is it just a refactoring?

RFC 1260 requires resolving crate::main, which is better done during late resolution (then the result can be passed further via ResolverOutputs), or during AST lowering (then the result can be stored in HIR).
This may require moving code detecting the entry point to an earlier or later point in compilation.
(UPD: When for loop desugaring and friends were migrated to lang items, path resolution methods were removed from ResolverAstLowering, so late resolution is a better candidate, it should happen somewhere around fn resolve_crate.)

In this PR the code is rearranged to switch from #[main] (#[rustc_main] after renaming) to path resolution in #[rustc_main(path)]? This is a pretty arbitrary change, I don't think it's something that needs to be done at this point if your goals are to implement RFC 1260, or remove #[main] and close its tracking issue. (These two goals are also independent and are better implemented as two separate PRs.)

crlf0710 · 2021-04-11T17:52:27Z

@petrochenkov Thank you for your review.
I'll try to sketch out what was in my mind, then let's discuss and make a better plan here.

Task A: Removing #[main]. Removing itself is quite easy, however the harder part is fixing the test harness generator.
The test harness generator currently needs to:

Suppress a potential existing main function be recognized as a main function.
Generate a new main function which may or may not be literally called main according to compilation options.
Avoid the function in 1 be recognized as dead code.

This PR in its current form resolves all these issues by:
a. Using this new rustc_main attribute to satisfy 1 & 2.
b. Abusing this new rustc_main attribute with a special form to satisfy 3.

Task B: Implement RFC 1260: If 1 & 2 is still satisfied, i have to write all these generated data as path to be resolved.
This leads to me wanting to use an attribute to kept all these data passing around.

I wonder if you could give me some implementation advice on this... Maybe by setting up a middle ground so i can split this pr into two.

reason i didn't re-use the existing sym::main: that would be user-facing and maybe requires a t-lang mcp or something... i want to make this change totally in the scope of t-compiler.

For the resolution implementation, i just didn't know much about late resolution. I'll try to work it out if that's a better candidate.

crlf0710 · 2021-04-17T17:09:05Z

I guess the next steps here will be giving resolution of crate::main during late resolution stage a try. Sadly i will have to be mostly afk during this Sunday. Will see if i can pick it up in the evening (probably not).

Any mentoring notes are welcome!

crlf0710 · 2021-04-19T16:02:30Z

status update: skimmed through rustc_resolve/src/late.rs today. It seems i can add a visit_crate method there, and make it call smart_resolve_path method. Not sure where it would store the resolve result though. Maybe it will also associate the resolve result with a NodeId? Then i'll have to allocate a brand-new NodeId there too.

petrochenkov · 2021-04-19T21:22:35Z

make it call smart_resolve_path method

resolve_path probably, because smart_resolve_path is going to report errors in case of unsuccessful resolution.
resolve_ast_path_in_value_ns in this PR basically does everything right.

Not sure where it would store the resolve result though

First in Resolver, then in ResolverOutputs, then it later ends up in tcx.

After some thought the alternative with adding fn resolve_main to ResolverAstLowering (and then storing the result in HIR) is also ok.
The difference is that the entry point query will has to use an eval_always query somewhere if it gets the result from ResolverOutputs because it's untracked, but HIR is fully tracked (if I understand it correctly), so storing it in HIR will track dependencies automatically.

petrochenkov · 2021-04-19T21:25:38Z

Maybe it will also associate the resolve result with a NodeId? Then i'll have to allocate a brand-new NodeId there too.

Why? The result of resolving crate::main is a DefId (or Res depending on where you want to filter away crate::mains resolving to non-functions), which can be stored until the typeck where it becomes necessary for the entry point query. There's no need in any NodeIds here, especially new NodeIds.

crlf0710 · 2021-04-21T10:33:05Z

status update: i've implemented the changes we discussed above. But it seems it's necessary to avoid the use item establishing main re-export be recognized as unused import. Will give it another try soon.

petrochenkov · 2021-04-21T12:01:26Z

The easiest way to do that is to call resolve_path for the second time with record_used= true.
Alternatively, resolve_ident_in_lexical_scope can be called from the root module for resolving main and record_use can be called for marking the resulting NameBinding as used.

Also the resolution should happen before check_unused is called.

crlf0710 · 2021-04-21T15:50:46Z

Second part extracted to #84401 .

Urgau · 2021-04-22T09:56:07Z

Will this "solve" #62127 ? Where a proc-macro named main is in conflict with the [main] attribute.

crlf0710 · 2021-04-22T17:16:18Z

Will this "solve" #62127 ? Where a proc-macro named main is in conflict with the [main] attribute.

I think so, that change has landed in #84217 , you can try it out on nightly.

…enkov Implement RFC 1260 with feature_name `imported_main`. This is the second extraction part of rust-lang#84062 plus additional adjustments. This (mostly) implements RFC 1260. However there's still one test case failure in the extern crate case. Maybe `LocalDefId` doesn't work here? I'm not sure. cc rust-lang#28937 r? `@petrochenkov`

crlf0710 · 2021-04-30T14:03:51Z

The second part has landed in #84401.
The third part is extracted to #84507

There's nothing left here, so let me close this.

…enkov Implement RFC 1260 with feature_name `imported_main`. This is the second extraction part of rust-lang#84062 plus additional adjustments. This (mostly) implements RFC 1260. However there's still one test case failure in the extern crate case. Maybe `LocalDefId` doesn't work here? I'm not sure. cc rust-lang#28937 r? `@petrochenkov`

…per, r=nagisa Add primary marker on codegen unit and generate main wrapper on primary codegen. This is the codegen part of changes extracted from rust-lang#84062. This add a marker called `primary` on each codegen units, where exactly one codegen unit will be `primary = true` at a time. This specific codegen unit will take charge of generating `main` wrapper when `main` is imported from a foreign crate after the implementation of RFC 1260. cc rust-lang#28937 I'm not sure who should i ask for review for codegen changes, so feel free to reassign. r? `@nagisa`

…r, r=nagisa Add primary marker on codegen unit and generate main wrapper on primary codegen. This is the codegen part of changes extracted from rust-lang#84062. This add a marker called `primary` on each codegen units, where exactly one codegen unit will be `primary = true` at a time. This specific codegen unit will take charge of generating `main` wrapper when `main` is imported from a foreign crate after the implementation of RFC 1260. cc rust-lang#28937 I'm not sure who should i ask for review for codegen changes, so feel free to reassign. r? `@nagisa`

…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`.

rust-highfive assigned petrochenkov Apr 10, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 10, 2021

crlf0710 force-pushed the remove_main_attribute branch from c4eea4d to 9d2aa48 Compare April 10, 2021 16:06