From 5834772177540912b53dc4f19413a4ba3c804ea4 Mon Sep 17 00:00:00 2001 From: Veera Date: Thu, 4 Jul 2024 19:16:47 -0400 Subject: [PATCH 01/15] Update Tests --- ...ced-return-type-complex-type-issue-126311.rs | 5 +++++ ...return-type-complex-type-issue-126311.stderr | 14 ++++++++++++++ .../misplaced-return-type-issue-126311.rs | 5 +++++ .../misplaced-return-type-issue-126311.stderr | 14 ++++++++++++++ ...turn-type-where-in-next-line-issue-126311.rs | 11 +++++++++++ ...-type-where-in-next-line-issue-126311.stderr | 17 +++++++++++++++++ ...ced-return-type-without-type-issue-126311.rs | 6 ++++++ ...return-type-without-type-issue-126311.stderr | 8 ++++++++ ...ed-return-type-without-where-issue-126311.rs | 4 ++++ ...eturn-type-without-where-issue-126311.stderr | 8 ++++++++ 10 files changed, 92 insertions(+) create mode 100644 tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr diff --git a/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs new file mode 100644 index 0000000000000..722303dd03482 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs @@ -0,0 +1,5 @@ +fn foo() where T: Default -> impl Default + 'static {} +//~^ ERROR return type should be specified after the function parameters +//~| HELP place the return type after the function parameters + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr new file mode 100644 index 0000000000000..2ce3b78bb8ba9 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr @@ -0,0 +1,14 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-complex-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> impl Default + 'static {} + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + | +help: place the return type after the function parameters + | +LL - fn foo() where T: Default -> impl Default + 'static {} +LL + fn foo() -> impl Default + 'static where T: Default {} + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs new file mode 100644 index 0000000000000..ad164f77beea6 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs @@ -0,0 +1,5 @@ +fn foo() where T: Default -> u8 {} +//~^ ERROR return type should be specified after the function parameters +//~| HELP place the return type after the function parameters + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr new file mode 100644 index 0000000000000..e473b902ce3ef --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr @@ -0,0 +1,14 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> u8 {} + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + | +help: place the return type after the function parameters + | +LL - fn foo() where T: Default -> u8 {} +LL + fn foo() -> u8 where T: Default {} + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs new file mode 100644 index 0000000000000..782d7d5ee4905 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs @@ -0,0 +1,11 @@ +fn foo() +//~^ HELP place the return type after the function parameters +where + T: Default, + K: Clone, -> Result +//~^ ERROR return type should be specified after the function parameters +{ + Ok(0) +} + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr new file mode 100644 index 0000000000000..196a46d7ea54b --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr @@ -0,0 +1,17 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-where-in-next-line-issue-126311.rs:5:15 + | +LL | K: Clone, -> Result + | ^^ expected one of `{`, lifetime, or type + | +help: place the return type after the function parameters + | +LL ~ fn foo() -> Result +LL | +LL | where +LL | T: Default, +LL ~ K: Clone, + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs new file mode 100644 index 0000000000000..2c09edbc7926c --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs @@ -0,0 +1,6 @@ +fn foo() where T: Default -> { +//~^ ERROR expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->` + 0 +} + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr new file mode 100644 index 0000000000000..0eb3bb7d8126e --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr @@ -0,0 +1,8 @@ +error: expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->` + --> $DIR/misplaced-return-type-without-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> { + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs new file mode 100644 index 0000000000000..672233674a058 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs @@ -0,0 +1,4 @@ +fn bar() -> u8 -> u64 {} +//~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->` + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr new file mode 100644 index 0000000000000..730904d3671f1 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr @@ -0,0 +1,8 @@ +error: expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->` + --> $DIR/misplaced-return-type-without-where-issue-126311.rs:1:19 + | +LL | fn bar() -> u8 -> u64 {} + | ^^ expected one of 7 possible tokens + +error: aborting due to 1 previous error + From cf09cba20c317c1d2cfa78820a699ac5ee684bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 12 Jul 2024 18:52:52 +0000 Subject: [PATCH 02/15] When finding item gated behind a `cfg` flat, point at it Previously we would only mention that the item was gated out, and opportunisitically mention the feature flag name when possible. We now point to the place where the item was gated, which can be behind layers of macro indirection, or in different modules. ``` error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` --> $DIR/diagnostics-cross-crate.rs:18:23 | LL | cfged_out::inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out --> $DIR/auxiliary/cfged_out.rs:6:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ note: the item is gated here --> $DIR/auxiliary/cfged_out.rs:5:5 | LL | #[cfg(FALSE)] | ^^^^^^^^^^^^^ ``` --- compiler/rustc_resolve/messages.ftl | 2 + compiler/rustc_resolve/src/diagnostics.rs | 8 +++- compiler/rustc_resolve/src/errors.rs | 9 ++++ tests/ui/cfg/diagnostics-cross-crate.rs | 3 ++ tests/ui/cfg/diagnostics-cross-crate.stderr | 27 +++++++++-- tests/ui/cfg/diagnostics-reexport.rs | 8 ++-- tests/ui/cfg/diagnostics-reexport.stderr | 20 ++++++++ tests/ui/cfg/diagnostics-same-crate.rs | 9 ++-- tests/ui/cfg/diagnostics-same-crate.stderr | 48 ++++++++++++++----- tests/ui/macros/builtin-std-paths-fail.stderr | 2 + tests/ui/macros/macro-outer-attributes.stderr | 11 +++++ 11 files changed, 122 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 4b9c36ad39fb5..73d1a2ea49a13 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -232,6 +232,8 @@ resolve_is_private = resolve_item_was_behind_feature = the item is gated behind the `{$feature}` feature +resolve_item_was_cfg_out = the item is gated here + resolve_items_in_traits_are_not_importable = items in traits are not importable diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 09221041031a2..3ce500866bde2 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2530,7 +2530,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { && let NestedMetaItem::MetaItem(meta_item) = &nested[0] && let MetaItemKind::NameValue(feature_name) = &meta_item.kind { - let note = errors::ItemWasBehindFeature { feature: feature_name.symbol }; + let note = errors::ItemWasBehindFeature { + feature: feature_name.symbol, + span: meta_item.span, + }; + err.subdiagnostic(note); + } else { + let note = errors::ItemWasCfgOut { span: cfg.span }; err.subdiagnostic(note); } } diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index 097f4af05c305..0a68231c6fee4 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -1228,6 +1228,15 @@ pub(crate) struct FoundItemConfigureOut { #[note(resolve_item_was_behind_feature)] pub(crate) struct ItemWasBehindFeature { pub(crate) feature: Symbol, + #[primary_span] + pub(crate) span: Span, +} + +#[derive(Subdiagnostic)] +#[note(resolve_item_was_cfg_out)] +pub(crate) struct ItemWasCfgOut { + #[primary_span] + pub(crate) span: Span, } #[derive(Diagnostic)] diff --git a/tests/ui/cfg/diagnostics-cross-crate.rs b/tests/ui/cfg/diagnostics-cross-crate.rs index 77dd91d6c2823..00ac7e2fd080e 100644 --- a/tests/ui/cfg/diagnostics-cross-crate.rs +++ b/tests/ui/cfg/diagnostics-cross-crate.rs @@ -11,12 +11,14 @@ fn main() { cfged_out::inner::uwu(); //~ ERROR cannot find function //~^ NOTE found an item that was configured out //~| NOTE not found in `cfged_out::inner` + //~| NOTE the item is gated here // The module isn't found - we would like to get a diagnostic, but currently don't due to // the awkward way the resolver diagnostics are currently implemented. cfged_out::inner::doesnt_exist::hello(); //~ ERROR failed to resolve //~^ NOTE could not find `doesnt_exist` in `inner` //~| NOTE found an item that was configured out + //~| NOTE the item is gated here // It should find the one in the right module, not the wrong one. cfged_out::inner::right::meow(); //~ ERROR cannot find function @@ -28,4 +30,5 @@ fn main() { cfged_out::vanished(); //~ ERROR cannot find function //~^ NOTE found an item that was configured out //~| NOTE not found in `cfged_out` + //~| NOTE the item is gated here } diff --git a/tests/ui/cfg/diagnostics-cross-crate.stderr b/tests/ui/cfg/diagnostics-cross-crate.stderr index 8a238f3640445..07ad4e3272d1b 100644 --- a/tests/ui/cfg/diagnostics-cross-crate.stderr +++ b/tests/ui/cfg/diagnostics-cross-crate.stderr @@ -1,5 +1,5 @@ error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` - --> $DIR/diagnostics-cross-crate.rs:17:23 + --> $DIR/diagnostics-cross-crate.rs:18:23 | LL | cfged_out::inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` @@ -9,6 +9,11 @@ note: found an item that was configured out | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:5:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in crate `cfged_out` --> $DIR/diagnostics-cross-crate.rs:7:16 @@ -27,9 +32,14 @@ note: found an item that was configured out | LL | pub fn uwu() {} | ^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:2:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `meow` in module `cfged_out::inner::right` - --> $DIR/diagnostics-cross-crate.rs:22:30 + --> $DIR/diagnostics-cross-crate.rs:24:30 | LL | cfged_out::inner::right::meow(); | ^^^^ not found in `cfged_out::inner::right` @@ -39,10 +49,14 @@ note: found an item that was configured out | LL | pub fn meow() {} | ^^^^ - = note: the item is gated behind the `what-a-cool-feature` feature +note: the item is gated behind the `what-a-cool-feature` feature + --> $DIR/auxiliary/cfged_out.rs:16:15 + | +LL | #[cfg(feature = "what-a-cool-feature")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0425]: cannot find function `vanished` in crate `cfged_out` - --> $DIR/diagnostics-cross-crate.rs:28:16 + --> $DIR/diagnostics-cross-crate.rs:30:16 | LL | cfged_out::vanished(); | ^^^^^^^^ not found in `cfged_out` @@ -52,6 +66,11 @@ note: found an item that was configured out | LL | pub fn vanished() {} | ^^^^^^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:21:1 + | +LL | #[cfg(i_dont_exist_and_you_can_do_nothing_about_it)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 5 previous errors diff --git a/tests/ui/cfg/diagnostics-reexport.rs b/tests/ui/cfg/diagnostics-reexport.rs index 9b3208cb87cf9..9ae7d931fcb8f 100644 --- a/tests/ui/cfg/diagnostics-reexport.rs +++ b/tests/ui/cfg/diagnostics-reexport.rs @@ -4,7 +4,7 @@ pub mod inner { pub fn uwu() {} } - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub use super::uwu; //~^ NOTE found an item that was configured out } @@ -14,7 +14,7 @@ pub use a::x; //~| NOTE no `x` in `a` mod a { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn x() {} //~^ NOTE found an item that was configured out } @@ -25,10 +25,10 @@ pub use b::{x, y}; //~| NOTE no `y` in `b` mod b { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn x() {} //~^ NOTE found an item that was configured out - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn y() {} //~^ NOTE found an item that was configured out } diff --git a/tests/ui/cfg/diagnostics-reexport.stderr b/tests/ui/cfg/diagnostics-reexport.stderr index e25b7cf86e210..737202fdf9ad9 100644 --- a/tests/ui/cfg/diagnostics-reexport.stderr +++ b/tests/ui/cfg/diagnostics-reexport.stderr @@ -9,6 +9,11 @@ note: found an item that was configured out | LL | pub fn x() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:17:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0432]: unresolved imports `b::x`, `b::y` --> $DIR/diagnostics-reexport.rs:22:13 @@ -23,11 +28,21 @@ note: found an item that was configured out | LL | pub fn x() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:28:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ note: found an item that was configured out --> $DIR/diagnostics-reexport.rs:32:12 | LL | pub fn y() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:31:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in module `inner` --> $DIR/diagnostics-reexport.rs:38:12 @@ -40,6 +55,11 @@ note: found an item that was configured out | LL | pub use super::uwu; | ^^^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:7:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error: aborting due to 3 previous errors diff --git a/tests/ui/cfg/diagnostics-same-crate.rs b/tests/ui/cfg/diagnostics-same-crate.rs index b2a0fb58dd6b5..d6f8dd21a9221 100644 --- a/tests/ui/cfg/diagnostics-same-crate.rs +++ b/tests/ui/cfg/diagnostics-same-crate.rs @@ -1,11 +1,13 @@ #![allow(unexpected_cfgs)] // since we want to recognize them as unexpected pub mod inner { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn uwu() {} //~^ NOTE found an item that was configured out - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here + //~^ NOTE the item is gated here + //~| NOTE the item is gated here pub mod doesnt_exist { //~^ NOTE found an item that was configured out //~| NOTE found an item that was configured out @@ -20,7 +22,7 @@ pub mod inner { } pub mod right { - #[cfg(feature = "what-a-cool-feature")] + #[cfg(feature = "what-a-cool-feature")] //~ NOTE the item is gated behind the `what-a-cool-feature` feature pub fn meow() {} //~^ NOTE found an item that was configured out } @@ -55,7 +57,6 @@ fn main() { // It should find the one in the right module, not the wrong one. inner::right::meow(); //~ ERROR cannot find function //~| NOTE not found in `inner::right - //~| NOTE the item is gated behind the `what-a-cool-feature` feature // Exists in the crate root - we would generally want a diagnostic, // but currently don't have one. diff --git a/tests/ui/cfg/diagnostics-same-crate.stderr b/tests/ui/cfg/diagnostics-same-crate.stderr index 86421736b8c60..dd0d10c6567ea 100644 --- a/tests/ui/cfg/diagnostics-same-crate.stderr +++ b/tests/ui/cfg/diagnostics-same-crate.stderr @@ -1,41 +1,56 @@ error[E0432]: unresolved import `super::inner::doesnt_exist` - --> $DIR/diagnostics-same-crate.rs:30:9 + --> $DIR/diagnostics-same-crate.rs:32:9 | LL | use super::inner::doesnt_exist; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ no `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0432]: unresolved import `super::inner::doesnt_exist` - --> $DIR/diagnostics-same-crate.rs:33:23 + --> $DIR/diagnostics-same-crate.rs:35:23 | LL | use super::inner::doesnt_exist::hi; | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` - --> $DIR/diagnostics-same-crate.rs:52:12 + --> $DIR/diagnostics-same-crate.rs:54:12 | LL | inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in module `inner` - --> $DIR/diagnostics-same-crate.rs:47:12 + --> $DIR/diagnostics-same-crate.rs:49:12 | LL | inner::uwu(); | ^^^ not found in `inner` @@ -45,28 +60,37 @@ note: found an item that was configured out | LL | pub fn uwu() {} | ^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:4:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `meow` in module `inner::right` - --> $DIR/diagnostics-same-crate.rs:56:19 + --> $DIR/diagnostics-same-crate.rs:58:19 | LL | inner::right::meow(); | ^^^^ not found in `inner::right` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:24:16 + --> $DIR/diagnostics-same-crate.rs:26:16 | LL | pub fn meow() {} | ^^^^ - = note: the item is gated behind the `what-a-cool-feature` feature +note: the item is gated behind the `what-a-cool-feature` feature + --> $DIR/diagnostics-same-crate.rs:25:15 + | +LL | #[cfg(feature = "what-a-cool-feature")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in this scope - --> $DIR/diagnostics-same-crate.rs:43:5 + --> $DIR/diagnostics-same-crate.rs:45:5 | LL | uwu(); | ^^^ not found in this scope error[E0425]: cannot find function `vanished` in this scope - --> $DIR/diagnostics-same-crate.rs:63:5 + --> $DIR/diagnostics-same-crate.rs:64:5 | LL | vanished(); | ^^^^^^^^ not found in this scope diff --git a/tests/ui/macros/builtin-std-paths-fail.stderr b/tests/ui/macros/builtin-std-paths-fail.stderr index 331943843c02a..49034c3987b24 100644 --- a/tests/ui/macros/builtin-std-paths-fail.stderr +++ b/tests/ui/macros/builtin-std-paths-fail.stderr @@ -104,6 +104,8 @@ LL | #[std::test] | note: found an item that was configured out --> $SRC_DIR/std/src/lib.rs:LL:COL +note: the item is gated here + --> $SRC_DIR/std/src/lib.rs:LL:COL error: aborting due to 16 previous errors diff --git a/tests/ui/macros/macro-outer-attributes.stderr b/tests/ui/macros/macro-outer-attributes.stderr index 87c0655a422d6..a8809f3fcff69 100644 --- a/tests/ui/macros/macro-outer-attributes.stderr +++ b/tests/ui/macros/macro-outer-attributes.stderr @@ -9,6 +9,17 @@ note: found an item that was configured out | LL | pub fn bar() { }); | ^^^ +note: the item is gated here + --> $DIR/macro-outer-attributes.rs:5:45 + | +LL | $i:item) => (mod $nm { #[$a] $i }); } + | ^^^^^ +LL | +LL | / test!(a, +LL | | #[cfg(FALSE)], +LL | | pub fn bar() { }); + | |_______________________- in this macro invocation + = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider importing this function | LL + use b::bar; From 2e1e6274306bed67291c1eefc7332d0bf972e6ac Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 18 Jul 2024 12:06:21 -0700 Subject: [PATCH 03/15] rustdoc: fix `current` class on sidebar modnav --- src/librustdoc/html/static/js/main.js | 8 +++++--- tests/rustdoc-gui/sidebar.goml | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 64c356607788c..116ce615d8c55 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -529,13 +529,15 @@ function preLoadCss(cssUrl) { } const link = document.createElement("a"); link.href = path; - if (path === current_page) { - link.className = "current"; - } link.textContent = name; const li = document.createElement("li"); li.appendChild(link); ul.appendChild(li); + // Don't "optimize" this to just use `path`. + // We want the browser to normalize this into an absolute URL. + if (link.href === current_page) { + li.classList.add("current"); + } } sidebar.appendChild(h3); sidebar.appendChild(ul); diff --git a/tests/rustdoc-gui/sidebar.goml b/tests/rustdoc-gui/sidebar.goml index 56453517a55a2..e499c159c6c76 100644 --- a/tests/rustdoc-gui/sidebar.goml +++ b/tests/rustdoc-gui/sidebar.goml @@ -72,6 +72,7 @@ click: "#structs + .item-table .item-name > a" assert-count: (".sidebar .sidebar-crate", 1) assert-count: (".sidebar .location", 1) assert-count: (".sidebar h2", 3) +assert-text: (".sidebar-elems ul.block > li.current > a", "Foo") // We check that there is no crate listed outside of the top level. assert-false: ".sidebar-elems > .crate" @@ -110,6 +111,7 @@ click: "#functions + .item-table .item-name > a" assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-count: (".sidebar .location", 0) assert-count: (".sidebar h2", 1) +assert-text: (".sidebar-elems ul.block > li.current > a", "foobar") // We check that we don't have the crate list. assert-false: ".sidebar-elems > .crate" @@ -118,6 +120,7 @@ assert-property: (".sidebar", {"clientWidth": "200"}) assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-text: (".sidebar > .location", "Module module") assert-count: (".sidebar .location", 1) +assert-text: (".sidebar-elems ul.block > li.current > a", "module") // Module page requires three headings: // - Presistent crate branding (name and version) // - Module name, followed by TOC for module headings @@ -138,6 +141,7 @@ assert-text: (".sidebar > .sidebar-elems > h2", "In lib2::module::sub_module") assert-property: (".sidebar > .sidebar-elems > h2 > a", { "href": "/module/sub_module/index.html", }, ENDS_WITH) +assert-text: (".sidebar-elems ul.block > li.current > a", "sub_sub_module") // We check that we don't have the crate list. assert-false: ".sidebar-elems .crate" assert-text: (".sidebar-elems > section ul > li:nth-child(1)", "Functions") From 4cad705017f34a6545deb1505d0ce85537c18df5 Mon Sep 17 00:00:00 2001 From: Veera Date: Thu, 4 Jul 2024 19:19:15 -0400 Subject: [PATCH 04/15] Parser: Suggest Placing the Return Type After Function Parameters --- compiler/rustc_parse/messages.ftl | 2 + compiler/rustc_parse/src/errors.rs | 22 +++- .../rustc_parse/src/parser/diagnostics.rs | 8 +- compiler/rustc_parse/src/parser/item.rs | 124 ++++++++++++++---- compiler/rustc_parse/src/parser/mod.rs | 1 + compiler/rustc_parse/src/parser/ty.rs | 5 + 6 files changed, 123 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index f08efe60d96b8..984d402390327 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -526,6 +526,8 @@ parse_mismatched_closing_delimiter = mismatched closing delimiter: `{$delimiter} .label_opening_candidate = closing delimiter possibly meant for this .label_unclosed = unclosed delimiter +parse_misplaced_return_type = place the return type after the function parameters + parse_missing_comma_after_match_arm = expected `,` following `match` arm .suggestion = missing a comma here to end this `match` arm diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 8d49887f16441..05bc0686e8efe 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -1434,6 +1434,20 @@ pub(crate) struct FnPtrWithGenerics { pub sugg: Option, } +#[derive(Subdiagnostic)] +#[multipart_suggestion( + parse_misplaced_return_type, + style = "verbose", + applicability = "maybe-incorrect" +)] +pub(crate) struct MisplacedReturnType { + #[suggestion_part(code = " {snippet}")] + pub fn_params_end: Span, + pub snippet: String, + #[suggestion_part(code = "")] + pub ret_ty_span: Span, +} + #[derive(Subdiagnostic)] #[multipart_suggestion(parse_suggestion, applicability = "maybe-incorrect")] pub(crate) struct FnPtrWithGenericsSugg { @@ -1448,7 +1462,6 @@ pub(crate) struct FnPtrWithGenericsSugg { pub(crate) struct FnTraitMissingParen { pub span: Span, - pub machine_applicable: bool, } impl Subdiagnostic for FnTraitMissingParen { @@ -1458,16 +1471,11 @@ impl Subdiagnostic for FnTraitMissingParen { _: &F, ) { diag.span_label(self.span, crate::fluent_generated::parse_fn_trait_missing_paren); - let applicability = if self.machine_applicable { - Applicability::MachineApplicable - } else { - Applicability::MaybeIncorrect - }; diag.span_suggestion_short( self.span.shrink_to_hi(), crate::fluent_generated::parse_add_paren, "()", - applicability, + Applicability::MachineApplicable, ); } } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 81d5f0fca0ec9..aecedc7d1c879 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -430,7 +430,7 @@ impl<'a> Parser<'a> { &mut self, edible: &[TokenKind], inedible: &[TokenKind], - ) -> PResult<'a, Recovered> { + ) -> PResult<'a, ErrorGuaranteed> { debug!("expected_one_of_not_found(edible: {:?}, inedible: {:?})", edible, inedible); fn tokens_to_string(tokens: &[TokenType]) -> String { let mut i = tokens.iter(); @@ -533,7 +533,7 @@ impl<'a> Parser<'a> { sugg: ExpectedSemiSugg::ChangeToSemi(self.token.span), }); self.bump(); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } else if self.look_ahead(0, |t| { t == &token::CloseDelim(Delimiter::Brace) || ((t.can_begin_expr() || t.can_begin_item()) @@ -557,7 +557,7 @@ impl<'a> Parser<'a> { unexpected_token_label: Some(self.token.span), sugg: ExpectedSemiSugg::AddSemi(span), }); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } } @@ -712,7 +712,7 @@ impl<'a> Parser<'a> { if self.check_too_many_raw_str_terminators(&mut err) { if expected.contains(&TokenType::Token(token::Semi)) && self.eat(&token::Semi) { let guar = err.emit(); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } else { return Err(err); } diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index abb6b51cebd68..077f498a2e02a 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -19,6 +19,7 @@ use rustc_span::edit_distance::edit_distance; use rustc_span::edition::Edition; use rustc_span::source_map; use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::ErrorGuaranteed; use rustc_span::{Span, DUMMY_SP}; use std::fmt::Write; use std::mem; @@ -2332,14 +2333,106 @@ impl<'a> Parser<'a> { } } }; + + // Store the end of function parameters to give better diagnostics + // inside `parse_fn_body()`. + let fn_params_end = self.prev_token.span.shrink_to_hi(); + generics.where_clause = self.parse_where_clause()?; // `where T: Ord` + // `fn_params_end` is needed only when it's followed by a where clause. + let fn_params_end = + if generics.where_clause.has_where_token { Some(fn_params_end) } else { None }; + let mut sig_hi = self.prev_token.span; - let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`. + // Either `;` or `{ ... }`. + let body = + self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body, fn_params_end)?; let fn_sig_span = sig_lo.to(sig_hi); Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body)) } + /// Provide diagnostics when function body is not found + fn error_fn_body_not_found( + &mut self, + ident_span: Span, + req_body: bool, + fn_params_end: Option, + ) -> PResult<'a, ErrorGuaranteed> { + let expected = if req_body { + &[token::OpenDelim(Delimiter::Brace)][..] + } else { + &[token::Semi, token::OpenDelim(Delimiter::Brace)] + }; + match self.expected_one_of_not_found(&[], expected) { + Ok(error_guaranteed) => Ok(error_guaranteed), + Err(mut err) => { + if self.token.kind == token::CloseDelim(Delimiter::Brace) { + // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in + // the AST for typechecking. + err.span_label(ident_span, "while parsing this `fn`"); + Ok(err.emit()) + } else if self.token.kind == token::RArrow + && let Some(fn_params_end) = fn_params_end + { + // Instead of a function body, the parser has encountered a right arrow + // preceded by a where clause. + + // Find whether token behind the right arrow is a function trait and + // store its span. + let fn_trait_span = + [sym::FnOnce, sym::FnMut, sym::Fn].into_iter().find_map(|symbol| { + if self.prev_token.is_ident_named(symbol) { + Some(self.prev_token.span) + } else { + None + } + }); + + // Parse the return type (along with the right arrow) and store its span. + // If there's a parse error, cancel it and return the existing error + // as we are primarily concerned with the + // expected-function-body-but-found-something-else error here. + let arrow_span = self.token.span; + let ty_span = match self.parse_ret_ty( + AllowPlus::Yes, + RecoverQPath::Yes, + RecoverReturnSign::Yes, + ) { + Ok(ty_span) => ty_span.span().shrink_to_hi(), + Err(parse_error) => { + parse_error.cancel(); + return Err(err); + } + }; + let ret_ty_span = arrow_span.to(ty_span); + + if let Some(fn_trait_span) = fn_trait_span { + // Typo'd Fn* trait bounds such as + // fn foo() where F: FnOnce -> () {} + err.subdiagnostic(errors::FnTraitMissingParen { span: fn_trait_span }); + } else if let Ok(snippet) = self.psess.source_map().span_to_snippet(ret_ty_span) + { + // If token behind right arrow is not a Fn* trait, the programmer + // probably misplaced the return type after the where clause like + // `fn foo() where T: Default -> u8 {}` + err.primary_message( + "return type should be specified after the function parameters", + ); + err.subdiagnostic(errors::MisplacedReturnType { + fn_params_end, + snippet, + ret_ty_span, + }); + } + Err(err) + } else { + Err(err) + } + } + } + } + /// Parse the "body" of a function. /// This can either be `;` when there's no body, /// or e.g. a block when the function is a provided one. @@ -2349,6 +2442,7 @@ impl<'a> Parser<'a> { ident: &Ident, sig_hi: &mut Span, req_body: bool, + fn_params_end: Option, ) -> PResult<'a, Option>> { let has_semi = if req_body { self.token.kind == TokenKind::Semi @@ -2377,33 +2471,7 @@ impl<'a> Parser<'a> { }); (AttrVec::new(), Some(self.mk_block_err(span, guar))) } else { - let expected = if req_body { - &[token::OpenDelim(Delimiter::Brace)][..] - } else { - &[token::Semi, token::OpenDelim(Delimiter::Brace)] - }; - if let Err(mut err) = self.expected_one_of_not_found(&[], expected) { - if self.token.kind == token::CloseDelim(Delimiter::Brace) { - // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in - // the AST for typechecking. - err.span_label(ident.span, "while parsing this `fn`"); - err.emit(); - } else { - // check for typo'd Fn* trait bounds such as - // fn foo() where F: FnOnce -> () {} - if self.token.kind == token::RArrow { - let machine_applicable = [sym::FnOnce, sym::FnMut, sym::Fn] - .into_iter() - .any(|s| self.prev_token.is_ident_named(s)); - - err.subdiagnostic(errors::FnTraitMissingParen { - span: self.prev_token.span, - machine_applicable, - }); - } - return Err(err); - } - } + self.error_fn_body_not_found(ident.span, req_body, fn_params_end)?; (AttrVec::new(), None) }; attrs.extend(inner_attrs); diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index cfd0a72c056d5..c3c35f8e6c680 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -506,6 +506,7 @@ impl<'a> Parser<'a> { FatalError.raise(); } else { self.expected_one_of_not_found(edible, inedible) + .map(|error_guaranteed| Recovered::Yes(error_guaranteed)) } } diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 1e5b227aaa9be..17a0829c6d2f6 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -21,6 +21,11 @@ use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{ErrorGuaranteed, Span, Symbol}; use thin_vec::{thin_vec, ThinVec}; +/// Signals whether parsing a type should allow `+`. +/// +/// For example, let T be the type `impl Default + 'static` +/// With `AllowPlus::Yes`, T will be parsed successfully +/// With `AllowPlus::No`, parsing T will return a parse error #[derive(Copy, Clone, PartialEq)] pub(super) enum AllowPlus { Yes, From 2f5a84ea16f232f8a0709ea567bca0cfd90b44cf Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jul 2024 18:02:09 -0400 Subject: [PATCH 05/15] Don't allow unsafe statics outside of extern blocks --- compiler/rustc_ast_passes/messages.ftl | 3 +++ compiler/rustc_ast_passes/src/ast_validation.rs | 8 ++++++++ compiler/rustc_ast_passes/src/errors.rs | 7 +++++++ tests/ui/rust-2024/safe-outside-extern.gated.stderr | 8 +++++++- tests/ui/rust-2024/safe-outside-extern.rs | 3 +++ tests/ui/rust-2024/safe-outside-extern.ungated.stderr | 8 +++++++- 6 files changed, 35 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl index 02bdff96aa6d0..8f7dd77420709 100644 --- a/compiler/rustc_ast_passes/messages.ftl +++ b/compiler/rustc_ast_passes/messages.ftl @@ -269,6 +269,9 @@ ast_passes_unsafe_negative_impl = negative impls cannot be unsafe .negative = negative because of this .unsafe = unsafe because of this +ast_passes_unsafe_static = + static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + ast_passes_visibility_not_permitted = visibility qualifiers are not permitted here .enum_variant = enum variants and their fields always share the visibility of the enum they are in diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 83249dea82a3e..34aac6e447304 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -438,6 +438,11 @@ impl<'a> AstValidator<'a> { } } + /// This ensures that items can only be `unsafe` (or unmarked) outside of extern + /// blocks. + /// + /// This additionally ensures that within extern blocks, items can only be + /// `safe`/`unsafe` inside of a `unsafe`-adorned extern block. fn check_item_safety(&self, span: Span, safety: Safety) { match self.extern_mod_safety { Some(extern_safety) => { @@ -1177,6 +1182,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } ItemKind::Static(box StaticItem { expr, safety, .. }) => { self.check_item_safety(item.span, *safety); + if matches!(safety, Safety::Unsafe(_)) { + self.dcx().emit_err(errors::UnsafeStatic { span: item.span }); + } if expr.is_none() { self.dcx().emit_err(errors::StaticWithoutBody { diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 460da254653c6..783bca6b6958d 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -224,6 +224,13 @@ pub struct InvalidSafetyOnBareFn { pub span: Span, } +#[derive(Diagnostic)] +#[diag(ast_passes_unsafe_static)] +pub struct UnsafeStatic { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(ast_passes_bound_in_context)] pub struct BoundInContext<'a> { diff --git a/tests/ui/rust-2024/safe-outside-extern.gated.stderr b/tests/ui/rust-2024/safe-outside-extern.gated.stderr index 18a3361f35b8e..e0b218281f365 100644 --- a/tests/ui/rust-2024/safe-outside-extern.gated.stderr +++ b/tests/ui/rust-2024/safe-outside-extern.gated.stderr @@ -28,5 +28,11 @@ error: function pointers cannot be declared with `safe` safety qualifier LL | type FnPtr = safe fn(i32, i32) -> i32; | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + --> $DIR/safe-outside-extern.rs:28:1 + | +LL | unsafe static LOL: u8 = 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/ui/rust-2024/safe-outside-extern.rs b/tests/ui/rust-2024/safe-outside-extern.rs index 9ec0c5c70e19e..6773df5ef03b7 100644 --- a/tests/ui/rust-2024/safe-outside-extern.rs +++ b/tests/ui/rust-2024/safe-outside-extern.rs @@ -25,4 +25,7 @@ type FnPtr = safe fn(i32, i32) -> i32; //~^ ERROR: function pointers cannot be declared with `safe` safety qualifier //[ungated]~| ERROR: unsafe extern {}` blocks and `safe` keyword are experimental [E0658] +unsafe static LOL: u8 = 0; +//~^ ERROR: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + fn main() {} diff --git a/tests/ui/rust-2024/safe-outside-extern.ungated.stderr b/tests/ui/rust-2024/safe-outside-extern.ungated.stderr index 9ea6d451e8c47..98a4c0eab921a 100644 --- a/tests/ui/rust-2024/safe-outside-extern.ungated.stderr +++ b/tests/ui/rust-2024/safe-outside-extern.ungated.stderr @@ -28,6 +28,12 @@ error: function pointers cannot be declared with `safe` safety qualifier LL | type FnPtr = safe fn(i32, i32) -> i32; | ^^^^^^^^^^^^^^^^^^^^^^^^ +error: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + --> $DIR/safe-outside-extern.rs:28:1 + | +LL | unsafe static LOL: u8 = 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + error[E0658]: `unsafe extern {}` blocks and `safe` keyword are experimental --> $DIR/safe-outside-extern.rs:4:1 | @@ -78,6 +84,6 @@ LL | type FnPtr = safe fn(i32, i32) -> i32; = help: add `#![feature(unsafe_extern_blocks)]` to the crate attributes to enable = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0658`. From a87122e3eded52ec4f472702d40ed54161784c4e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 12 Jul 2024 15:47:16 +1000 Subject: [PATCH 06/15] Simplify `CaptureState::inner_attr_ranges`. The `Option`s within the `ReplaceRange`s within the hashmap are always `None`. This PR omits them and inserts them when they are extracted from the hashmap. --- compiler/rustc_parse/src/parser/attr.rs | 6 +++--- compiler/rustc_parse/src/parser/attr_wrapper.rs | 2 +- compiler/rustc_parse/src/parser/mod.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index a8fe35f45b31e..aeef6e62fda25 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -303,7 +303,6 @@ impl<'a> Parser<'a> { None }; if let Some(attr) = attr { - let end_pos = self.num_bump_calls; // If we are currently capturing tokens, mark the location of this inner attribute. // If capturing ends up creating a `LazyAttrTokenStream`, we will include // this replace range with it, removing the inner attribute from the final @@ -311,9 +310,10 @@ impl<'a> Parser<'a> { // During macro expansion, they are selectively inserted back into the // token stream (the first inner attribute is removed each time we invoke the // corresponding macro). - let range = start_pos..end_pos; if let Capturing::Yes = self.capture_state.capturing { - self.capture_state.inner_attr_ranges.insert(attr.id, (range, None)); + let end_pos = self.num_bump_calls; + let range = start_pos..end_pos; + self.capture_state.inner_attr_ranges.insert(attr.id, range); } attrs.push(attr); } else { diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 7e56e92f87b31..efe9c55f523d6 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -260,7 +260,7 @@ impl<'a> Parser<'a> { // Take the captured ranges for any inner attributes that we parsed. for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { - inner_attr_replace_ranges.push(attr_range); + inner_attr_replace_ranges.push((attr_range, None)); } else { self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute"); } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c03527acb2cfe..9a63a205242bb 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -225,7 +225,7 @@ enum Capturing { struct CaptureState { capturing: Capturing, replace_ranges: Vec, - inner_attr_ranges: FxHashMap, + inner_attr_ranges: FxHashMap>, } /// Iterator over a `TokenStream` that produces `Token`s. It's a bit odd that From 9ee82f7a7c8c809fbeabfc075169926394a9de74 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 12 Jul 2024 16:03:51 +1000 Subject: [PATCH 07/15] Remove `final_attrs` local variable. It's no shorter than `ret.attrs()`, and `ret.attrs()` is used multiple times earlier in the function. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index efe9c55f523d6..1743cb0bdbcc6 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -313,15 +313,13 @@ impl<'a> Parser<'a> { *target_tokens = Some(tokens.clone()); } - let final_attrs = ret.attrs(); - // If `capture_cfg` is set and we're inside a recursive call to // `collect_tokens_trailing_token`, then we need to register a replace range // if we have `#[cfg]` or `#[cfg_attr]`. This allows us to run eager cfg-expansion // on the captured token stream. if self.capture_cfg && matches!(self.capture_state.capturing, Capturing::Yes) - && has_cfg_or_cfg_attr(final_attrs) + && has_cfg_or_cfg_attr(ret.attrs()) { assert!(!self.break_last_token, "Should not have unglued last token with cfg attr"); @@ -329,7 +327,7 @@ impl<'a> Parser<'a> { // `target`. If this AST node is inside an item that has `#[derive]`, then this will // allow us to cfg-expand this AST node. let start_pos = if has_outer_attrs { attrs.start_pos } else { start_pos }; - let target = AttrsTarget { attrs: final_attrs.iter().cloned().collect(), tokens }; + let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens }; self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); } else if matches!(self.capture_state.capturing, Capturing::No) { From 28c7ae76e941776c280f64d7d9eae96abed6bde6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 15 Jul 2024 10:33:21 +1000 Subject: [PATCH 08/15] Move `inner_attr` code downwards. This puts it just before the `replace_ranges` initialization, which makes sense because the two variables are closely related. --- .../rustc_parse/src/parser/attr_wrapper.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 1743cb0bdbcc6..3b37a2dd9e46d 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -256,16 +256,6 @@ impl<'a> Parser<'a> { return Ok(ret); } - let mut inner_attr_replace_ranges = Vec::new(); - // Take the captured ranges for any inner attributes that we parsed. - for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { - if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { - inner_attr_replace_ranges.push((attr_range, None)); - } else { - self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute"); - } - } - let replace_ranges_end = self.capture_state.replace_ranges.len(); assert!( @@ -283,6 +273,16 @@ impl<'a> Parser<'a> { let num_calls = end_pos - start_pos; + // Take the captured ranges for any inner attributes that we parsed. + let mut inner_attr_replace_ranges = Vec::new(); + for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { + if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { + inner_attr_replace_ranges.push((attr_range, None)); + } else { + self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute"); + } + } + // This is hot enough for `deep-vector` that checking the conditions for an empty iterator // is measurably faster than actually executing the iterator. let replace_ranges: Box<[ReplaceRange]> = From 78bfffa3cdf9caf331117f3efe985632b5e91c77 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 17:59:05 +1000 Subject: [PATCH 09/15] Make `Parser::num_bump_calls` 0-indexed. Currently in `collect_tokens_trailing_token`, `start_pos` and `end_pos` are 1-indexed by `replace_ranges` is 0-indexed, which is really confusing. Making them both 0-indexed makes debugging much easier. --- compiler/rustc_parse/src/parser/mod.rs | 5 +++++ compiler/rustc_parse/src/parser/tests.rs | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 9a63a205242bb..33900503520d2 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -425,6 +425,11 @@ impl<'a> Parser<'a> { // Make parser point to the first token. parser.bump(); + // Change this from 1 back to 0 after the bump. This eases debugging of + // `Parser::collect_tokens_trailing_token` nicer because it makes the + // token positions 0-indexed which is nicer than 1-indexed. + parser.num_bump_calls = 0; + parser } diff --git a/compiler/rustc_parse/src/parser/tests.rs b/compiler/rustc_parse/src/parser/tests.rs index cf791d332a2fc..491aa71155af2 100644 --- a/compiler/rustc_parse/src/parser/tests.rs +++ b/compiler/rustc_parse/src/parser/tests.rs @@ -1522,7 +1522,7 @@ fn debug_lookahead() { }, }, tokens: [], - approx_token_stream_pos: 1, + approx_token_stream_pos: 0, .. }" ); @@ -1566,7 +1566,7 @@ fn debug_lookahead() { Parenthesis, ), ], - approx_token_stream_pos: 1, + approx_token_stream_pos: 0, .. }" ); @@ -1631,7 +1631,7 @@ fn debug_lookahead() { Semi, Eof, ], - approx_token_stream_pos: 1, + approx_token_stream_pos: 0, .. }" ); @@ -1663,7 +1663,7 @@ fn debug_lookahead() { No, ), ], - approx_token_stream_pos: 9, + approx_token_stream_pos: 8, .. }" ); @@ -1701,7 +1701,7 @@ fn debug_lookahead() { No, ), ], - approx_token_stream_pos: 9, + approx_token_stream_pos: 8, .. }" ); @@ -1728,7 +1728,7 @@ fn debug_lookahead() { tokens: [ Eof, ], - approx_token_stream_pos: 15, + approx_token_stream_pos: 14, .. }" ); From e69ff1c1065933b3737e662f8f237ffbee964755 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 02:21:39 +1000 Subject: [PATCH 10/15] Remove an unnecessary `ForceCollect::Yes`. No need to collect tokens on this recovery path, because the parsed statement isn't even looked at. --- compiler/rustc_parse/src/parser/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c03527acb2cfe..77965921fdf78 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -942,11 +942,10 @@ impl<'a> Parser<'a> { let initial_semicolon = self.token.span; while self.eat(&TokenKind::Semi) { - let _ = - self.parse_stmt_without_recovery(false, ForceCollect::Yes).unwrap_or_else(|e| { - e.cancel(); - None - }); + let _ = self.parse_stmt_without_recovery(false, ForceCollect::No).unwrap_or_else(|e| { + e.cancel(); + None + }); } expect_err From 7d7e2a173aac29bd89435096347613f445a5202f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 02:27:01 +1000 Subject: [PATCH 11/15] Don't always force collect tokens in `recover_stmt_local_after_let`. Use a parameter to decide whether to force collect, as is done for the closely related `parse_local_mk` method. --- compiler/rustc_parse/src/parser/stmt.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 3ec891b4eea34..69dd3a3d65aac 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -72,6 +72,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::MissingLet, + force_collect, )? } else if self.is_kw_followed_by_ident(kw::Auto) && self.may_recover() { self.bump(); // `auto` @@ -79,6 +80,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::UseLetNotAuto, + force_collect, )? } else if self.is_kw_followed_by_ident(sym::var) && self.may_recover() { self.bump(); // `var` @@ -86,6 +88,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::UseLetNotVar, + force_collect, )? } else if self.check_path() && !self.token.is_qpath_start() @@ -231,13 +234,13 @@ impl<'a> Parser<'a> { lo: Span, attrs: AttrWrapper, subdiagnostic: fn(Span) -> errors::InvalidVariableDeclarationSub, + force_collect: ForceCollect, ) -> PResult<'a, Stmt> { - let stmt = - self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { - let local = this.parse_local(attrs)?; - // FIXME - maybe capture semicolon in recovery? - Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false)) - })?; + let stmt = self.collect_tokens_trailing_token(attrs, force_collect, |this, attrs| { + let local = this.parse_local(attrs)?; + // FIXME - maybe capture semicolon in recovery? + Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false)) + })?; self.dcx() .emit_err(errors::InvalidVariableDeclaration { span: lo, sub: subdiagnostic(lo) }); Ok(stmt) From 9d908a2877ec54101e3f4c4b7d5e13facba9d3e0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 13:14:35 +1000 Subject: [PATCH 12/15] Use `ForceCollect` in `parse_attr_item`. Instead of a `bool`. Because `ForceCollect` is used in this way everywhere else. --- .../rustc_builtin_macros/src/cmdline_attrs.rs | 16 +++++++++------- compiler/rustc_parse/src/parser/attr.rs | 11 +++++++---- compiler/rustc_parse/src/parser/nonterminal.rs | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs index 58928815e8930..bffd5672b9b0c 100644 --- a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs +++ b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs @@ -4,6 +4,7 @@ use crate::errors; use rustc_ast::attr::mk_attr; use rustc_ast::token; use rustc_ast::{self as ast, AttrItem, AttrStyle}; +use rustc_parse::parser::ForceCollect; use rustc_parse::{new_parser_from_source_str, unwrap_or_emit_fatal}; use rustc_session::parse::ParseSess; use rustc_span::FileName; @@ -17,13 +18,14 @@ pub fn inject(krate: &mut ast::Crate, psess: &ParseSess, attrs: &[String]) { )); let start_span = parser.token.span; - let AttrItem { unsafety, path, args, tokens: _ } = match parser.parse_attr_item(false) { - Ok(ai) => ai, - Err(err) => { - err.emit(); - continue; - } - }; + let AttrItem { unsafety, path, args, tokens: _ } = + match parser.parse_attr_item(ForceCollect::No) { + Ok(ai) => ai, + Err(err) => { + err.emit(); + continue; + } + }; let end_span = parser.token.span; if parser.token != token::Eof { psess.dcx().emit_err(errors::InvalidCrateAttr { span: start_span.to(end_span) }); diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index a8fe35f45b31e..347cef86d981d 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -124,7 +124,7 @@ impl<'a> Parser<'a> { if this.eat(&token::Not) { ast::AttrStyle::Inner } else { ast::AttrStyle::Outer }; this.expect(&token::OpenDelim(Delimiter::Bracket))?; - let item = this.parse_attr_item(false)?; + let item = this.parse_attr_item(ForceCollect::No)?; this.expect(&token::CloseDelim(Delimiter::Bracket))?; let attr_sp = lo.to(this.prev_token.span); @@ -248,7 +248,7 @@ impl<'a> Parser<'a> { /// PATH /// PATH `=` UNSUFFIXED_LIT /// The delimiters or `=` are still put into the resulting token stream. - pub fn parse_attr_item(&mut self, capture_tokens: bool) -> PResult<'a, ast::AttrItem> { + pub fn parse_attr_item(&mut self, force_collect: ForceCollect) -> PResult<'a, ast::AttrItem> { maybe_whole!(self, NtMeta, |attr| attr.into_inner()); let do_parse = |this: &mut Self| { @@ -271,7 +271,10 @@ impl<'a> Parser<'a> { Ok(ast::AttrItem { unsafety, path, args, tokens: None }) }; // Attr items don't have attributes - if capture_tokens { self.collect_tokens_no_attrs(do_parse) } else { do_parse(self) } + match force_collect { + ForceCollect::Yes => self.collect_tokens_no_attrs(do_parse), + ForceCollect::No => do_parse(self), + } } /// Parses attributes that appear after the opening of an item. These should @@ -344,7 +347,7 @@ impl<'a> Parser<'a> { let mut expanded_attrs = Vec::with_capacity(1); while self.token.kind != token::Eof { let lo = self.token.span; - let item = self.parse_attr_item(true)?; + let item = self.parse_attr_item(ForceCollect::Yes)?; expanded_attrs.push((item, lo.to(self.prev_token.span))); if !self.eat(&token::Comma) { break; diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index 41e31d76d62d5..886d6af173535 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -171,7 +171,7 @@ impl<'a> Parser<'a> { NonterminalKind::Path => { NtPath(P(self.collect_tokens_no_attrs(|this| this.parse_path(PathStyle::Type))?)) } - NonterminalKind::Meta => NtMeta(P(self.parse_attr_item(true)?)), + NonterminalKind::Meta => NtMeta(P(self.parse_attr_item(ForceCollect::Yes)?)), NonterminalKind::Vis => { NtVis(P(self .collect_tokens_no_attrs(|this| this.parse_visibility(FollowedByType::Yes))?)) From 4b20da7994e042b07acea882b533b02a2b26d546 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 15 Jul 2024 10:37:27 +1000 Subject: [PATCH 13/15] Overhaul comments in `collect_tokens_trailing_token`. Adding details, clarifying lots of little things, etc. In particular, the commit adds details of an example. I find this very helpful, because it's taken me a long time to understand how this code works. --- compiler/rustc_parse/src/parser/attr.rs | 13 +- .../rustc_parse/src/parser/attr_wrapper.rs | 190 ++++++++++++------ compiler/rustc_parse/src/parser/mod.rs | 1 + 3 files changed, 129 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index aeef6e62fda25..a2e40d3398a9a 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -303,13 +303,9 @@ impl<'a> Parser<'a> { None }; if let Some(attr) = attr { - // If we are currently capturing tokens, mark the location of this inner attribute. - // If capturing ends up creating a `LazyAttrTokenStream`, we will include - // this replace range with it, removing the inner attribute from the final - // `AttrTokenStream`. Inner attributes are stored in the parsed AST note. - // During macro expansion, they are selectively inserted back into the - // token stream (the first inner attribute is removed each time we invoke the - // corresponding macro). + // If we are currently capturing tokens (i.e. we are within a call to + // `Parser::collect_tokens_trailing_tokens`) record the token positions of this + // inner attribute, for possible later processing in a `LazyAttrTokenStream`. if let Capturing::Yes = self.capture_state.capturing { let end_pos = self.num_bump_calls; let range = start_pos..end_pos; @@ -463,7 +459,8 @@ impl<'a> Parser<'a> { } } -/// The attributes are complete if all attributes are either a doc comment or a builtin attribute other than `cfg_attr` +/// The attributes are complete if all attributes are either a doc comment or a +/// builtin attribute other than `cfg_attr`. pub fn is_complete(attrs: &[ast::Attribute]) -> bool { attrs.iter().all(|attr| { attr.is_doc_comment() diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 3b37a2dd9e46d..c56c46ec133a3 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -17,12 +17,12 @@ use std::{iter, mem}; /// /// This wrapper prevents direct access to the underlying `ast::AttrVec`. /// Parsing code can only get access to the underlying attributes -/// by passing an `AttrWrapper` to `collect_tokens_trailing_tokens`. +/// by passing an `AttrWrapper` to `collect_tokens_trailing_token`. /// This makes it difficult to accidentally construct an AST node /// (which stores an `ast::AttrVec`) without first collecting tokens. /// /// This struct has its own module, to ensure that the parser code -/// cannot directly access the `attrs` field +/// cannot directly access the `attrs` field. #[derive(Debug, Clone)] pub struct AttrWrapper { attrs: AttrVec, @@ -76,14 +76,13 @@ fn has_cfg_or_cfg_attr(attrs: &[Attribute]) -> bool { }) } -// Produces a `TokenStream` on-demand. Using `cursor_snapshot` -// and `num_calls`, we can reconstruct the `TokenStream` seen -// by the callback. This allows us to avoid producing a `TokenStream` -// if it is never needed - for example, a captured `macro_rules!` -// argument that is never passed to a proc macro. -// In practice token stream creation happens rarely compared to -// calls to `collect_tokens` (see some statistics in #78736), -// so we are doing as little up-front work as possible. +// From a value of this type we can reconstruct the `TokenStream` seen by the +// `f` callback passed to a call to `Parser::collect_tokens_trailing_token`, by +// replaying the getting of the tokens. This saves us producing a `TokenStream` +// if it is never needed, e.g. a captured `macro_rules!` argument that is never +// passed to a proc macro. In practice, token stream creation happens rarely +// compared to calls to `collect_tokens` (see some statistics in #78736) so we +// are doing as little up-front work as possible. // // This also makes `Parser` very cheap to clone, since // there is no intermediate collection buffer to clone. @@ -163,46 +162,55 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { } impl<'a> Parser<'a> { - /// Records all tokens consumed by the provided callback, - /// including the current token. These tokens are collected - /// into a `LazyAttrTokenStream`, and returned along with the first part of - /// the callback's result. The second (bool) part of the callback's result - /// indicates if an extra token should be captured, e.g. a comma or + /// Parses code with `f`. If appropriate, it records the tokens (in + /// `LazyAttrTokenStream` form) that were parsed in the result, accessible + /// via the `HasTokens` trait. The second (bool) part of the callback's + /// result indicates if an extra token should be captured, e.g. a comma or /// semicolon. /// /// The `attrs` passed in are in `AttrWrapper` form, which is opaque. The /// `AttrVec` within is passed to `f`. See the comment on `AttrWrapper` for /// details. /// - /// Note: If your callback consumes an opening delimiter - /// (including the case where you call `collect_tokens` - /// when the current token is an opening delimiter), - /// you must also consume the corresponding closing delimiter. + /// Note: If your callback consumes an opening delimiter (including the + /// case where `self.token` is an opening delimiter on entry to this + /// function), you must also consume the corresponding closing delimiter. + /// E.g. you can consume `something ([{ }])` or `([{}])`, but not `([{}]`. + /// This restriction isn't a problem in practice, because parsed AST items + /// always have matching delimiters. /// - /// That is, you can consume - /// `something ([{ }])` or `([{}])`, but not `([{}]` - /// - /// This restriction shouldn't be an issue in practice, - /// since this function is used to record the tokens for - /// a parsed AST item, which always has matching delimiters. + /// The following example code will be used to explain things in comments + /// below. It has an outer attribute and an inner attribute. Parsing it + /// involves two calls to this method, one of which is indirectly + /// recursive. + /// ```ignore + /// #[cfg_eval] // token pos + /// mod m { // 0.. 3 + /// #[cfg_attr(cond1, attr1)] // 3..12 + /// fn g() { // 12..17 + /// #![cfg_attr(cond2, attr2)] // 17..27 + /// let _x = 3; // 27..32 + /// } // 32..33 + /// } // 33..34 + /// ``` pub fn collect_tokens_trailing_token( &mut self, attrs: AttrWrapper, force_collect: ForceCollect, f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>, ) -> PResult<'a, R> { - // We only bail out when nothing could possibly observe the collected tokens: - // 1. We cannot be force collecting tokens (since force-collecting requires tokens - // by definition + // Skip collection when nothing could observe the collected tokens, i.e. + // all of the following conditions hold. + // - We are not force collecting tokens (because force collection + // requires tokens by definition). if matches!(force_collect, ForceCollect::No) - // None of our outer attributes can require tokens (e.g. a proc-macro) + // - None of our outer attributes require tokens. && attrs.is_complete() - // If our target supports custom inner attributes, then we cannot bail - // out early, since we may need to capture tokens for a custom inner attribute - // invocation. + // - Our target doesn't support custom inner attributes (custom + // inner attribute invocation might require token capturing). && !R::SUPPORTS_CUSTOM_INNER_ATTRS - // Never bail out early in `capture_cfg` mode, since there might be `#[cfg]` - // or `#[cfg_attr]` attributes. + // - We are not in `capture_cfg` mode (which requires tokens if + // the parsed node has `#[cfg]` or `#[cfg_attr]` attributes). && !self.capture_cfg { return Ok(f(self, attrs.attrs)?.0); @@ -214,6 +222,12 @@ impl<'a> Parser<'a> { let has_outer_attrs = !attrs.attrs.is_empty(); let replace_ranges_start = self.capture_state.replace_ranges.len(); + // We set and restore `Capturing::Yes` on either side of the call to + // `f`, so we can distinguish the outermost call to + // `collect_tokens_trailing_token` (e.g. parsing `m` in the example + // above) from any inner (indirectly recursive) calls (e.g. parsing `g` + // in the example above). This distinction is used below and in + // `Parser::parse_inner_attributes`. let (mut ret, capture_trailing) = { let prev_capturing = mem::replace(&mut self.capture_state.capturing, Capturing::Yes); let ret_and_trailing = f(self, attrs.attrs); @@ -221,37 +235,42 @@ impl<'a> Parser<'a> { ret_and_trailing? }; - // When we're not in `capture-cfg` mode, then bail out early if: - // 1. Our target doesn't support tokens at all (e.g we're parsing an `NtIdent`) - // so there's nothing for us to do. - // 2. Our target already has tokens set (e.g. we've parsed something - // like `#[my_attr] $item`). The actual parsing code takes care of - // prepending any attributes to the nonterminal, so we don't need to - // modify the already captured tokens. - // Note that this check is independent of `force_collect`- if we already - // have tokens, or can't even store them, then there's never a need to - // force collection of new tokens. + // When we're not in `capture_cfg` mode, then skip collecting and + // return early if either of the following conditions hold. + // - `None`: Our target doesn't support tokens at all (e.g. `NtIdent`). + // - `Some(Some(_))`: Our target already has tokens set (e.g. we've + // parsed something like `#[my_attr] $item`). The actual parsing code + // takes care of prepending any attributes to the nonterminal, so we + // don't need to modify the already captured tokens. + // + // Note that this check is independent of `force_collect`. There's no + // need to collect tokens when we don't support tokens or already have + // tokens. if !self.capture_cfg && matches!(ret.tokens_mut(), None | Some(Some(_))) { return Ok(ret); } - // This is very similar to the bail out check at the start of this function. - // Now that we've parsed an AST node, we have more information available. + // This is similar to the "skip collection" check at the start of this + // function, but now that we've parsed an AST node we have more + // information available. (If we return early here that means the + // setup, such as cloning the token cursor, was unnecessary. That's + // hard to avoid.) + // + // Skip collection when nothing could observe the collected tokens, i.e. + // all of the following conditions hold. + // - We are not force collecting tokens. if matches!(force_collect, ForceCollect::No) - // We now have inner attributes available, so this check is more precise - // than `attrs.is_complete()` at the start of the function. - // As a result, we don't need to check `R::SUPPORTS_CUSTOM_INNER_ATTRS` + // - None of our outer *or* inner attributes require tokens. + // (`attrs` was just outer attributes, but `ret.attrs()` is outer + // and inner attributes. That makes this check more precise than + // `attrs.is_complete()` at the start of the function, and we can + // skip the subsequent check on `R::SUPPORTS_CUSTOM_INNER_ATTRS`. && crate::parser::attr::is_complete(ret.attrs()) - // Subtle: We call `has_cfg_or_cfg_attr` with the attrs from `ret`. - // This ensures that we consider inner attributes (e.g. `#![cfg]`), - // which require us to have tokens available - // We also call `has_cfg_or_cfg_attr` at the beginning of this function, - // but we only bail out if there's no possibility of inner attributes - // (!R::SUPPORTS_CUSTOM_INNER_ATTRS) - // We only capture about `#[cfg]` or `#[cfg_attr]` in `capture_cfg` - // mode - during normal parsing, we don't need any special capturing - // for those attributes, since they're builtin. - && !(self.capture_cfg && has_cfg_or_cfg_attr(ret.attrs())) + // - We are not in `capture_cfg` mode, or we are but there are no + // `#[cfg]` or `#[cfg_attr]` attributes. (During normal + // non-`capture_cfg` parsing, we don't need any special capturing + // for those attributes, because they're builtin.) + && (!self.capture_cfg || !has_cfg_or_cfg_attr(ret.attrs())) { return Ok(ret); } @@ -273,7 +292,10 @@ impl<'a> Parser<'a> { let num_calls = end_pos - start_pos; - // Take the captured ranges for any inner attributes that we parsed. + // Take the captured ranges for any inner attributes that we parsed in + // `Parser::parse_inner_attributes`, and pair them in a `ReplaceRange` + // with `None`, which means the relevant tokens will be removed. (More + // details below.) let mut inner_attr_replace_ranges = Vec::new(); for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { @@ -289,9 +311,9 @@ impl<'a> Parser<'a> { if replace_ranges_start == replace_ranges_end && inner_attr_replace_ranges.is_empty() { Box::new([]) } else { - // Grab any replace ranges that occur *inside* the current AST node. - // We will perform the actual replacement when we convert the `LazyAttrTokenStream` - // to an `AttrTokenStream`. + // Grab any replace ranges that occur *inside* the current AST node. We will + // perform the actual replacement only when we convert the `LazyAttrTokenStream` to + // an `AttrTokenStream`. self.capture_state.replace_ranges[replace_ranges_start..replace_ranges_end] .iter() .cloned() @@ -300,6 +322,28 @@ impl<'a> Parser<'a> { .collect() }; + // What is the status here when parsing the example code at the top of this method? + // + // When parsing `g`: + // - `start_pos..end_pos` is `12..33` (`fn g { ... }`, excluding the outer attr). + // - `inner_attr_replace_ranges` has one entry (`5..15`, when counting from `fn`), to + // delete the inner attr's tokens. + // - This entry is put into the lazy tokens for `g`, i.e. deleting the inner attr from + // those tokens (if they get evaluated). + // - Those lazy tokens are also put into an `AttrsTarget` that is appended to `self`'s + // replace ranges at the bottom of this function, for processing when parsing `m`. + // - `replace_ranges_start..replace_ranges_end` is empty. + // + // When parsing `m`: + // - `start_pos..end_pos` is `0..34` (`mod m`, excluding the `#[cfg_eval]` attribute). + // - `inner_attr_replace_ranges` is empty. + // - `replace_range_start..replace_ranges_end` has two entries. + // - One to delete the inner attribute (`17..27`), obtained when parsing `g` (see above). + // - One `AttrsTarget` (added below when parsing `g`) to replace all of `g` (`3..33`, + // including its outer attribute), with: + // - `attrs`: includes the outer and the inner attr. + // - `tokens`: lazy tokens for `g` (with its inner attr deleted). + let tokens = LazyAttrTokenStream::new(LazyAttrTokenStreamImpl { start_token, num_calls, @@ -323,15 +367,27 @@ impl<'a> Parser<'a> { { assert!(!self.break_last_token, "Should not have unglued last token with cfg attr"); - // Replace the entire AST node that we just parsed, including attributes, with - // `target`. If this AST node is inside an item that has `#[derive]`, then this will - // allow us to cfg-expand this AST node. + // What is the status here when parsing the example code at the top of this method? + // + // When parsing `g`, we add two entries: + // - The `start_pos..end_pos` (`3..33`) entry has a new `AttrsTarget` with: + // - `attrs`: includes the outer and the inner attr. + // - `tokens`: lazy tokens for `g` (with its inner attr deleted). + // - `inner_attr_replace_ranges` contains the one entry to delete the inner attr's + // tokens (`17..27`). + // + // When parsing `m`, we do nothing here. + + // Set things up so that the entire AST node that we just parsed, including attributes, + // will be replaced with `target` in the lazy token stream. This will allow us to + // cfg-expand this AST node. let start_pos = if has_outer_attrs { attrs.start_pos } else { start_pos }; let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens }; self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); } else if matches!(self.capture_state.capturing, Capturing::No) { - // Only clear the ranges once we've finished capturing entirely. + // Only clear the ranges once we've finished capturing entirely, i.e. we've finished + // the outermost call to this method. self.capture_state.replace_ranges.clear(); self.capture_state.inner_attr_ranges.clear(); } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 33900503520d2..06545e85dd161 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -221,6 +221,7 @@ enum Capturing { Yes, } +// This state is used by `Parser::collect_tokens_trailing_token`. #[derive(Clone, Debug)] struct CaptureState { capturing: Capturing, From 4158a1c48fb19bfce31e4001593faa2353b7201b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 14:02:37 +1000 Subject: [PATCH 14/15] Only check `force_collect` in `collect_tokens_trailing_token`. There are three places where we currently check `force_collect` and call `collect_tokens_no_attrs` for `ForceCollect::Yes` and a vanilla parsing function for `ForceCollect::No`. But we can instead just pass in `force_collect` and let `collect_tokens_trailing_token` do the appropriate thing. --- compiler/rustc_parse/src/parser/attr.rs | 12 +++------ compiler/rustc_parse/src/parser/stmt.rs | 33 +++++++++++++------------ 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 347cef86d981d..0922150870a74 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -251,13 +251,12 @@ impl<'a> Parser<'a> { pub fn parse_attr_item(&mut self, force_collect: ForceCollect) -> PResult<'a, ast::AttrItem> { maybe_whole!(self, NtMeta, |attr| attr.into_inner()); - let do_parse = |this: &mut Self| { + let do_parse = |this: &mut Self, _empty_attrs| { let is_unsafe = this.eat_keyword(kw::Unsafe); let unsafety = if is_unsafe { let unsafe_span = this.prev_token.span; this.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span); this.expect(&token::OpenDelim(Delimiter::Parenthesis))?; - ast::Safety::Unsafe(unsafe_span) } else { ast::Safety::Default @@ -268,13 +267,10 @@ impl<'a> Parser<'a> { if is_unsafe { this.expect(&token::CloseDelim(Delimiter::Parenthesis))?; } - Ok(ast::AttrItem { unsafety, path, args, tokens: None }) + Ok((ast::AttrItem { unsafety, path, args, tokens: None }, false)) }; - // Attr items don't have attributes - match force_collect { - ForceCollect::Yes => self.collect_tokens_no_attrs(do_parse), - ForceCollect::No => do_parse(self), - } + // Attr items don't have attributes. + self.collect_tokens_trailing_token(AttrWrapper::empty(), force_collect, do_parse) } /// Parses attributes that appear after the opening of an item. These should diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 69dd3a3d65aac..d8de7c1bfa1c5 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -99,17 +99,17 @@ impl<'a> Parser<'a> { // or `auto trait` items. We aim to parse an arbitrary path `a::b` but not something // that starts like a path (1 token), but it fact not a path. // Also, we avoid stealing syntax from `parse_item_`. - match force_collect { - ForceCollect::Yes => { - self.collect_tokens_no_attrs(|this| this.parse_stmt_path_start(lo, attrs))? + let stmt = self.collect_tokens_trailing_token( + AttrWrapper::empty(), + force_collect, + |this, _empty_attrs| Ok((this.parse_stmt_path_start(lo, attrs)?, false)), + ); + match stmt { + Ok(stmt) => stmt, + Err(mut err) => { + self.suggest_add_missing_let_for_stmt(&mut err); + return Err(err); } - ForceCollect::No => match self.parse_stmt_path_start(lo, attrs) { - Ok(stmt) => stmt, - Err(mut err) => { - self.suggest_add_missing_let_for_stmt(&mut err); - return Err(err); - } - }, } } else if let Some(item) = self.parse_item_common( attrs.clone(), @@ -126,12 +126,13 @@ impl<'a> Parser<'a> { self.mk_stmt(lo, StmtKind::Empty) } else if self.token != token::CloseDelim(Delimiter::Brace) { // Remainder are line-expr stmts. - let e = match force_collect { - ForceCollect::Yes => self.collect_tokens_no_attrs(|this| { - this.parse_expr_res(Restrictions::STMT_EXPR, attrs) - })?, - ForceCollect::No => self.parse_expr_res(Restrictions::STMT_EXPR, attrs)?, - }; + let e = self.collect_tokens_trailing_token( + AttrWrapper::empty(), + force_collect, + |this, _empty_attrs| { + Ok((this.parse_expr_res(Restrictions::STMT_EXPR, attrs)?, false)) + }, + )?; if matches!(e.kind, ExprKind::Assign(..)) && self.eat_keyword(kw::Else) { let bl = self.parse_block()?; // Destructuring assignment ... else. From 0c932b763d24a9f57ce3285927ea54308b7be902 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 18 Jul 2024 19:49:32 -0700 Subject: [PATCH 15/15] Rearrange sidebar modnav builder to more logical order --- src/librustdoc/html/static/js/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 116ce615d8c55..9506bc9ed220b 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -531,13 +531,13 @@ function preLoadCss(cssUrl) { link.href = path; link.textContent = name; const li = document.createElement("li"); - li.appendChild(link); - ul.appendChild(li); // Don't "optimize" this to just use `path`. // We want the browser to normalize this into an absolute URL. if (link.href === current_page) { li.classList.add("current"); } + li.appendChild(link); + ul.appendChild(li); } sidebar.appendChild(h3); sidebar.appendChild(ul);