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

Properly handle attributes on statements #78296

Merged
merged 7 commits into from
Nov 28, 2020

Conversation

Aaron1011
Copy link
Member

@Aaron1011 Aaron1011 commented Oct 23, 2020

We now collect tokens for the underlying node wrapped by StmtKind
nstead of storing tokens directly in Stmt.

LazyTokenStream now supports capturing a trailing semicolon after it
is initially constructed. This allows us to avoid refactoring statement
parsing to wrap the parsing of the semicolon in parse_tokens.

Attributes on item statements
(e.g. fn foo() { #[bar] struct MyStruct; }) are now treated as
item attributes, not statement attributes, which is consistent with how
we handle attributes on other kinds of statements. The feature-gating
code is adjusted so that proc-macro attributes are still allowed on item
statements on stable.

Two built-in macros (#[global_allocator] and #[test]) needed to be
adjusted to support being passed Annotatable::Stmt.

@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 Oct 23, 2020
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Oct 23, 2020

⌛ Trying commit e280124325b5aab5a6723557959dcb734d7bbc01 with merge 548f7f9bf4b5f0cdafe133e9709c0720bdcf303a...

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020
@Aaron1011
Copy link
Member Author

Before this is merged, we'll probably want to reach a decision about whether or not #[allow(doc_comments)] applies to doc comments on the same item: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Doc.20comments.20on.20nested.20items

@Aaron1011
Copy link
Member Author

Blocked on #78306

@Aaron1011 Aaron1011 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2020
@bors

This comment has been minimized.

@Aaron1011
Copy link
Member Author

Now blocked on making progress on #61733 (comment).

@Aaron1011 Aaron1011 changed the title [WIP] Properly handle custom attributes on statements Properly handle attributes on statements Nov 17, 2020
@Aaron1011
Copy link
Member Author

Any changes regarding #61733 (comment) (e.g. #78991 and #78685) will need to go through a warning period (if not an edition), since they will cause a significant amount of ecosystem breakage. Therefore, any code to support trailing macro statement expressions (or trailing-semicolon `macro_rules~ macros) will need to continue to exist for quite some time.

Fortunately, I think this PR doesn't actually require either of those issues to be resolved.

@petrochenkov: This is now ready for review

@Aaron1011 Aaron1011 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Nov 18, 2020
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Nov 18, 2020

⌛ Trying commit ff440127e52e1c9385ea6c81c8c87b93b79de079 with merge 3d7a430081377f37233ad25b70ec193884238670...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Nov 27, 2020

📌 Commit 772292f 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 Nov 27, 2020
@bors
Copy link
Contributor

bors commented Nov 27, 2020

⌛ Testing commit 772292f with merge 5d945e4ab94196c12a449771f3fa800840d0976d...

@bors
Copy link
Contributor

bors commented Nov 27, 2020

💔 Test failed - checks-actions

@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 Nov 27, 2020
@petrochenkov
Copy link
Contributor

 Documenting rustc_ast_passes v0.0.0 (/checkout/compiler/rustc_ast_passes)
error[E0275]: overflow evaluating the requirement `rustc_ast::Local: std::marker::Sync`
  |
  = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`rustc_ast_passes`)

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2020
When cfg(parallel_compiler) is enabled, we end up trying to prove
Send/Sync bounds for some deeply nested types (at least when rustdoc is
run).
@Aaron1011
Copy link
Member Author

Bumping the recursion limit in that crate was enough to get rustdoc to succeed with cfg(parallel_compiler) enabled.

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Nov 27, 2020

📌 Commit 92bfa05 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 27, 2020
@bors
Copy link
Contributor

bors commented Nov 28, 2020

⌛ Testing commit 92bfa05 with merge 4ae328b...

@bors
Copy link
Contributor

bors commented Nov 28, 2020

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 4ae328b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 28, 2020
@bors bors merged commit 4ae328b into rust-lang:master Nov 28, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 28, 2020
bors added a commit to rust-lang/cargo that referenced this pull request Nov 29, 2020
Remove unnecessary trailing semicolons

PR rust-lang/rust#78296 will cause rustc to
start warning on semicolons following item statements.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 30, 2020
…k, r=petrochenkov

Replace pretty-print/compare/retokenize hack with targeted workarounds

Based on rust-lang#78296
cc rust-lang#43081

The 'pretty-print/compare/retokenize' hack is used to try to avoid passing an outdated `TokenStream` to a proc-macro when the underlying AST is modified in some way (e.g. cfg-stripping before derives). Unfortunately, retokenizing throws away spans (including hygiene information), which causes issues of its own. Every improvement to the accuracy of the pretty-print/retokenize comparison has resulted in non-trivial ecosystem breakage due to hygiene changes. In extreme cases, users deliberately wrote unhygienic `macro_rules!` macros (likely because they did not realize that the compiler's behavior was a bug).

Additionaly, the comparison between the original and pretty-printed/retoknized token streams comes at a non-trivial runtime cost, as shown by rust-lang#79338

This PR removes the pretty-print/compare/retokenize logic from `nt_to_tokenstream`. We only discard the original `TokenStream` under two circumstances:
* Inner attributes are used (detected by examining the AST)
* `cfg`/`cfg_attr` processing modifies the AST. This is detected by making the visitor update a flag when it performs a modification, instead of trying to detect the modification after-the-fact. Note that a 'matching' `cfg` (e.g. `#[cfg(not(FALSE)]`) does not actually get removed from the AST, allowing us to preserve the original `TokenStream`.

In all other cases, we preserve the original `TokenStream`.

This could use a bit of refactoring/renaming - opening for a Crater run.

r? `@ghost`
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 19, 2021
Pkgsrc changes:
 * Adjust patches, re-compute line offsets, fix capitalization.
 * Remove i686/FreeBSD support, no longer provided upstream.
 * Bump bootstraps to 1.49.0.
 * Change USE_TOOLS from bsdtar to gtar.
 * Reduce diffs to pkgsrc-wip package patches.
 * Allow rust.BUILD_TARGET to override automatic choice of target.
 * Add an i586/NetBSD (pentium) bootstrap variant (needs testing),
   not yet added as bootstrap since 1.49 doesn't have that variant.

Upstream changes:

Version 1.50.0 (2021-02-11)
============================

Language
-----------------------
- [You can now use `const` values for `x` in `[x; N]` array
  expressions.][79270] This has been technically possible since
  1.38.0, as it was unintentionally stabilized.
- [Assignments to `ManuallyDrop<T>` union fields are now considered
  safe.][78068]

Compiler
-----------------------
- [Added tier 3\* support for the `armv5te-unknown-linux-uclibceabi`
  target.][78142]
- [Added tier 3 support for the `aarch64-apple-ios-macabi` target.][77484]
- [The `x86_64-unknown-freebsd` is now built with the full toolset.][79484]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
-----------------------

- [`proc_macro::Punct` now implements `PartialEq<char>`.][78636]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
  arrays of any length.][74989]
- [On Unix platforms, the `std::fs::File` type now has a "niche"
  of `-1`.][74699] This value cannot be a valid file descriptor,
  and now means `Option<File>` takes up the same amount of space
  as `File`.

Stabilized APIs
---------------

- [`bool::then`]
- [`btree_map::Entry::or_insert_with_key`]
- [`f32::clamp`]
- [`f64::clamp`]
- [`hash_map::Entry::or_insert_with_key`]
- [`Ord::clamp`]
- [`RefCell::take`]
- [`slice::fill`]
- [`UnsafeCell::get_mut`]

The following previously stable methods are now `const`.

- [`IpAddr::is_ipv4`]
- [`IpAddr::is_ipv6`]
- [`Layout::size`]
- [`Layout::align`]
- [`Layout::from_size_align`]
- `pow` for all integer types.
- `checked_pow` for all integer types.
- `saturating_pow` for all integer types.
- `wrapping_pow` for all integer types.
- `next_power_of_two` for all unsigned integer types.
- `checked_power_of_two` for all unsigned integer types.

Cargo
-----------------------

- [Added the `[build.rustc-workspace-wrapper]` option.][cargo/8976]
  This option sets a wrapper to execute instead of `rustc`, for
  workspace members only.
- [`cargo:rerun-if-changed` will now, if provided a directory, scan the entire
  contents of that directory for changes.][cargo/8973]
- [Added the `--workspace` flag to the `cargo update` command.][cargo/8725]

Misc
----

- [The search results tab and the help button are focusable with
  keyboard in rustdoc.][79896]
- [Running tests will now print the total time taken to execute.][75752]

Compatibility Notes
-------------------

- [The `compare_and_swap` method on atomics has been deprecated.][79261]
  It's recommended to use the `compare_exchange` and
  `compare_exchange_weak` methods instead.
- [Changes in how `TokenStream`s are checked have fixed some cases
  where you could write unhygenic `macro_rules!` macros.][79472]
- [`#![test]` as an inner attribute is now considered unstable like
  other inner macro attributes, and reports an error by default
  through the `soft_unstable` lint.][79003]
- [Overriding a `forbid` lint at the same level that it was set is
  now a hard error.][78864]
- [Dropped support for all cloudabi targets.][78439]
- [You can no longer intercept `panic!` calls by supplying your
  own macro.][78343] It's recommended to use the `#[panic_handler]`
  attribute to provide your own implementation.
- [Semi-colons after item statements (e.g. `struct Foo {};`) now
  produce a warning.][78296]

[74989]: rust-lang/rust#74989
[79261]: rust-lang/rust#79261
[79896]: rust-lang/rust#79896
[79484]: rust-lang/rust#79484
[79472]: rust-lang/rust#79472
[79270]: rust-lang/rust#79270
[79003]: rust-lang/rust#79003
[78864]: rust-lang/rust#78864
[78636]: rust-lang/rust#78636
[78439]: rust-lang/rust#78439
[78343]: rust-lang/rust#78343
[78296]: rust-lang/rust#78296
[78068]: rust-lang/rust#78068
[75752]: rust-lang/rust#75752
[74699]: rust-lang/rust#74699
[78142]: rust-lang/rust#78142
[77484]: rust-lang/rust#77484
[cargo/8976]: rust-lang/cargo#8976
[cargo/8973]: rust-lang/cargo#8973
[cargo/8725]: rust-lang/cargo#8725
[`IpAddr::is_ipv4`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv4
[`IpAddr::is_ipv6`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv6
[`Layout::align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.align
[`Layout::from_size_align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.from_size_align
[`Layout::size`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.size
[`Ord::clamp`]: https://doc.rust-lang.org/stable/std/cmp/trait.Ord.html#method.clamp
[`RefCell::take`]: https://doc.rust-lang.org/stable/std/cell/struct.RefCell.html#method.take
[`UnsafeCell::get_mut`]: https://doc.rust-lang.org/stable/std/cell/struct.UnsafeCell.html#method.get_mut
[`bool::then`]: https://doc.rust-lang.org/stable/std/primitive.bool.html#method.then
[`btree_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/btree_map/enum.Entry.html#method.or_insert_with_key
[`f32::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f32.html#method.clamp
[`f64::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f64.html#method.clamp
[`hash_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/hash_map/enum.Entry.html#method.or_insert_with_key
[`slice::fill`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.fill
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants