From 5834772177540912b53dc4f19413a4ba3c804ea4 Mon Sep 17 00:00:00 2001 From: Veera Date: Thu, 4 Jul 2024 19:16:47 -0400 Subject: [PATCH 01/12] 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/12] 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 9dc08e30bd0668396dee18eacbcb96dc6d0ddda3 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 11 Jul 2024 14:01:37 -0400 Subject: [PATCH 03/12] Rewrite and rename issue-22131 to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/issue-22131/Makefile | 8 -------- .../foo.rs | 0 .../run-make/rustdoc-cfgspec-parsing/rmake.rs | 19 +++++++++++++++++++ 4 files changed, 19 insertions(+), 9 deletions(-) delete mode 100644 tests/run-make/issue-22131/Makefile rename tests/run-make/{issue-22131 => rustdoc-cfgspec-parsing}/foo.rs (100%) create mode 100644 tests/run-make/rustdoc-cfgspec-parsing/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 745f00c4f5276..7a787255da6d4 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -38,7 +38,6 @@ run-make/interdependent-c-libraries/Makefile run-make/issue-107094/Makefile run-make/issue-14698/Makefile run-make/issue-15460/Makefile -run-make/issue-22131/Makefile run-make/issue-26006/Makefile run-make/issue-28595/Makefile run-make/issue-33329/Makefile diff --git a/tests/run-make/issue-22131/Makefile b/tests/run-make/issue-22131/Makefile deleted file mode 100644 index 4f33a4659cc1e..0000000000000 --- a/tests/run-make/issue-22131/Makefile +++ /dev/null @@ -1,8 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: foo.rs - $(RUSTC) --cfg 'feature="bar"' --crate-type lib foo.rs - $(RUSTDOC) --test --cfg 'feature="bar"' \ - -L $(TMPDIR) foo.rs |\ - $(CGREP) 'foo.rs - foo (line 1) ... ok' diff --git a/tests/run-make/issue-22131/foo.rs b/tests/run-make/rustdoc-cfgspec-parsing/foo.rs similarity index 100% rename from tests/run-make/issue-22131/foo.rs rename to tests/run-make/rustdoc-cfgspec-parsing/foo.rs diff --git a/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs b/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs new file mode 100644 index 0000000000000..7cb66575c5481 --- /dev/null +++ b/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs @@ -0,0 +1,19 @@ +// A rustdoc bug caused the `feature=bar` syntax for the cfg flag to be interpreted +// wrongly, with `feature=bar` instead of just `bar` being understood as the feature name. +// After this was fixed in #22135, this test checks that this bug does not make a resurgence. +// See https://github.com/rust-lang/rust/issues/22131 + +//FIXME(Oneirical): try test-various + +use run_make_support::{rustc, rustdoc}; + +fn main() { + rustc().cfg(r#"feature="bar""#).crate_type("lib").input("foo.rs").run(); + rustdoc() + .arg("--test") + .arg("--cfg") + .arg(r#"feature="bar""#) + .input("foo.rs") + .run() + .assert_stdout_contains("foo.rs - foo (line 1) ... ok"); +} From fc4404c230c2a7a818e73255d3ceb877b30b592f Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 11 Jul 2024 14:48:33 -0400 Subject: [PATCH 04/12] Rewrite and rename issue-26006 to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../invalid-symlink-search-path/in/bar/lib.rs | 5 +++ .../invalid-symlink-search-path/in/foo/lib.rs | 3 ++ .../invalid-symlink-search-path/rmake.rs | 33 +++++++++++++++++++ tests/run-make/issue-26006/Makefile | 17 ---------- tests/run-make/issue-26006/in/libc/lib.rs | 3 -- tests/run-make/issue-26006/in/time/lib.rs | 4 --- .../run-make/rustdoc-cfgspec-parsing/rmake.rs | 6 ++-- 8 files changed, 45 insertions(+), 27 deletions(-) create mode 100644 tests/run-make/invalid-symlink-search-path/in/bar/lib.rs create mode 100644 tests/run-make/invalid-symlink-search-path/in/foo/lib.rs create mode 100644 tests/run-make/invalid-symlink-search-path/rmake.rs delete mode 100644 tests/run-make/issue-26006/Makefile delete mode 100644 tests/run-make/issue-26006/in/libc/lib.rs delete mode 100644 tests/run-make/issue-26006/in/time/lib.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 7a787255da6d4..c49684b45cbcc 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -38,7 +38,6 @@ run-make/interdependent-c-libraries/Makefile run-make/issue-107094/Makefile run-make/issue-14698/Makefile run-make/issue-15460/Makefile -run-make/issue-26006/Makefile run-make/issue-28595/Makefile run-make/issue-33329/Makefile run-make/issue-35164/Makefile diff --git a/tests/run-make/invalid-symlink-search-path/in/bar/lib.rs b/tests/run-make/invalid-symlink-search-path/in/bar/lib.rs new file mode 100644 index 0000000000000..58c3e33bb6ca6 --- /dev/null +++ b/tests/run-make/invalid-symlink-search-path/in/bar/lib.rs @@ -0,0 +1,5 @@ +extern crate foo; + +pub fn main() { + let _ = foo::hello_world(); +} diff --git a/tests/run-make/invalid-symlink-search-path/in/foo/lib.rs b/tests/run-make/invalid-symlink-search-path/in/foo/lib.rs new file mode 100644 index 0000000000000..07b66f8ca454a --- /dev/null +++ b/tests/run-make/invalid-symlink-search-path/in/foo/lib.rs @@ -0,0 +1,3 @@ +pub fn hello_world() -> i32 { + 42 +} diff --git a/tests/run-make/invalid-symlink-search-path/rmake.rs b/tests/run-make/invalid-symlink-search-path/rmake.rs new file mode 100644 index 0000000000000..ed2cd9c4bd265 --- /dev/null +++ b/tests/run-make/invalid-symlink-search-path/rmake.rs @@ -0,0 +1,33 @@ +// In this test, the symlink created is invalid (valid relative to the root, but not +// relatively to where it is located), and used to cause an internal +// compiler error (ICE) when passed as a library search path. This was fixed in #26044, +// and this test checks that the invalid symlink is instead simply ignored. +// See https://github.com/rust-lang/rust/issues/26006 + +//@ needs-symlink +//Reason: symlink requires elevated permission in Windows + +use run_make_support::{rfs, rustc}; + +fn main() { + // We create two libs: `bar` which depends on `foo`. We need to compile `foo` first. + rfs::create_dir("out"); + rfs::create_dir("out/foo"); + rustc() + .input("in/foo/lib.rs") + .crate_name("foo") + .crate_type("lib") + .metadata("foo") + .output("out/foo/libfoo.rlib") + .run(); + rfs::create_dir("out/bar"); + rfs::create_dir("out/bar/deps"); + rfs::create_symlink("out/foo/libfoo.rlib", "out/bar/deps/libfoo.rlib"); + // Check that the invalid symlink does not cause an ICE + rustc() + .input("in/bar/lib.rs") + .library_search_path("dependency=out/bar/deps") + .run_fail() + .assert_exit_code(1) + .assert_stderr_not_contains("internal compiler error"); +} diff --git a/tests/run-make/issue-26006/Makefile b/tests/run-make/issue-26006/Makefile deleted file mode 100644 index b679c121530ad..0000000000000 --- a/tests/run-make/issue-26006/Makefile +++ /dev/null @@ -1,17 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -# ignore-windows - -OUT := $(TMPDIR)/out - -all: time - -time: libc - mkdir -p $(OUT)/time $(OUT)/time/deps - ln -sf $(OUT)/libc/liblibc.rlib $(OUT)/time/deps/ - $(RUSTC) in/time/lib.rs -Ldependency=$(OUT)/time/deps/ - -libc: - mkdir -p $(OUT)/libc - $(RUSTC) in/libc/lib.rs --crate-name=libc -Cmetadata=foo -o $(OUT)/libc/liblibc.rlib diff --git a/tests/run-make/issue-26006/in/libc/lib.rs b/tests/run-make/issue-26006/in/libc/lib.rs deleted file mode 100644 index bad155a99bd61..0000000000000 --- a/tests/run-make/issue-26006/in/libc/lib.rs +++ /dev/null @@ -1,3 +0,0 @@ -#![crate_type = "rlib"] - -pub fn something() {} diff --git a/tests/run-make/issue-26006/in/time/lib.rs b/tests/run-make/issue-26006/in/time/lib.rs deleted file mode 100644 index 51ed27cd713f9..0000000000000 --- a/tests/run-make/issue-26006/in/time/lib.rs +++ /dev/null @@ -1,4 +0,0 @@ -#![feature(rustc_private)] -extern crate libc; - -fn main() {} diff --git a/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs b/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs index 7cb66575c5481..9c8c71b19a620 100644 --- a/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs +++ b/tests/run-make/rustdoc-cfgspec-parsing/rmake.rs @@ -3,9 +3,10 @@ // After this was fixed in #22135, this test checks that this bug does not make a resurgence. // See https://github.com/rust-lang/rust/issues/22131 -//FIXME(Oneirical): try test-various +//@ ignore-cross-compile +// Reason: rustdoc fails to find the "foo" crate -use run_make_support::{rustc, rustdoc}; +use run_make_support::{cwd, rustc, rustdoc}; fn main() { rustc().cfg(r#"feature="bar""#).crate_type("lib").input("foo.rs").run(); @@ -13,6 +14,7 @@ fn main() { .arg("--test") .arg("--cfg") .arg(r#"feature="bar""#) + .library_search_path(cwd()) .input("foo.rs") .run() .assert_stdout_contains("foo.rs - foo (line 1) ... ok"); From 2e1e6274306bed67291c1eefc7332d0bf972e6ac Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 18 Jul 2024 12:06:21 -0700 Subject: [PATCH 05/12] 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 06/12] 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 07/12] 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 e69ff1c1065933b3737e662f8f237ffbee964755 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 02:21:39 +1000 Subject: [PATCH 08/12] 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 09/12] 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 10/12] 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 4158a1c48fb19bfce31e4001593faa2353b7201b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 14:02:37 +1000 Subject: [PATCH 11/12] 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 12/12] 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);