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.

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

Give built-in macros stable addresses in the standard library #63056

Merged
merged 1 commit into from Aug 10, 2019

Conversation

@petrochenkov
Copy link
Contributor

commented Jul 28, 2019

Continuation of #62086.

Derive macros corresponding to traits from libcore are now available through the same paths as those traits:

  • Clone - {core,std}::clone::Clone
  • PartialEq - {core,std}::cmp::PartialEq
  • Eq - {core,std}::cmp::Eq
  • PartialOrd - {core,std}::cmp::PartialOrd
  • Ord - {core,std}::cmp::Ord
  • Default - {core,std}::default::Default
  • Debug - {core,std}::fmt::Debug
  • Hash - {core,std}::hash::Hash
  • Copy - {core,std}::marker::Copy

Fn-like built-in macros are now available through libcore and libstd's root module, by analogy with non-builtin macros defined by libcore and libstd:

{core,std}::{
    __rust_unstable_column,
    asm,
    assert,
    cfg,
    column,
    compile_error,
    concat,
    concat_idents,
    env,
    file,
    format_args,
    format_args_nl,
    global_asm,
    include,
    include_bytes,
    include_str,
    line,
    log_syntax,
    module_path,
    option_env,
    stringify,
    trace_macros,
}

Derive macros without a corresponding trait in libcore or libstd are still available only through prelude (also see #62507).
Attribute macros also keep being available only through prelude, mostly because they don't have an existing practice to follow. An advice from the library team on their eventual placement would be appreciated.

    RustcDecodable,
    RustcEncodable,
    bench,
    global_allocator,
    test,
    test_case,

r? @alexcrichton

@alexcrichton
Copy link
Member

left a comment

As for my opinions about the other attributes, I think in general this is a good PR and wouldn't want to bog it down too much with where to put the remaining macros. They're pretty inconsequential so I think it's fine to deal with them in follow-up PRs. That being said some thoughts I'd have are:

  • Encodable/Decodable - being removed soon in #62507 (yay!) so probably not too relevant
  • RustcEncodable/RustcDecodable - I think it's fine to relegate these to "only in the prelude" for now, there's basically zero new users of these macros so it's basically a game of "what's the minimum we can do to keep what's working still working"
  • global_allocator - my guess would be core::alloc as a good place for this.
  • test, test_case, bench - my hunch is that as we stabilize #[test_case] we're going to create a std::test module. In that world they'd naturally all fit into std::test. For now though I think leaving them in the prelude is fine.
Show resolved Hide resolved src/libcore/clone.rs Outdated
Show resolved Hide resolved src/libcore/lib.rs Outdated
Show resolved Hide resolved src/libcore/lib.rs Outdated
Show resolved Hide resolved src/libcore/prelude/v1.rs
Show resolved Hide resolved src/libstd/lib.rs Outdated
@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

I'm tweaking the tags since I'll run this through rfcbot once some of the review above has been addressed.

@petrochenkov petrochenkov force-pushed the petrochenkov:macstd2 branch from 764be5b to 881cdf8 Jul 29, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Updated.

  • Derive macros are moved into modules to avoid reexports.
  • pub macro is converted to #[macro_export] macro_rules (which is automatically placed into the root) for fn-like macros to avoid more reexports.
  • Other comments are addressed.
Show resolved Hide resolved src/libstd/lib.rs Outdated
@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

@rfcbot fcp merge

Ok this all looks great to me, thanks again @petrochenkov!

As a heads up to the @rust-lang/libs folks, this is a follow-up to #62086 where builtin macros now are earning stable locations throughout the standard library (e.g. libcore and libstd). The diff is pretty easy to read and the description is comprehensive as well. Want to get a sign off though since this will actually affect some stable surface area!

@rfcbot

This comment has been minimized.

Copy link

commented Jul 30, 2019

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

When defining a proc macro, the choice of using #[proc_macro] v.s. #[proc_macro_derive(…)] v.s. #[proc_macro_attribute] determines what kind of macro it is.

What is the equivalent of that for pub macro items? For example, what in this PR makes #[derive(Clone)] valid but not Clone!(…) ?

@rfcbot

This comment has been minimized.

Copy link

commented Jul 30, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2019

@SimonSapin
The compiler maintains a pre-defined map Map<Name, Signature> of built-in macros.
When the compiler sees a "lang item" macro (#[rustc_builtin_macro]) , it looks it signature up in that map. The macro kind (bang/attr/derive) is a part of that signature.

This machinery was implemented earlier in #62086 rather than in this PR.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

If the signature is built into the compiler, does that mean the $item:item part of pub macro Clone($item:item) { /* compiler built-in */ } is ignored? (Except perhaps by rustdoc?)

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2019

Yes, the $item:item is for rustdoc.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

Sounds good, thanks!

@Centril Centril added the relnotes label Jul 31, 2019

@Centril Centril added this to the 1.38 milestone Jul 31, 2019

@petrochenkov petrochenkov force-pushed the petrochenkov:macstd2 branch from 881cdf8 to 7880416 Jul 31, 2019

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

☔️ The latest upstream changes (presumably #62507) made this pull request unmergeable. Please resolve the merge conflicts.

@rfcbot

This comment has been minimized.

Copy link

commented Aug 9, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

r=me with a rebase

@petrochenkov petrochenkov force-pushed the petrochenkov:macstd2 branch from 7880416 to cbcc7dd Aug 9, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

@bors r=alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

📌 Commit cbcc7dd has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2019

⌛️ Testing commit cbcc7dd with merge a08d919...

bors added a commit that referenced this pull request Aug 10, 2019

Auto merge of #63056 - petrochenkov:macstd2, r=alexcrichton
Give built-in macros stable addresses in the standard library

Continuation of #62086.

Derive macros corresponding to traits from libcore are now available through the same paths as those traits:
- `Clone` - `{core,std}::clone::Clone`
- `PartialEq` - `{core,std}::cmp::PartialEq`
- `Eq` - `{core,std}::cmp::Eq`
- `PartialOrd` - `{core,std}::cmp::PartialOrd`
- `Ord` - `{core,std}::cmp::Ord`
- `Default` - `{core,std}::default::Default`
- `Debug` - `{core,std}::fmt::Debug`
- `Hash` - `{core,std}:#️⃣:Hash`
- `Copy` - `{core,std}::marker::Copy`

Fn-like built-in macros are now available through libcore and libstd's root module, by analogy with non-builtin macros defined by libcore and libstd:
```rust
{core,std}::{
    __rust_unstable_column,
    asm,
    assert,
    cfg,
    column,
    compile_error,
    concat,
    concat_idents,
    env,
    file,
    format_args,
    format_args_nl,
    global_asm,
    include,
    include_bytes,
    include_str,
    line,
    log_syntax,
    module_path,
    option_env,
    stringify,
    trace_macros,
}
```

Derive macros without a corresponding trait in libcore or libstd are still available only through prelude (also see #62507).
Attribute macros also keep being available only through prelude, mostly because they don't have an existing practice to follow. An advice from the library team on their eventual placement would be appreciated.
```rust
    RustcDecodable,
    RustcEncodable,
    bench,
    global_allocator,
    test,
    test_case,
```

r? @alexcrichton

Centril added a commit to Centril/rust that referenced this pull request Aug 10, 2019

Rollup merge of rust-lang#63056 - petrochenkov:macstd2, r=alexcrichton
Give built-in macros stable addresses in the standard library

Continuation of rust-lang#62086.

Derive macros corresponding to traits from libcore are now available through the same paths as those traits:
- `Clone` - `{core,std}::clone::Clone`
- `PartialEq` - `{core,std}::cmp::PartialEq`
- `Eq` - `{core,std}::cmp::Eq`
- `PartialOrd` - `{core,std}::cmp::PartialOrd`
- `Ord` - `{core,std}::cmp::Ord`
- `Default` - `{core,std}::default::Default`
- `Debug` - `{core,std}::fmt::Debug`
- `Hash` - `{core,std}:#️⃣:Hash`
- `Copy` - `{core,std}::marker::Copy`

Fn-like built-in macros are now available through libcore and libstd's root module, by analogy with non-builtin macros defined by libcore and libstd:
```rust
{core,std}::{
    __rust_unstable_column,
    asm,
    assert,
    cfg,
    column,
    compile_error,
    concat,
    concat_idents,
    env,
    file,
    format_args,
    format_args_nl,
    global_asm,
    include,
    include_bytes,
    include_str,
    line,
    log_syntax,
    module_path,
    option_env,
    stringify,
    trace_macros,
}
```

Derive macros without a corresponding trait in libcore or libstd are still available only through prelude (also see rust-lang#62507).
Attribute macros also keep being available only through prelude, mostly because they don't have an existing practice to follow. An advice from the library team on their eventual placement would be appreciated.
```rust
    RustcDecodable,
    RustcEncodable,
    bench,
    global_allocator,
    test,
    test_case,
```

r? @alexcrichton
@Centril

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

@bors retry rolled up.

bors added a commit that referenced this pull request Aug 10, 2019

Auto merge of #63427 - Centril:rollup-6db5f5g, r=Centril
Rollup of 8 pull requests

Successful merges:

 - #63056 (Give built-in macros stable addresses in the standard library)
 - #63149 (resolve: Populate external modules in more automatic and lazy way)
 - #63337 (Tweak mismatched types error)
 - #63350 (Use associated_type_bounds where applicable - closes #61738)
 - #63394 (Add test for issue 36804)
 - #63399 (More explicit diagnostic when using a `vec![]` in a pattern)
 - #63419 (check against more collisions for TypeId of fn pointer)
 - #63423 (Mention that tuple structs are private if any of their fields are)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Aug 10, 2019

Rollup merge of rust-lang#63056 - petrochenkov:macstd2, r=alexcrichton
Give built-in macros stable addresses in the standard library

Continuation of rust-lang#62086.

Derive macros corresponding to traits from libcore are now available through the same paths as those traits:
- `Clone` - `{core,std}::clone::Clone`
- `PartialEq` - `{core,std}::cmp::PartialEq`
- `Eq` - `{core,std}::cmp::Eq`
- `PartialOrd` - `{core,std}::cmp::PartialOrd`
- `Ord` - `{core,std}::cmp::Ord`
- `Default` - `{core,std}::default::Default`
- `Debug` - `{core,std}::fmt::Debug`
- `Hash` - `{core,std}:#️⃣:Hash`
- `Copy` - `{core,std}::marker::Copy`

Fn-like built-in macros are now available through libcore and libstd's root module, by analogy with non-builtin macros defined by libcore and libstd:
```rust
{core,std}::{
    __rust_unstable_column,
    asm,
    assert,
    cfg,
    column,
    compile_error,
    concat,
    concat_idents,
    env,
    file,
    format_args,
    format_args_nl,
    global_asm,
    include,
    include_bytes,
    include_str,
    line,
    log_syntax,
    module_path,
    option_env,
    stringify,
    trace_macros,
}
```

Derive macros without a corresponding trait in libcore or libstd are still available only through prelude (also see rust-lang#62507).
Attribute macros also keep being available only through prelude, mostly because they don't have an existing practice to follow. An advice from the library team on their eventual placement would be appreciated.
```rust
    RustcDecodable,
    RustcEncodable,
    bench,
    global_allocator,
    test,
    test_case,
```

r? @alexcrichton

bors added a commit that referenced this pull request Aug 10, 2019

Auto merge of #63428 - Centril:rollup-c2ru1z1, r=Centril
Rollup of 7 pull requests

Successful merges:

 - #63056 (Give built-in macros stable addresses in the standard library)
 - #63337 (Tweak mismatched types error)
 - #63350 (Use associated_type_bounds where applicable - closes #61738)
 - #63394 (Add test for issue 36804)
 - #63399 (More explicit diagnostic when using a `vec![]` in a pattern)
 - #63419 (check against more collisions for TypeId of fn pointer)
 - #63423 (Mention that tuple structs are private if any of their fields are)

Failed merges:

r? @ghost
@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2019

⌛️ Testing commit cbcc7dd with merge d19a359...

@bors bors merged commit cbcc7dd into rust-lang:master Aug 10, 2019

3 of 5 checks passed

pr
Details
homu Testing commit cbcc7dd182b9bf67d664508b82284c5539ef8819 with merge d19a359444295bab01de7ff44a9d72302e573bc9...
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
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.