From 12a833acfd963beeabbc17bf000783168a0a032b Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Tue, 22 Oct 2024 15:49:58 +0100 Subject: [PATCH 01/11] Add lint rule for `#[deprecated]` on `use` --- compiler/rustc_passes/src/stability.rs | 3 +++ library/core/src/alloc/mod.rs | 5 ----- library/std/src/collections/mod.rs | 3 +-- tests/ui/deprecation/deprecated_use.fixed | 10 ++++++++++ tests/ui/deprecation/deprecated_use.rs | 11 +++++++++++ tests/ui/deprecation/deprecated_use.stderr | 10 ++++++++++ tests/ui/imports/unused-import-issue-87973.fixed | 1 + tests/ui/imports/unused-import-issue-87973.rs | 1 + tests/ui/imports/unused-import-issue-87973.stderr | 2 +- 9 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 tests/ui/deprecation/deprecated_use.fixed create mode 100644 tests/ui/deprecation/deprecated_use.rs create mode 100644 tests/ui/deprecation/deprecated_use.stderr diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 751c87a9fe519..9296907465810 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -366,6 +366,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> { kind = AnnotationKind::DeprecationProhibited; const_stab_inherit = InheritConstStability::Yes; } + hir::ItemKind::Use(_, _) => { + kind = AnnotationKind::DeprecationProhibited; + } hir::ItemKind::Struct(ref sd, _) => { if let Some(ctor_def_id) = sd.ctor_def_id() { self.annotate( diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index aa841db045ce7..f17687ed6e41b 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -10,11 +10,6 @@ pub use self::global::GlobalAlloc; #[stable(feature = "alloc_layout", since = "1.28.0")] pub use self::layout::Layout; #[stable(feature = "alloc_layout", since = "1.28.0")] -#[deprecated( - since = "1.52.0", - note = "Name does not follow std convention, use LayoutError", - suggestion = "LayoutError" -)] #[allow(deprecated, deprecated_in_future)] pub use self::layout::LayoutErr; #[stable(feature = "alloc_layout_error", since = "1.50.0")] diff --git a/library/std/src/collections/mod.rs b/library/std/src/collections/mod.rs index 889ed3c538035..ba57c5eb3530d 100644 --- a/library/std/src/collections/mod.rs +++ b/library/std/src/collections/mod.rs @@ -433,8 +433,7 @@ pub use self::hash_map::HashMap; #[doc(inline)] pub use self::hash_set::HashSet; #[stable(feature = "rust1", since = "1.0.0")] -// FIXME(#82080) The deprecation here is only theoretical, and does not actually produce a warning. -#[deprecated(note = "moved to `std::ops::Bound`", since = "1.26.0")] +// FIXME(#82080) This has moved but #[deprecated] on `use` is unsupported. #[doc(hidden)] pub use crate::ops::Bound; diff --git a/tests/ui/deprecation/deprecated_use.fixed b/tests/ui/deprecation/deprecated_use.fixed new file mode 100644 index 0000000000000..09d6869fd1a20 --- /dev/null +++ b/tests/ui/deprecation/deprecated_use.fixed @@ -0,0 +1,10 @@ +//@ run-rustfix +mod a_module { + pub struct ActiveType; +} + +//~^ ERROR this `#[deprecated]` annotation has no effect +pub use a_module::ActiveType; + +fn main() { +} diff --git a/tests/ui/deprecation/deprecated_use.rs b/tests/ui/deprecation/deprecated_use.rs new file mode 100644 index 0000000000000..1712b40675988 --- /dev/null +++ b/tests/ui/deprecation/deprecated_use.rs @@ -0,0 +1,11 @@ +//@ run-rustfix +mod a_module { + pub struct ActiveType; +} + +#[deprecated] +//~^ ERROR this `#[deprecated]` annotation has no effect +pub use a_module::ActiveType; + +fn main() { +} diff --git a/tests/ui/deprecation/deprecated_use.stderr b/tests/ui/deprecation/deprecated_use.stderr new file mode 100644 index 0000000000000..31f75237902ba --- /dev/null +++ b/tests/ui/deprecation/deprecated_use.stderr @@ -0,0 +1,10 @@ +error: this `#[deprecated]` annotation has no effect + --> $DIR/deprecated_use.rs:6:1 + | +LL | #[deprecated] + | ^^^^^^^^^^^^^ help: remove the unnecessary deprecation attribute + | + = note: `#[deny(useless_deprecated)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/imports/unused-import-issue-87973.fixed b/tests/ui/imports/unused-import-issue-87973.fixed index d1167d7d4861a..402655e4fbc88 100644 --- a/tests/ui/imports/unused-import-issue-87973.fixed +++ b/tests/ui/imports/unused-import-issue-87973.fixed @@ -1,5 +1,6 @@ //@ run-rustfix #![deny(unused_imports)] +#![allow(useless_deprecated)] // Check that attributes get removed too. See #87973. //~^ ERROR unused import diff --git a/tests/ui/imports/unused-import-issue-87973.rs b/tests/ui/imports/unused-import-issue-87973.rs index 1b016ff814c6b..0186ddd5ec75f 100644 --- a/tests/ui/imports/unused-import-issue-87973.rs +++ b/tests/ui/imports/unused-import-issue-87973.rs @@ -1,5 +1,6 @@ //@ run-rustfix #![deny(unused_imports)] +#![allow(useless_deprecated)] // Check that attributes get removed too. See #87973. #[deprecated] diff --git a/tests/ui/imports/unused-import-issue-87973.stderr b/tests/ui/imports/unused-import-issue-87973.stderr index a43e92b145802..3628f319411ca 100644 --- a/tests/ui/imports/unused-import-issue-87973.stderr +++ b/tests/ui/imports/unused-import-issue-87973.stderr @@ -1,5 +1,5 @@ error: unused import: `std::fs` - --> $DIR/unused-import-issue-87973.rs:8:5 + --> $DIR/unused-import-issue-87973.rs:9:5 | LL | use std::fs; | ^^^^^^^ From ca664bb95dfaa13c2f2a5ce90898a992b050d511 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Tue, 22 Oct 2024 16:10:55 +0100 Subject: [PATCH 02/11] Merge `#[deprecated]` on `use` test with existing `deprecation-sanity` UI test --- tests/ui/deprecation/deprecated_use.fixed | 10 ---------- tests/ui/deprecation/deprecated_use.rs | 11 ----------- tests/ui/deprecation/deprecated_use.stderr | 10 ---------- tests/ui/deprecation/deprecation-sanity.rs | 7 +++++++ tests/ui/deprecation/deprecation-sanity.stderr | 8 +++++++- 5 files changed, 14 insertions(+), 32 deletions(-) delete mode 100644 tests/ui/deprecation/deprecated_use.fixed delete mode 100644 tests/ui/deprecation/deprecated_use.rs delete mode 100644 tests/ui/deprecation/deprecated_use.stderr diff --git a/tests/ui/deprecation/deprecated_use.fixed b/tests/ui/deprecation/deprecated_use.fixed deleted file mode 100644 index 09d6869fd1a20..0000000000000 --- a/tests/ui/deprecation/deprecated_use.fixed +++ /dev/null @@ -1,10 +0,0 @@ -//@ run-rustfix -mod a_module { - pub struct ActiveType; -} - -//~^ ERROR this `#[deprecated]` annotation has no effect -pub use a_module::ActiveType; - -fn main() { -} diff --git a/tests/ui/deprecation/deprecated_use.rs b/tests/ui/deprecation/deprecated_use.rs deleted file mode 100644 index 1712b40675988..0000000000000 --- a/tests/ui/deprecation/deprecated_use.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@ run-rustfix -mod a_module { - pub struct ActiveType; -} - -#[deprecated] -//~^ ERROR this `#[deprecated]` annotation has no effect -pub use a_module::ActiveType; - -fn main() { -} diff --git a/tests/ui/deprecation/deprecated_use.stderr b/tests/ui/deprecation/deprecated_use.stderr deleted file mode 100644 index 31f75237902ba..0000000000000 --- a/tests/ui/deprecation/deprecated_use.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: this `#[deprecated]` annotation has no effect - --> $DIR/deprecated_use.rs:6:1 - | -LL | #[deprecated] - | ^^^^^^^^^^^^^ help: remove the unnecessary deprecation attribute - | - = note: `#[deny(useless_deprecated)]` on by default - -error: aborting due to 1 previous error - diff --git a/tests/ui/deprecation/deprecation-sanity.rs b/tests/ui/deprecation/deprecation-sanity.rs index 9ea75b68f81ce..27a8fc3580ae0 100644 --- a/tests/ui/deprecation/deprecation-sanity.rs +++ b/tests/ui/deprecation/deprecation-sanity.rs @@ -39,4 +39,11 @@ impl Default for X { } } +mod inner { + pub struct Y; +} + +#[deprecated] //~ ERROR this `#[deprecated]` annotation has no effect +pub use inner::Y; + fn main() { } diff --git a/tests/ui/deprecation/deprecation-sanity.stderr b/tests/ui/deprecation/deprecation-sanity.stderr index 383212ad9b4b8..e05fbc0aa14cd 100644 --- a/tests/ui/deprecation/deprecation-sanity.stderr +++ b/tests/ui/deprecation/deprecation-sanity.stderr @@ -68,7 +68,13 @@ LL | #[deprecated = "hello"] | = note: `#[deny(useless_deprecated)]` on by default -error: aborting due to 10 previous errors +error: this `#[deprecated]` annotation has no effect + --> $DIR/deprecation-sanity.rs:46:1 + | +LL | #[deprecated] + | ^^^^^^^^^^^^^ help: remove the unnecessary deprecation attribute + +error: aborting due to 11 previous errors Some errors have detailed explanations: E0538, E0539, E0541, E0565. For more information about an error, try `rustc --explain E0538`. From 2ce89ee1c420798aca7ed17359f3363e86799719 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 9 Dec 2024 14:45:16 +0000 Subject: [PATCH 03/11] Add a test for mangling of named constants in const generics and array length --- tests/ui/symbol-names/types.legacy.stderr | 20 ++++++++++++++++++- tests/ui/symbol-names/types.rs | 11 ++++++++++ tests/ui/symbol-names/types.v0.stderr | 20 ++++++++++++++++++- .../symbol-names/types.verbose-legacy.stderr | 20 ++++++++++++++++++- 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/tests/ui/symbol-names/types.legacy.stderr b/tests/ui/symbol-names/types.legacy.stderr index 87c3acae0bd6c..8ccf5317fdd14 100644 --- a/tests/ui/symbol-names/types.legacy.stderr +++ b/tests/ui/symbol-names/types.legacy.stderr @@ -502,5 +502,23 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(a::b::Type<[u8; _]>::h[HASH]) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(a::b::Type<[u8; _]>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors diff --git a/tests/ui/symbol-names/types.rs b/tests/ui/symbol-names/types.rs index 7ed19e0e5a825..bedfcd80a04a8 100644 --- a/tests/ui/symbol-names/types.rs +++ b/tests/ui/symbol-names/types.rs @@ -266,6 +266,17 @@ pub fn b() { //[v0]~| ERROR ::b::Type<[_; _]>>) //[v0]~| ERROR demangling-alt(>) impl Type<[T; N]> {} + + const ZERO: usize = 0; + + #[rustc_symbol_name] + //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$ + //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; _]>:: + //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; _]>) + //[v0]~^^^^ ERROR symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) + //[v0]~| ERROR ::b::Type<[u8; 0usize]>>) + //[v0]~| ERROR demangling-alt(>) + impl Type<[u8; ZERO]> {} } fn main() {} diff --git a/tests/ui/symbol-names/types.v0.stderr b/tests/ui/symbol-names/types.v0.stderr index 58680e002022a..90012a2dcf72f 100644 --- a/tests/ui/symbol-names/types.v0.stderr +++ b/tests/ui/symbol-names/types.v0.stderr @@ -502,5 +502,23 @@ error: demangling-alt(>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors diff --git a/tests/ui/symbol-names/types.verbose-legacy.stderr b/tests/ui/symbol-names/types.verbose-legacy.stderr index 87c3acae0bd6c..8ccf5317fdd14 100644 --- a/tests/ui/symbol-names/types.verbose-legacy.stderr +++ b/tests/ui/symbol-names/types.verbose-legacy.stderr @@ -502,5 +502,23 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 84 previous errors +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling(a::b::Type<[u8; _]>::h[HASH]) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: demangling-alt(a::b::Type<[u8; _]>) + --> $DIR/types.rs:272:5 + | +LL | #[rustc_symbol_name] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 87 previous errors From 9ecdc54d82fe1e797103f8ac750baf9f8f861bec Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 9 Dec 2024 14:45:16 +0000 Subject: [PATCH 04/11] Try to evaluate constants in legacy mangling --- compiler/rustc_symbol_mangling/src/legacy.rs | 29 ++++++++++++++++++- tests/ui/symbol-names/types.legacy.stderr | 6 ++-- tests/ui/symbol-names/types.rs | 6 ++-- .../symbol-names/types.verbose-legacy.stderr | 6 ++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_symbol_mangling/src/legacy.rs b/compiler/rustc_symbol_mangling/src/legacy.rs index 59ccd6dff8588..0d6d8488a23ca 100644 --- a/compiler/rustc_symbol_mangling/src/legacy.rs +++ b/compiler/rustc_symbol_mangling/src/legacy.rs @@ -2,7 +2,7 @@ use std::fmt::{self, Write}; use std::mem::{self, discriminant}; use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher}; -use rustc_hir::def_id::CrateNum; +use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; use rustc_middle::bug; use rustc_middle::ty::print::{PrettyPrinter, Print, PrintError, Printer}; @@ -378,6 +378,33 @@ impl<'tcx> Printer<'tcx> for SymbolPrinter<'tcx> { Ok(()) } } + + fn print_impl_path( + &mut self, + impl_def_id: DefId, + args: &'tcx [GenericArg<'tcx>], + mut self_ty: Ty<'tcx>, + mut impl_trait_ref: Option>, + ) -> Result<(), PrintError> { + let mut typing_env = ty::TypingEnv::post_analysis(self.tcx, impl_def_id); + if !args.is_empty() { + typing_env.param_env = + ty::EarlyBinder::bind(typing_env.param_env).instantiate(self.tcx, args); + } + + match &mut impl_trait_ref { + Some(impl_trait_ref) => { + assert_eq!(impl_trait_ref.self_ty(), self_ty); + *impl_trait_ref = self.tcx.normalize_erasing_regions(typing_env, *impl_trait_ref); + self_ty = impl_trait_ref.self_ty(); + } + None => { + self_ty = self.tcx.normalize_erasing_regions(typing_env, self_ty); + } + } + + self.default_print_impl_path(impl_def_id, args, self_ty, impl_trait_ref) + } } impl<'tcx> PrettyPrinter<'tcx> for SymbolPrinter<'tcx> { diff --git a/tests/ui/symbol-names/types.legacy.stderr b/tests/ui/symbol-names/types.legacy.stderr index 8ccf5317fdd14..c368b31860989 100644 --- a/tests/ui/symbol-names/types.legacy.stderr +++ b/tests/ui/symbol-names/types.legacy.stderr @@ -502,19 +502,19 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$17h[HASH]E) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling(a::b::Type<[u8; _]>::h[HASH]) +error: demangling(a::b::Type<[u8; 0]>::h[HASH]) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling-alt(a::b::Type<[u8; _]>) +error: demangling-alt(a::b::Type<[u8; 0]>) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] diff --git a/tests/ui/symbol-names/types.rs b/tests/ui/symbol-names/types.rs index bedfcd80a04a8..38735e1aa5098 100644 --- a/tests/ui/symbol-names/types.rs +++ b/tests/ui/symbol-names/types.rs @@ -270,9 +270,9 @@ pub fn b() { const ZERO: usize = 0; #[rustc_symbol_name] - //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$ - //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; _]>:: - //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; _]>) + //[legacy,verbose-legacy]~^ ERROR symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$ + //[legacy,verbose-legacy]~| ERROR demangling(a::b::Type<[u8; 0]>:: + //[legacy,verbose-legacy]~| ERROR demangling-alt(a::b::Type<[u8; 0]>) //[v0]~^^^^ ERROR symbol-name(_RMsq_NvCsCRATE_HASH_1a1bINtB_4TypeAhj0_E) //[v0]~| ERROR ::b::Type<[u8; 0usize]>>) //[v0]~| ERROR demangling-alt(>) diff --git a/tests/ui/symbol-names/types.verbose-legacy.stderr b/tests/ui/symbol-names/types.verbose-legacy.stderr index 8ccf5317fdd14..c368b31860989 100644 --- a/tests/ui/symbol-names/types.verbose-legacy.stderr +++ b/tests/ui/symbol-names/types.verbose-legacy.stderr @@ -502,19 +502,19 @@ error: demangling-alt(a::b::Type<[T; N]>) LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$_$u5d$$GT$17h[HASH]E) +error: symbol-name(_ZN1a1b35Type$LT$$u5b$u8$u3b$$u20$0$u5d$$GT$17h[HASH]E) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling(a::b::Type<[u8; _]>::h[HASH]) +error: demangling(a::b::Type<[u8; 0]>::h[HASH]) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] | ^^^^^^^^^^^^^^^^^^^^ -error: demangling-alt(a::b::Type<[u8; _]>) +error: demangling-alt(a::b::Type<[u8; 0]>) --> $DIR/types.rs:272:5 | LL | #[rustc_symbol_name] From 3f52583c6a85f243e8444145e533fa799a40f777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Dec 2024 21:02:00 +0000 Subject: [PATCH 05/11] Add test for resolve errors caused by mod with parse errors --- tests/ui/resolve/parse-error-resolve.rs | 7 +++++ tests/ui/resolve/parse-error-resolve.stderr | 35 +++++++++++++++++++++ tests/ui/resolve/parse_error.rs | 5 +++ tests/ui/resolve/parse_error.stderr | 13 ++++++++ 4 files changed, 60 insertions(+) create mode 100644 tests/ui/resolve/parse-error-resolve.rs create mode 100644 tests/ui/resolve/parse-error-resolve.stderr create mode 100644 tests/ui/resolve/parse_error.rs create mode 100644 tests/ui/resolve/parse_error.stderr diff --git a/tests/ui/resolve/parse-error-resolve.rs b/tests/ui/resolve/parse-error-resolve.rs new file mode 100644 index 0000000000000..b5b6637bcfa74 --- /dev/null +++ b/tests/ui/resolve/parse-error-resolve.rs @@ -0,0 +1,7 @@ +mod parse_error; +use parse_error::Canonical; //~ ERROR E0432 + +fn main() { + let _ = "" + 1; //~ ERROR E0369 + parse_error::Canonical.foo(); //~ ERROR E0425 +} diff --git a/tests/ui/resolve/parse-error-resolve.stderr b/tests/ui/resolve/parse-error-resolve.stderr new file mode 100644 index 0000000000000..72456b797d181 --- /dev/null +++ b/tests/ui/resolve/parse-error-resolve.stderr @@ -0,0 +1,35 @@ +error: expected one of `+`, `,`, `::`, `=`, or `>`, found `From` + --> $DIR/parse_error.rs:2:46 + | +LL | impl> From for Canonical { + | ^^^^ expected one of `+`, `,`, `::`, `=`, or `>` + | +help: you might have meant to end the type parameters here + | +LL | impl>> From for Canonical { + | + + +error[E0432]: unresolved import `parse_error::Canonical` + --> $DIR/parse-error-resolve.rs:2:5 + | +LL | use parse_error::Canonical; + | ^^^^^^^^^^^^^^^^^^^^^^ no `Canonical` in `parse_error` + +error[E0425]: cannot find value `Canonical` in module `parse_error` + --> $DIR/parse-error-resolve.rs:6:18 + | +LL | parse_error::Canonical.foo(); + | ^^^^^^^^^ not found in `parse_error` + +error[E0369]: cannot add `{integer}` to `&str` + --> $DIR/parse-error-resolve.rs:5:16 + | +LL | let _ = "" + 1; + | -- ^ - {integer} + | | + | &str + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0369, E0425, E0432. +For more information about an error, try `rustc --explain E0369`. diff --git a/tests/ui/resolve/parse_error.rs b/tests/ui/resolve/parse_error.rs new file mode 100644 index 0000000000000..4cd025e1edf5b --- /dev/null +++ b/tests/ui/resolve/parse_error.rs @@ -0,0 +1,5 @@ +struct Canonical; +impl> From for Canonical { //~ ERROR expected + fn foo() {} +} +fn bar() {} diff --git a/tests/ui/resolve/parse_error.stderr b/tests/ui/resolve/parse_error.stderr new file mode 100644 index 0000000000000..f764f08875ff7 --- /dev/null +++ b/tests/ui/resolve/parse_error.stderr @@ -0,0 +1,13 @@ +error: expected one of `+`, `,`, `::`, `=`, or `>`, found `From` + --> $DIR/parse_error.rs:2:46 + | +LL | impl> From for Canonical { + | ^^^^ expected one of `+`, `,`, `::`, `=`, or `>` + | +help: you might have meant to end the type parameters here + | +LL | impl>> From for Canonical { + | + + +error: aborting due to 1 previous error + From 69fb612608f6b3d7f21a4efae869e69ca949955f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Dec 2024 21:19:08 +0000 Subject: [PATCH 06/11] Keep track of parse errors in `mod`s and don't emit resolve errors for paths involving them When we expand a `mod foo;` and parse `foo.rs`, we now track whether that file had an unrecovered parse error that reached the end of the file. If so, we keep that information around. When resolving a path like `foo::bar`, we do not emit any errors for "`bar` not found in `foo`", as we know that the parse error might have caused `bar` to not be parsed and accounted for. When this happens in an existing project, every path referencing `foo` would be an irrelevant compile error. Instead, we now skip emitting anything until `foo.rs` is fixed. Tellingly enough, we didn't have any test for errors caused by `mod` expansion. Fix #97734. --- compiler/rustc_ast/src/ast.rs | 2 +- compiler/rustc_ast/src/mut_visit.rs | 7 +- compiler/rustc_ast/src/visit.rs | 2 +- compiler/rustc_ast_lowering/src/item.rs | 2 +- .../rustc_ast_passes/src/ast_validation.rs | 2 +- .../rustc_builtin_macros/src/test_harness.rs | 6 +- compiler/rustc_expand/src/expand.rs | 32 +++++--- compiler/rustc_expand/src/module.rs | 10 ++- compiler/rustc_lint/src/builtin.rs | 2 +- compiler/rustc_parse/src/parser/item.rs | 2 +- .../rustc_resolve/src/build_reduced_graph.rs | 6 +- compiler/rustc_resolve/src/diagnostics.rs | 2 +- compiler/rustc_resolve/src/ident.rs | 82 ++++++++++++------- compiler/rustc_resolve/src/imports.rs | 1 + compiler/rustc_resolve/src/late.rs | 7 ++ compiler/rustc_resolve/src/lib.rs | 6 ++ compiler/rustc_resolve/src/macros.rs | 2 +- .../clippy/clippy_lints/src/duplicate_mod.rs | 2 +- .../clippy_lints/src/excessive_nesting.rs | 2 +- .../clippy/clippy_utils/src/ast_utils.rs | 2 +- src/tools/rustfmt/src/items.rs | 2 +- src/tools/rustfmt/src/modules.rs | 7 +- src/tools/rustfmt/src/visitor.rs | 2 +- tests/ui/parser/circular_modules_main.stderr | 19 +---- tests/ui/resolve/parse-error-resolve.rs | 2 +- tests/ui/resolve/parse-error-resolve.stderr | 10 +-- 26 files changed, 128 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 650525a2f520e..a219f1dc03299 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2877,7 +2877,7 @@ pub enum ModKind { /// or with definition outlined to a separate file `mod foo;` and already loaded from it. /// The inner span is from the first token past `{` to the last token until `}`, /// or from the first to the last token in the loaded file. - Loaded(ThinVec>, Inline, ModSpans), + Loaded(ThinVec>, Inline, ModSpans, Result<(), ErrorGuaranteed>), /// Module with definition outlined to a separate file `mod foo;` but not yet loaded from it. Unloaded, } diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 2c09059fe1904..0e299c604790c 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1212,7 +1212,12 @@ impl WalkItemKind for ItemKind { ItemKind::Mod(safety, mod_kind) => { visit_safety(vis, safety); match mod_kind { - ModKind::Loaded(items, _inline, ModSpans { inner_span, inject_use_span }) => { + ModKind::Loaded( + items, + _inline, + ModSpans { inner_span, inject_use_span }, + _, + ) => { items.flat_map_in_place(|item| vis.flat_map_item(item)); vis.visit_span(inner_span); vis.visit_span(inject_use_span); diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index a7f7c37693a84..a6232b6dfaf86 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -380,7 +380,7 @@ impl WalkItemKind for ItemKind { try_visit!(visitor.visit_fn(kind, span, id)); } ItemKind::Mod(_unsafety, mod_kind) => match mod_kind { - ModKind::Loaded(items, _inline, _inner_span) => { + ModKind::Loaded(items, _inline, _inner_span, _) => { walk_list!(visitor, visit_item, items); } ModKind::Unloaded => {} diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 7d6c41992eb09..d63131eacb555 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -238,7 +238,7 @@ impl<'hir> LoweringContext<'_, 'hir> { }) } ItemKind::Mod(_, mod_kind) => match mod_kind { - ModKind::Loaded(items, _, spans) => { + ModKind::Loaded(items, _, spans, _) => { hir::ItemKind::Mod(self.lower_mod(items, spans)) } ModKind::Unloaded => panic!("`mod` items should have been loaded by now"), diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 64e91c91e2ca3..290c2e5297014 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -1029,7 +1029,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.dcx().emit_err(errors::UnsafeItem { span, kind: "module" }); } // Ensure that `path` attributes on modules are recorded as used (cf. issue #35584). - if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _)) + if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _)) && !attr::contains_name(&item.attrs, sym::path) { self.check_mod_file_item_asciionly(item.ident); diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index b2048c534a48a..e7ff65e08f9cd 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -141,8 +141,10 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { // We don't want to recurse into anything other than mods, since // mods or tests inside of functions will break things - if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. })) = - item.kind + if let ast::ItemKind::Mod( + _, + ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. }, _), + ) = item.kind { let prev_tests = mem::take(&mut self.tests); walk_item_kind( diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 6a6496f982702..690e080fbfce5 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -723,7 +723,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { item_inner.kind, ItemKind::Mod( _, - ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _), + ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _, _), ) ) => { @@ -889,7 +889,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { fn visit_item(&mut self, item: &'ast ast::Item) { match &item.kind { ItemKind::Mod(_, mod_kind) - if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _)) => + if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _)) => { feature_err( self.sess, @@ -1195,7 +1195,7 @@ impl InvocationCollectorNode for P { let ecx = &mut collector.cx; let (file_path, dir_path, dir_ownership) = match mod_kind { - ModKind::Loaded(_, inline, _) => { + ModKind::Loaded(_, inline, _, _) => { // Inline `mod foo { ... }`, but we still need to push directories. let (dir_path, dir_ownership) = mod_dir_path( ecx.sess, @@ -1217,15 +1217,21 @@ impl InvocationCollectorNode for P { ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. let old_attrs_len = attrs.len(); - let ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } = - parse_external_mod( - ecx.sess, - ident, - span, - &ecx.current_expansion.module, - ecx.current_expansion.dir_ownership, - &mut attrs, - ); + let ParsedExternalMod { + items, + spans, + file_path, + dir_path, + dir_ownership, + had_parse_error, + } = parse_external_mod( + ecx.sess, + ident, + span, + &ecx.current_expansion.module, + ecx.current_expansion.dir_ownership, + &mut attrs, + ); if let Some(lint_store) = ecx.lint_store { lint_store.pre_expansion_lint( @@ -1239,7 +1245,7 @@ impl InvocationCollectorNode for P { ); } - *mod_kind = ModKind::Loaded(items, Inline::No, spans); + *mod_kind = ModKind::Loaded(items, Inline::No, spans, had_parse_error); node.attrs = attrs; if node.attrs.len() > old_attrs_len { // If we loaded an out-of-line module and added some inner attributes, diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 614f52bbd2851..85ea42e78ad49 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -37,6 +37,7 @@ pub(crate) struct ParsedExternalMod { pub file_path: PathBuf, pub dir_path: PathBuf, pub dir_ownership: DirOwnership, + pub had_parse_error: Result<(), ErrorGuaranteed>, } pub enum ModError<'a> { @@ -74,14 +75,17 @@ pub(crate) fn parse_external_mod( attrs.extend(inner_attrs); (items, inner_span, mp.file_path) }; + // (1) ...instead, we return a dummy module. - let (items, spans, file_path) = - result.map_err(|err| err.report(sess, span)).unwrap_or_default(); + let ((items, spans, file_path), had_parse_error) = match result { + Err(err) => (Default::default(), Err(err.report(sess, span))), + Ok(result) => (result, Ok(())), + }; // Extract the directory path for submodules of the module. let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); - ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } + ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership, had_parse_error } } pub(crate) fn mod_dir_path( diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 093cc16fb4c18..3543784bc7283 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3038,7 +3038,7 @@ impl EarlyLintPass for SpecialModuleName { for item in &krate.items { if let ast::ItemKind::Mod( _, - ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _), + ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _, _), ) = item.kind { if item.attrs.iter().any(|a| a.has_name(sym::path)) { diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 58b4bf8980b1f..e27fc963eb901 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -45,7 +45,7 @@ impl<'a> Parser<'a> { let (inner_attrs, items, inner_span) = self.parse_mod(&token::CloseDelim(Delimiter::Brace))?; attrs.extend(inner_attrs); - ModKind::Loaded(items, Inline::Yes, inner_span) + ModKind::Loaded(items, Inline::Yes, inner_span, Ok(())) }; Ok((id, ItemKind::Mod(safety, mod_kind))) } diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 293cee500bbd8..924b8afa329f2 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -770,7 +770,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ); } - ItemKind::Mod(..) => { + ItemKind::Mod(.., ref mod_kind) => { let module = self.r.new_module( Some(parent), ModuleKind::Def(def_kind, def_id, ident.name), @@ -781,6 +781,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ); self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion)); + if let ast::ModKind::Loaded(_, _, _, Err(_)) = mod_kind { + self.r.mods_with_parse_errors.insert(def_id); + } + // Descend into the module. self.parent_scope.module = module; } diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 4c76617a3918d..368eb3c26c7b3 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -3056,7 +3056,7 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder { fn visit_item(&mut self, item: &'tcx ast::Item) { if self.target_module == item.id { - if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans)) = &item.kind { + if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans, _)) = &item.kind { let inject = mod_spans.inject_use_span; if is_span_suitable_for_use_injection(inject) { self.first_legal_span = Some(inject); diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 466e190028ac9..5906a682f3e1a 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -1428,6 +1428,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_import: Option>, ) -> PathResult<'ra> { let mut module = None; + let mut module_had_parse_errors = false; let mut allow_super = true; let mut second_binding = None; @@ -1471,9 +1472,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { continue; } } - return PathResult::failed(ident, false, finalize.is_some(), module, || { - ("there are too many leading `super` keywords".to_string(), None) - }); + return PathResult::failed( + ident, + false, + finalize.is_some(), + module_had_parse_errors, + module, + || ("there are too many leading `super` keywords".to_string(), None), + ); } if segment_idx == 0 { if name == kw::SelfLower { @@ -1511,19 +1517,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Report special messages for path segment keywords in wrong positions. if ident.is_path_segment_keyword() && segment_idx != 0 { - return PathResult::failed(ident, false, finalize.is_some(), module, || { - let name_str = if name == kw::PathRoot { - "crate root".to_string() - } else { - format!("`{name}`") - }; - let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot { - format!("global paths cannot start with {name_str}") - } else { - format!("{name_str} in paths can only be used in start position") - }; - (label, None) - }); + return PathResult::failed( + ident, + false, + finalize.is_some(), + module_had_parse_errors, + module, + || { + let name_str = if name == kw::PathRoot { + "crate root".to_string() + } else { + format!("`{name}`") + }; + let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot { + format!("global paths cannot start with {name_str}") + } else { + format!("{name_str} in paths can only be used in start position") + }; + (label, None) + }, + ); } let binding = if let Some(module) = module { @@ -1589,6 +1602,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res); if let Some(next_module) = binding.module() { + if self.mods_with_parse_errors.contains(&next_module.def_id()) { + module_had_parse_errors = true; + } module = Some(ModuleOrUniformRoot::Module(next_module)); record_segment_res(self, res); } else if res == Res::ToolMod && !is_last && opt_ns.is_some() { @@ -1614,6 +1630,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident, is_last, finalize.is_some(), + module_had_parse_errors, module, || { let label = format!( @@ -1637,19 +1654,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - return PathResult::failed(ident, is_last, finalize.is_some(), module, || { - self.report_path_resolution_error( - path, - opt_ns, - parent_scope, - ribs, - ignore_binding, - ignore_import, - module, - segment_idx, - ident, - ) - }); + return PathResult::failed( + ident, + is_last, + finalize.is_some(), + module_had_parse_errors, + module, + || { + self.report_path_resolution_error( + path, + opt_ns, + parent_scope, + ribs, + ignore_binding, + ignore_import, + module, + segment_idx, + ident, + ) + }, + ); } } } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 51fbcb8ebb805..5b8eefbb88db9 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -898,6 +898,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { label, suggestion, module, + error_implied_by_parse_error: _, } => { if no_ambiguity { assert!(import.imported_module.get().is_none()); diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 789d74876f722..5cb7bb9b461c4 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -4395,6 +4395,12 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => { PartialRes::new(module.res().unwrap()) } + // A part of this path references a `mod` that had a parse error. To avoid resolution + // errors for each reference to that module, we don't emit an error for them until the + // `mod` is fixed. this can have a significant cascade effect. + PathResult::Failed { error_implied_by_parse_error: true, .. } => { + PartialRes::new(Res::Err) + } // In `a(::assoc_item)*` `a` cannot be a module. If `a` does resolve to a module we // don't report an error right away, but try to fallback to a primitive type. // So, we are still able to successfully resolve something like @@ -4443,6 +4449,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { suggestion, module, segment_name, + error_implied_by_parse_error: _, } => { return Err(respan(span, ResolutionError::FailedToResolve { segment: Some(segment_name), diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index ca4adce37ce03..94adfcd3b915d 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -450,6 +450,7 @@ enum PathResult<'ra> { module: Option>, /// The segment name of target segment_name: Symbol, + error_implied_by_parse_error: bool, }, } @@ -458,6 +459,7 @@ impl<'ra> PathResult<'ra> { ident: Ident, is_error_from_last_segment: bool, finalize: bool, + error_implied_by_parse_error: bool, module: Option>, label_and_suggestion: impl FnOnce() -> (String, Option), ) -> PathResult<'ra> { @@ -470,6 +472,7 @@ impl<'ra> PathResult<'ra> { suggestion, is_error_from_last_segment, module, + error_implied_by_parse_error, } } } @@ -1198,6 +1201,8 @@ pub struct Resolver<'ra, 'tcx> { /// This is the `Span` where an `extern crate foo;` suggestion would be inserted, if `foo` /// could be a crate that wasn't imported. For diagnostics use only. current_crate_outer_attr_insert_span: Span, + + mods_with_parse_errors: FxHashSet, } /// This provides memory for the rest of the crate. The `'ra` lifetime that is @@ -1543,6 +1548,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { impl_unexpanded_invocations: Default::default(), impl_binding_keys: Default::default(), current_crate_outer_attr_insert_span, + mods_with_parse_errors: Default::default(), }; let root_parent_scope = ParentScope::module(graph_root, &resolver); diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 0b4d0e04c295c..669a9c2428f8d 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -166,7 +166,7 @@ fn soft_custom_inner_attributes_gate(path: &ast::Path, invoc: &Invocation) -> bo [seg1, seg2] if seg1.ident.name == sym::rustfmt && seg2.ident.name == sym::skip => { if let InvocationKind::Attr { item, .. } = &invoc.kind { if let Annotatable::Item(item) = item { - if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, _)) = item.kind { + if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, _, _)) = item.kind { return true; } } diff --git a/src/tools/clippy/clippy_lints/src/duplicate_mod.rs b/src/tools/clippy/clippy_lints/src/duplicate_mod.rs index ed27e38ef2d19..1dac7b971f957 100644 --- a/src/tools/clippy/clippy_lints/src/duplicate_mod.rs +++ b/src/tools/clippy/clippy_lints/src/duplicate_mod.rs @@ -63,7 +63,7 @@ impl_lint_pass!(DuplicateMod => [DUPLICATE_MOD]); impl EarlyLintPass for DuplicateMod { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, mod_spans)) = &item.kind + if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, mod_spans, _)) = &item.kind && let FileName::Real(real) = cx.sess().source_map().span_to_filename(mod_spans.inner_span) && let Some(local_path) = real.into_local_path() && let Ok(absolute_path) = local_path.canonicalize() diff --git a/src/tools/clippy/clippy_lints/src/excessive_nesting.rs b/src/tools/clippy/clippy_lints/src/excessive_nesting.rs index ffc76366983aa..dfea40db182f2 100644 --- a/src/tools/clippy/clippy_lints/src/excessive_nesting.rs +++ b/src/tools/clippy/clippy_lints/src/excessive_nesting.rs @@ -163,7 +163,7 @@ impl Visitor<'_> for NestingVisitor<'_, '_> { } match &item.kind { - ItemKind::Trait(_) | ItemKind::Impl(_) | ItemKind::Mod(.., ModKind::Loaded(_, Inline::Yes, _)) => { + ItemKind::Trait(_) | ItemKind::Impl(_) | ItemKind::Mod(.., ModKind::Loaded(_, Inline::Yes, _, _)) => { self.nest_level += 1; if !self.check_indent(item.span, item.id) { diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index 620a9b56eb557..12074dd16e604 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -379,7 +379,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { (Mod(lu, lmk), Mod(ru, rmk)) => { lu == ru && match (lmk, rmk) { - (ModKind::Loaded(litems, linline, _), ModKind::Loaded(ritems, rinline, _)) => { + (ModKind::Loaded(litems, linline, _, _), ModKind::Loaded(ritems, rinline, _, _)) => { linline == rinline && over(litems, ritems, |l, r| eq_item(l, r, eq_item_kind)) }, (ModKind::Unloaded, ModKind::Unloaded) => true, diff --git a/src/tools/rustfmt/src/items.rs b/src/tools/rustfmt/src/items.rs index 18932587f1fb0..c3debc2f4f045 100644 --- a/src/tools/rustfmt/src/items.rs +++ b/src/tools/rustfmt/src/items.rs @@ -3597,7 +3597,7 @@ pub(crate) fn rewrite_extern_crate( pub(crate) fn is_mod_decl(item: &ast::Item) -> bool { !matches!( item.kind, - ast::ItemKind::Mod(_, ast::ModKind::Loaded(_, ast::Inline::Yes, _)) + ast::ItemKind::Mod(_, ast::ModKind::Loaded(_, ast::Inline::Yes, _, _)) ) } diff --git a/src/tools/rustfmt/src/modules.rs b/src/tools/rustfmt/src/modules.rs index 493b04f16c6f4..a40ee7f66a978 100644 --- a/src/tools/rustfmt/src/modules.rs +++ b/src/tools/rustfmt/src/modules.rs @@ -316,12 +316,11 @@ impl<'ast, 'psess, 'c> ModResolver<'ast, 'psess> { self.directory = directory; } match (sub_mod.ast_mod_kind, sub_mod.items) { - (Some(Cow::Borrowed(ast::ModKind::Loaded(items, _, _))), _) => { + (Some(Cow::Borrowed(ast::ModKind::Loaded(items, _, _, _))), _) => { self.visit_mod_from_ast(items) } - (Some(Cow::Owned(ast::ModKind::Loaded(items, _, _))), _) | (_, Cow::Owned(items)) => { - self.visit_mod_outside_ast(items) - } + (Some(Cow::Owned(ast::ModKind::Loaded(items, _, _, _))), _) + | (_, Cow::Owned(items)) => self.visit_mod_outside_ast(items), (_, _) => Ok(()), } } diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index 9b116b620b79d..805e13b780367 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -927,7 +927,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let ident_str = rewrite_ident(&self.get_context(), ident).to_owned(); self.push_str(&ident_str); - if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans) = mod_kind { + if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans, _) = mod_kind { let ast::ModSpans { inner_span, inject_use_span: _, diff --git a/tests/ui/parser/circular_modules_main.stderr b/tests/ui/parser/circular_modules_main.stderr index 2de70789358b0..e1780fe9fd9d0 100644 --- a/tests/ui/parser/circular_modules_main.stderr +++ b/tests/ui/parser/circular_modules_main.stderr @@ -4,22 +4,5 @@ error: circular modules: $DIR/circular_modules_main.rs -> $DIR/circular_modules_ LL | mod circular_modules_main; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0425]: cannot find function `hi_str` in module `circular_modules_main` - --> $DIR/circular_modules_hello.rs:7:43 - | -LL | println!("{}", circular_modules_main::hi_str()); - | ^^^^^^ not found in `circular_modules_main` - | -help: consider importing this function - | -LL + use hi_str; - | -help: if you import `hi_str`, refer to it directly - | -LL - println!("{}", circular_modules_main::hi_str()); -LL + println!("{}", hi_str()); - | - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0425`. diff --git a/tests/ui/resolve/parse-error-resolve.rs b/tests/ui/resolve/parse-error-resolve.rs index b5b6637bcfa74..9cb58f5512892 100644 --- a/tests/ui/resolve/parse-error-resolve.rs +++ b/tests/ui/resolve/parse-error-resolve.rs @@ -3,5 +3,5 @@ use parse_error::Canonical; //~ ERROR E0432 fn main() { let _ = "" + 1; //~ ERROR E0369 - parse_error::Canonical.foo(); //~ ERROR E0425 + parse_error::Canonical.foo(); // ok, `parse_error.rs` had parse errors } diff --git a/tests/ui/resolve/parse-error-resolve.stderr b/tests/ui/resolve/parse-error-resolve.stderr index 72456b797d181..c05a9c6dafbda 100644 --- a/tests/ui/resolve/parse-error-resolve.stderr +++ b/tests/ui/resolve/parse-error-resolve.stderr @@ -15,12 +15,6 @@ error[E0432]: unresolved import `parse_error::Canonical` LL | use parse_error::Canonical; | ^^^^^^^^^^^^^^^^^^^^^^ no `Canonical` in `parse_error` -error[E0425]: cannot find value `Canonical` in module `parse_error` - --> $DIR/parse-error-resolve.rs:6:18 - | -LL | parse_error::Canonical.foo(); - | ^^^^^^^^^ not found in `parse_error` - error[E0369]: cannot add `{integer}` to `&str` --> $DIR/parse-error-resolve.rs:5:16 | @@ -29,7 +23,7 @@ LL | let _ = "" + 1; | | | &str -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0369, E0425, E0432. +Some errors have detailed explanations: E0369, E0432. For more information about an error, try `rustc --explain E0369`. From 27420c69d830d7a32544dd42f40c757d9cb4e095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Dec 2024 21:47:06 +0000 Subject: [PATCH 07/11] Silence `use foo::Bar;` error if `Bar` isn't found in `foo` and `foo.rs` has parse errors --- compiler/rustc_resolve/src/imports.rs | 7 ++++++- tests/ui/resolve/parse-error-resolve.rs | 2 +- tests/ui/resolve/parse-error-resolve.stderr | 11 ++--------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 5b8eefbb88db9..2ed3f4d2c4fbb 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -670,9 +670,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { fn throw_unresolved_import_error( &mut self, - errors: Vec<(Import<'_>, UnresolvedImportError)>, + mut errors: Vec<(Import<'_>, UnresolvedImportError)>, glob_error: bool, ) { + errors.retain(|(_import, err)| match err.module { + // Skip `use` errors for `use foo::Bar;` if `foo.rs` has unrecovered parse errors. + Some(def_id) if self.mods_with_parse_errors.contains(&def_id) => false, + _ => true, + }); if errors.is_empty() { return; } diff --git a/tests/ui/resolve/parse-error-resolve.rs b/tests/ui/resolve/parse-error-resolve.rs index 9cb58f5512892..1e0772648afec 100644 --- a/tests/ui/resolve/parse-error-resolve.rs +++ b/tests/ui/resolve/parse-error-resolve.rs @@ -1,5 +1,5 @@ mod parse_error; -use parse_error::Canonical; //~ ERROR E0432 +use parse_error::Canonical; // ok, `parse_error.rs` had parse errors fn main() { let _ = "" + 1; //~ ERROR E0369 diff --git a/tests/ui/resolve/parse-error-resolve.stderr b/tests/ui/resolve/parse-error-resolve.stderr index c05a9c6dafbda..30475aa0ee63e 100644 --- a/tests/ui/resolve/parse-error-resolve.stderr +++ b/tests/ui/resolve/parse-error-resolve.stderr @@ -9,12 +9,6 @@ help: you might have meant to end the type parameters here LL | impl>> From for Canonical { | + -error[E0432]: unresolved import `parse_error::Canonical` - --> $DIR/parse-error-resolve.rs:2:5 - | -LL | use parse_error::Canonical; - | ^^^^^^^^^^^^^^^^^^^^^^ no `Canonical` in `parse_error` - error[E0369]: cannot add `{integer}` to `&str` --> $DIR/parse-error-resolve.rs:5:16 | @@ -23,7 +17,6 @@ LL | let _ = "" + 1; | | | &str -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0369, E0432. -For more information about an error, try `rustc --explain E0369`. +For more information about this error, try `rustc --explain E0369`. From 724052f65342f2c11a6ab20559a897eadc325d8b Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 12 Dec 2024 15:38:50 +0300 Subject: [PATCH 08/11] validate `--skip` and `--exclude` paths Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 28 ++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index b06147055f2a7..1d17e8987beda 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1314,7 +1314,33 @@ impl Config { // Set flags. config.paths = std::mem::take(&mut flags.paths); - config.skip = flags.skip.into_iter().chain(flags.exclude).collect(); + config.skip = flags + .skip + .into_iter() + .chain(flags.exclude) + .map(|p| { + let p = if cfg!(windows) { + PathBuf::from(p.to_str().unwrap().replace('/', "\\")) + } else { + p + }; + + // Jump to top-level project path to support passing paths + // from sub directories. + let top_level_path = config.src.join(&p); + assert!( + config.src.join(&top_level_path).exists(), + "{} does not exist.", + top_level_path.display() + ); + + // Never return top-level path here as it would break `--skip` + // logic on rustc's internal test framework which is utilized + // by compiletest. + p + }) + .collect(); + config.include_default_paths = flags.include_default_paths; config.rustc_error_format = flags.rustc_error_format; config.json_output = flags.json_output; From 3d2e36cfe210bf0f29a5010440b0ffcb2f901663 Mon Sep 17 00:00:00 2001 From: Manuel Drehwald Date: Thu, 12 Dec 2024 13:33:48 -0500 Subject: [PATCH 09/11] upstream rustc_codegen_llvm changes for enzyme/autodiff --- .../rustc_ast/src/expand/autodiff_attrs.rs | 19 +- compiler/rustc_codegen_gcc/src/lib.rs | 9 + compiler/rustc_codegen_llvm/messages.ftl | 4 + compiler/rustc_codegen_llvm/src/back/lto.rs | 9 +- compiler/rustc_codegen_llvm/src/back/write.rs | 134 ++++++++- compiler/rustc_codegen_llvm/src/builder.rs | 264 +++++++++++++++++- compiler/rustc_codegen_llvm/src/errors.rs | 8 + compiler/rustc_codegen_llvm/src/lib.rs | 18 +- .../rustc_codegen_llvm/src/llvm/enzyme_ffi.rs | 38 +++ compiler/rustc_codegen_llvm/src/llvm/mod.rs | 7 + compiler/rustc_codegen_ssa/src/back/lto.rs | 19 ++ .../rustc_codegen_ssa/src/traits/write.rs | 7 + .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 52 ++++ 13 files changed, 561 insertions(+), 27 deletions(-) create mode 100644 compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs diff --git a/compiler/rustc_ast/src/expand/autodiff_attrs.rs b/compiler/rustc_ast/src/expand/autodiff_attrs.rs index 05714731b9d4d..7ef8bc1797384 100644 --- a/compiler/rustc_ast/src/expand/autodiff_attrs.rs +++ b/compiler/rustc_ast/src/expand/autodiff_attrs.rs @@ -6,7 +6,6 @@ use std::fmt::{self, Display, Formatter}; use std::str::FromStr; -use crate::expand::typetree::TypeTree; use crate::expand::{Decodable, Encodable, HashStable_Generic}; use crate::ptr::P; use crate::{Ty, TyKind}; @@ -79,10 +78,6 @@ pub struct AutoDiffItem { /// The name of the function being generated pub target: String, pub attrs: AutoDiffAttrs, - /// Describe the memory layout of input types - pub inputs: Vec, - /// Describe the memory layout of the output type - pub output: TypeTree, } #[derive(Clone, Eq, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)] pub struct AutoDiffAttrs { @@ -262,22 +257,14 @@ impl AutoDiffAttrs { !matches!(self.mode, DiffMode::Error | DiffMode::Source) } - pub fn into_item( - self, - source: String, - target: String, - inputs: Vec, - output: TypeTree, - ) -> AutoDiffItem { - AutoDiffItem { source, target, inputs, output, attrs: self } + pub fn into_item(self, source: String, target: String) -> AutoDiffItem { + AutoDiffItem { source, target, attrs: self } } } impl fmt::Display for AutoDiffItem { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Differentiating {} -> {}", self.source, self.target)?; - write!(f, " with attributes: {:?}", self.attrs)?; - write!(f, " with inputs: {:?}", self.inputs)?; - write!(f, " with output: {:?}", self.output) + write!(f, " with attributes: {:?}", self.attrs) } } diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index 452e92bffa235..096e9a5961794 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -93,6 +93,7 @@ use gccjit::{CType, Context, OptimizationLevel}; #[cfg(feature = "master")] use gccjit::{TargetInfo, Version}; use rustc_ast::expand::allocator::AllocatorKind; +use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule}; use rustc_codegen_ssa::back::write::{ CodegenContext, FatLtoInput, ModuleConfig, TargetMachineFactoryFn, @@ -439,6 +440,14 @@ impl WriteBackendMethods for GccCodegenBackend { ) -> Result, FatalError> { back::write::link(cgcx, dcx, modules) } + fn autodiff( + _cgcx: &CodegenContext, + _module: &ModuleCodegen, + _diff_fncs: Vec, + _config: &ModuleConfig, + ) -> Result<(), FatalError> { + unimplemented!() + } } /// This is the entrypoint for a hot plugged rustc_codegen_gccjit diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl index 63c64269eb805..3982c37528df8 100644 --- a/compiler/rustc_codegen_llvm/messages.ftl +++ b/compiler/rustc_codegen_llvm/messages.ftl @@ -1,3 +1,5 @@ +codegen_llvm_autodiff_without_lto = using the autodiff feature requires using fat-lto + codegen_llvm_copy_bitcode = failed to copy bitcode to object file: {$err} codegen_llvm_dynamic_linking_with_lto = @@ -47,6 +49,8 @@ codegen_llvm_parse_bitcode_with_llvm_err = failed to parse bitcode for LTO modul codegen_llvm_parse_target_machine_config = failed to parse target machine config to target machine: {$error} +codegen_llvm_prepare_autodiff = failed to prepare autodiff: src: {$src}, target: {$target}, {$error} +codegen_llvm_prepare_autodiff_with_llvm_err = failed to prepare autodiff: {$llvm_err}, src: {$src}, target: {$target}, {$error} codegen_llvm_prepare_thin_lto_context = failed to prepare thin LTO context codegen_llvm_prepare_thin_lto_context_with_llvm_err = failed to prepare thin LTO context: {$llvm_err} diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 48beb9be2b2a1..769ec03e8dbdb 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -604,7 +604,14 @@ pub(crate) fn run_pass_manager( debug!("running the pass manager"); let opt_stage = if thin { llvm::OptStage::ThinLTO } else { llvm::OptStage::FatLTO }; let opt_level = config.opt_level.unwrap_or(config::OptLevel::No); - unsafe { write::llvm_optimize(cgcx, dcx, module, config, opt_level, opt_stage) }?; + + // If this rustc version was build with enzyme/autodiff enabled, and if users applied the + // `#[autodiff]` macro at least once, then we will later call llvm_optimize a second time. + let first_run = true; + debug!("running llvm pm opt pipeline"); + unsafe { + write::llvm_optimize(cgcx, dcx, module, config, opt_level, opt_stage, first_run)?; + } debug!("lto done"); Ok(()) } diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index a65ae4df1e378..2fc0d2bddd38b 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -8,6 +8,7 @@ use libc::{c_char, c_int, c_void, size_t}; use llvm::{ LLVMRustLLVMHasZlibCompressionForDebugSymbols, LLVMRustLLVMHasZstdCompressionForDebugSymbols, }; +use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::link::ensure_removed; use rustc_codegen_ssa::back::versioned_llvm_target; use rustc_codegen_ssa::back::write::{ @@ -28,7 +29,7 @@ use rustc_session::config::{ use rustc_span::InnerSpan; use rustc_span::symbol::sym; use rustc_target::spec::{CodeModel, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel}; -use tracing::debug; +use tracing::{debug, trace}; use crate::back::lto::ThinBuffer; use crate::back::owned_target_machine::OwnedTargetMachine; @@ -517,9 +518,38 @@ pub(crate) unsafe fn llvm_optimize( config: &ModuleConfig, opt_level: config::OptLevel, opt_stage: llvm::OptStage, + skip_size_increasing_opts: bool, ) -> Result<(), FatalError> { - let unroll_loops = - opt_level != config::OptLevel::Size && opt_level != config::OptLevel::SizeMin; + // Enzyme: + // The whole point of compiler based AD is to differentiate optimized IR instead of unoptimized + // source code. However, benchmarks show that optimizations increasing the code size + // tend to reduce AD performance. Therefore deactivate them before AD, then differentiate the code + // and finally re-optimize the module, now with all optimizations available. + // FIXME(ZuseZ4): In a future update we could figure out how to only optimize individual functions getting + // differentiated. + + let unroll_loops; + let vectorize_slp; + let vectorize_loop; + + // When we build rustc with enzyme/autodiff support, we want to postpone size-increasing + // optimizations until after differentiation. FIXME(ZuseZ4): Before shipping on nightly, + // we should make this more granular, or at least check that the user has at least one autodiff + // call in their code, to justify altering the compilation pipeline. + if skip_size_increasing_opts && cfg!(llvm_enzyme) { + unroll_loops = false; + vectorize_slp = false; + vectorize_loop = false; + } else { + unroll_loops = + opt_level != config::OptLevel::Size && opt_level != config::OptLevel::SizeMin; + vectorize_slp = config.vectorize_slp; + vectorize_loop = config.vectorize_loop; + } + trace!( + "Enzyme: Running with unroll_loops: {}, vectorize_slp: {}, vectorize_loop: {}", + unroll_loops, vectorize_slp, vectorize_loop + ); let using_thin_buffers = opt_stage == llvm::OptStage::PreLinkThinLTO || config.bitcode_needed(); let pgo_gen_path = get_pgo_gen_path(config); let pgo_use_path = get_pgo_use_path(config); @@ -583,8 +613,8 @@ pub(crate) unsafe fn llvm_optimize( using_thin_buffers, config.merge_functions, unroll_loops, - config.vectorize_slp, - config.vectorize_loop, + vectorize_slp, + vectorize_loop, config.no_builtins, config.emit_lifetime_markers, sanitizer_options.as_ref(), @@ -606,6 +636,83 @@ pub(crate) unsafe fn llvm_optimize( result.into_result().map_err(|()| llvm_err(dcx, LlvmError::RunLlvmPasses)) } +pub(crate) fn differentiate( + module: &ModuleCodegen, + cgcx: &CodegenContext, + diff_items: Vec, + config: &ModuleConfig, +) -> Result<(), FatalError> { + for item in &diff_items { + trace!("{}", item); + } + + let llmod = module.module_llvm.llmod(); + let llcx = &module.module_llvm.llcx; + let diag_handler = cgcx.create_dcx(); + + // Before dumping the module, we want all the tt to become part of the module. + for item in diff_items.iter() { + let name = CString::new(item.source.clone()).unwrap(); + let fn_def: Option<&llvm::Value> = + unsafe { llvm::LLVMGetNamedFunction(llmod, name.as_ptr()) }; + let fn_def = match fn_def { + Some(x) => x, + None => { + return Err(llvm_err(diag_handler.handle(), LlvmError::PrepareAutoDiff { + src: item.source.clone(), + target: item.target.clone(), + error: "could not find source function".to_owned(), + })); + } + }; + let target_name = CString::new(item.target.clone()).unwrap(); + debug!("target name: {:?}", &target_name); + let fn_target: Option<&llvm::Value> = + unsafe { llvm::LLVMGetNamedFunction(llmod, target_name.as_ptr()) }; + let fn_target = match fn_target { + Some(x) => x, + None => { + return Err(llvm_err(diag_handler.handle(), LlvmError::PrepareAutoDiff { + src: item.source.clone(), + target: item.target.clone(), + error: "could not find target function".to_owned(), + })); + } + }; + + crate::builder::generate_enzyme_call(llmod, llcx, fn_def, fn_target, item.attrs.clone()); + } + + // FIXME(ZuseZ4): support SanitizeHWAddress and prevent illegal/unsupported opts + + if let Some(opt_level) = config.opt_level { + let opt_stage = match cgcx.lto { + Lto::Fat => llvm::OptStage::PreLinkFatLTO, + Lto::Thin | Lto::ThinLocal => llvm::OptStage::PreLinkThinLTO, + _ if cgcx.opts.cg.linker_plugin_lto.enabled() => llvm::OptStage::PreLinkThinLTO, + _ => llvm::OptStage::PreLinkNoLTO, + }; + // This is our second opt call, so now we run all opts, + // to make sure we get the best performance. + let skip_size_increasing_opts = false; + trace!("running Module Optimization after differentiation"); + unsafe { + llvm_optimize( + cgcx, + diag_handler.handle(), + module, + config, + opt_level, + opt_stage, + skip_size_increasing_opts, + )? + }; + } + trace!("done with differentiate()"); + + Ok(()) +} + // Unsafe due to LLVM calls. pub(crate) unsafe fn optimize( cgcx: &CodegenContext, @@ -628,6 +735,8 @@ pub(crate) unsafe fn optimize( unsafe { llvm::LLVMWriteBitcodeToFile(llmod, out.as_ptr()) }; } + // FIXME(ZuseZ4): support SanitizeHWAddress and prevent illegal/unsupported opts + if let Some(opt_level) = config.opt_level { let opt_stage = match cgcx.lto { Lto::Fat => llvm::OptStage::PreLinkFatLTO, @@ -635,7 +744,20 @@ pub(crate) unsafe fn optimize( _ if cgcx.opts.cg.linker_plugin_lto.enabled() => llvm::OptStage::PreLinkThinLTO, _ => llvm::OptStage::PreLinkNoLTO, }; - return unsafe { llvm_optimize(cgcx, dcx, module, config, opt_level, opt_stage) }; + + // If we know that we will later run AD, then we disable vectorization and loop unrolling + let skip_size_increasing_opts = cfg!(llvm_enzyme); + return unsafe { + llvm_optimize( + cgcx, + dcx, + module, + config, + opt_level, + opt_stage, + skip_size_increasing_opts, + ) + }; } Ok(()) } diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index b5bb7630ca6c9..6a2c84f610821 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -5,6 +5,7 @@ use std::{iter, ptr}; use libc::{c_char, c_uint}; use rustc_abi as abi; use rustc_abi::{Align, Size, WrappingRange}; +use rustc_ast::expand::autodiff_attrs::{AutoDiffAttrs, DiffActivity, DiffMode}; use rustc_codegen_ssa::MemFlags; use rustc_codegen_ssa::common::{IntPredicate, RealPredicate, SynchronizationScope, TypeKind}; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; @@ -24,17 +25,276 @@ use rustc_span::Span; use rustc_target::callconv::FnAbi; use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target}; use smallvec::SmallVec; -use tracing::{debug, instrument}; +use tracing::{debug, instrument, trace}; use crate::abi::FnAbiLlvmExt; use crate::attributes; use crate::common::Funclet; use crate::context::CodegenCx; -use crate::llvm::{self, AtomicOrdering, AtomicRmwBinOp, BasicBlock, False, True}; +use crate::llvm::AttributePlace::Function; +use crate::llvm::{self, AtomicOrdering, AtomicRmwBinOp, BasicBlock, False, Metadata, True}; use crate::type_::Type; use crate::type_of::LayoutLlvmExt; use crate::value::Value; +fn get_params(fnc: &Value) -> Vec<&Value> { + unsafe { + let param_num = llvm::LLVMCountParams(fnc) as usize; + let mut fnc_args: Vec<&Value> = vec![]; + fnc_args.reserve(param_num); + llvm::LLVMGetParams(fnc, fnc_args.as_mut_ptr()); + fnc_args.set_len(param_num); + fnc_args + } +} + +/// When differentiating `fn_to_diff`, take a `outer_fn` and generate another +/// function with expected naming and calling conventions[^1] which will be +/// discovered by the enzyme LLVM pass and its body populated with the differentiated +/// `fn_to_diff`. `outer_fn` is then modified to have a call to the generated +/// function and handle the differences between the Rust calling convention and +/// Enzyme. +/// [^1]: +// FIXME(ZuseZ4): `outer_fn` should include upstream safety checks to +// cover some assumptions of enzyme/autodiff, which could lead to UB otherwise. +pub(crate) fn generate_enzyme_call<'ll>( + llmod: &'ll llvm::Module, + llcx: &'ll llvm::Context, + fn_to_diff: &'ll Value, + outer_fn: &'ll Value, + attrs: AutoDiffAttrs, +) { + let inputs = attrs.input_activity; + let output = attrs.ret_activity; + + // We have to pick the name depending on whether we want forward or reverse mode autodiff. + // FIXME(ZuseZ4): The new pass based approach should not need the {Forward/Reverse}First method anymore, since + // it will handle higher-order derivatives correctly automatically (in theory). Currently + // higher-order derivatives fail, so we should debug that before adjusting this code. + let mut ad_name: String = match attrs.mode { + DiffMode::Forward => "__enzyme_fwddiff", + DiffMode::Reverse => "__enzyme_autodiff", + DiffMode::ForwardFirst => "__enzyme_fwddiff", + DiffMode::ReverseFirst => "__enzyme_autodiff", + _ => panic!("logic bug in autodiff, unrecognized mode"), + } + .to_string(); + + // add outer_fn name to ad_name to make it unique, in case users apply autodiff to multiple + // functions. Unwrap will only panic, if LLVM gave us an invalid string. + let name = llvm::get_value_name(outer_fn); + let outer_fn_name = std::ffi::CStr::from_bytes_with_nul(name).unwrap().to_str().unwrap(); + ad_name.push_str(outer_fn_name.to_string().as_str()); + + // Let us assume the user wrote the following function square: + // + // ```llvm + // define double @square(double %x) { + // entry: + // %0 = fmul double %x, %x + // ret double %0 + // } + // ``` + // + // The user now applies autodiff to the function square, in which case fn_to_diff will be `square`. + // Our macro generates the following placeholder code (slightly simplified): + // + // ```llvm + // define double @dsquare(double %x) { + // ; placeholder code + // return 0.0; + // } + // ``` + // + // so our `outer_fn` will be `dsquare`. The unsafe code section below now removes the placeholder + // code and inserts an autodiff call. We also add a declaration for the __enzyme_autodiff call. + // Again, the arguments to all functions are slightly simplified. + // ```llvm + // declare double @__enzyme_autodiff_square(...) + // + // define double @dsquare(double %x) { + // entry: + // %0 = tail call double (...) @__enzyme_autodiff_square(double (double)* nonnull @square, double %x) + // ret double %0 + // } + // ``` + unsafe { + // On LLVM-IR, we can luckily declare __enzyme_ functions without specifying the input + // arguments. We do however need to declare them with their correct return type. + // We already figured the correct return type out in our frontend, when generating the outer_fn, + // so we can now just go ahead and use that. FIXME(ZuseZ4): This doesn't handle sret yet. + let fn_ty = llvm::LLVMGlobalGetValueType(outer_fn); + let ret_ty = llvm::LLVMGetReturnType(fn_ty); + + // LLVM can figure out the input types on it's own, so we take a shortcut here. + let enzyme_ty = llvm::LLVMFunctionType(ret_ty, ptr::null(), 0, True); + let ad_fn = llvm::LLVMRustGetOrInsertFunction( + llmod, + ad_name.as_ptr() as *const c_char, + ad_name.len().try_into().unwrap(), + enzyme_ty, + ); + // Otherwise LLVM might inline our temporary code before the enzyme pass has a chance to + // do it's work. + let attr = llvm::AttributeKind::NoInline.create_attr(llcx); + attributes::apply_to_llfn(ad_fn, Function, &[attr]); + + // first, remove all calls from fnc + let entry = llvm::LLVMGetFirstBasicBlock(outer_fn); + let br = llvm::LLVMRustGetTerminator(entry); + llvm::LLVMRustEraseInstFromParent(br); + + let builder = llvm::LLVMCreateBuilderInContext(llcx); + let last_inst = llvm::LLVMRustGetLastInstruction(entry).unwrap(); + llvm::LLVMPositionBuilderAtEnd(builder, entry); + + let num_args = llvm::LLVMCountParams(&fn_to_diff); + let mut args = Vec::with_capacity(num_args as usize + 1); + args.push(fn_to_diff); + + let enzyme_const = llvm::create_md_string(llcx, "enzyme_const"); + let enzyme_out = llvm::create_md_string(llcx, "enzyme_out"); + let enzyme_dup = llvm::create_md_string(llcx, "enzyme_dup"); + let enzyme_dupnoneed = llvm::create_md_string(llcx, "enzyme_dupnoneed"); + let enzyme_primal_ret = llvm::create_md_string(llcx, "enzyme_primal_return"); + + match output { + DiffActivity::Dual => { + args.push(llvm::LLVMMetadataAsValue(llcx, enzyme_primal_ret)); + } + DiffActivity::Active => { + args.push(llvm::LLVMMetadataAsValue(llcx, enzyme_primal_ret)); + } + _ => {} + } + + trace!("matching autodiff arguments"); + // We now handle the issue that Rust level arguments not always match the llvm-ir level + // arguments. A slice, `&[f32]`, for example, is represented as a pointer and a length on + // llvm-ir level. The number of activities matches the number of Rust level arguments, so we + // need to match those. + let mut outer_pos: usize = 0; + let mut activity_pos = 0; + let outer_args: Vec<&llvm::Value> = get_params(outer_fn); + while activity_pos < inputs.len() { + let activity = inputs[activity_pos as usize]; + // Duplicated arguments received a shadow argument, into which enzyme will write the + // gradient. + let (activity, duplicated): (&Metadata, bool) = match activity { + DiffActivity::None => panic!("not a valid input activity"), + DiffActivity::Const => (enzyme_const, false), + DiffActivity::Active => (enzyme_out, false), + DiffActivity::ActiveOnly => (enzyme_out, false), + DiffActivity::Dual => (enzyme_dup, true), + DiffActivity::DualOnly => (enzyme_dupnoneed, true), + DiffActivity::Duplicated => (enzyme_dup, true), + DiffActivity::DuplicatedOnly => (enzyme_dupnoneed, true), + DiffActivity::FakeActivitySize => (enzyme_const, false), + }; + let outer_arg = outer_args[outer_pos]; + args.push(llvm::LLVMMetadataAsValue(llcx, activity)); + args.push(outer_arg); + if duplicated { + // We know that duplicated args by construction have a following argument, + // so this can not be out of bounds. + let next_outer_arg = outer_args[outer_pos + 1]; + let next_outer_ty = llvm::LLVMTypeOf(next_outer_arg); + // FIXME(ZuseZ4): We should add support for Vec here too, but it's less urgent since + // vectors behind references (&Vec) are already supported. Users can not pass a + // Vec by value for reverse mode, so this would only help forward mode autodiff. + let slice = { + if activity_pos + 1 >= inputs.len() { + // If there is no arg following our ptr, it also can't be a slice, + // since that would lead to a ptr, int pair. + false + } else { + let next_activity = inputs[activity_pos + 1]; + // We analyze the MIR types and add this dummy activity if we visit a slice. + next_activity == DiffActivity::FakeActivitySize + } + }; + if slice { + // A duplicated slice will have the following two outer_fn arguments: + // (..., ptr1, int1, ptr2, int2, ...). We add the following llvm-ir to our __enzyme call: + // (..., metadata! enzyme_dup, ptr, ptr, int1, ...). + // FIXME(ZuseZ4): We will upstream a safety check later which asserts that + // int2 >= int1, which means the shadow vector is large enough to store the gradient. + assert!(llvm::LLVMRustGetTypeKind(next_outer_ty) == llvm::TypeKind::Integer); + let next_outer_arg2 = outer_args[outer_pos + 2]; + let next_outer_ty2 = llvm::LLVMTypeOf(next_outer_arg2); + assert!(llvm::LLVMRustGetTypeKind(next_outer_ty2) == llvm::TypeKind::Pointer); + let next_outer_arg3 = outer_args[outer_pos + 3]; + let next_outer_ty3 = llvm::LLVMTypeOf(next_outer_arg3); + assert!(llvm::LLVMRustGetTypeKind(next_outer_ty3) == llvm::TypeKind::Integer); + args.push(next_outer_arg2); + args.push(llvm::LLVMMetadataAsValue(llcx, enzyme_const)); + args.push(next_outer_arg); + outer_pos += 4; + activity_pos += 2; + } else { + // A duplicated pointer will have the following two outer_fn arguments: + // (..., ptr, ptr, ...). We add the following llvm-ir to our __enzyme call: + // (..., metadata! enzyme_dup, ptr, ptr, ...). + assert!(llvm::LLVMRustGetTypeKind(next_outer_ty) == llvm::TypeKind::Pointer); + args.push(next_outer_arg); + outer_pos += 2; + activity_pos += 1; + } + } else { + // We do not differentiate with resprect to this argument. + // We already added the metadata and argument above, so just increase the counters. + outer_pos += 1; + activity_pos += 1; + } + } + + let call = llvm::LLVMBuildCall2( + builder, + enzyme_ty, + ad_fn, + args.as_mut_ptr(), + args.len().try_into().unwrap(), + ad_name.as_ptr() as *const c_char, + ); + + // This part is a bit iffy. LLVM requires that a call to an inlineable function has some + // metadata attachted to it, but we just created this code oota. Given that the + // differentiated function already has partly confusing metadata, and given that this + // affects nothing but the auttodiff IR, we take a shortcut and just steal metadata from the + // dummy code which we inserted at a higher level. + // FIXME(ZuseZ4): Work with Enzyme core devs to clarify what debug metadata issues we have, + // and how to best improve it for enzyme core and rust-enzyme. + let md_ty = llvm::LLVMGetMDKindIDInContext( + llcx, + "dbg".as_ptr() as *const c_char, + "dbg".len() as c_uint, + ); + if llvm::LLVMRustHasMetadata(last_inst, md_ty) { + let md = llvm::LLVMRustDIGetInstMetadata(last_inst); + let md_todiff = llvm::LLVMMetadataAsValue(llcx, md); + llvm::LLVMSetMetadata(call, md_ty, md_todiff); + } else { + // We don't panic, since depending on whether we are in debug or release mode, we might + // have no debug info to copy, which would then be ok. + trace!("no dbg info"); + } + // Now that we copied the metadata, get rid of dummy code. + llvm::LLVMRustEraseInstBefore(entry, last_inst); + llvm::LLVMRustEraseInstFromParent(last_inst); + + let void_ty = llvm::LLVMVoidTypeInContext(llcx); + if llvm::LLVMTypeOf(call) != void_ty { + llvm::LLVMBuildRet(builder, call); + } else { + llvm::LLVMBuildRetVoid(builder); + }; + llvm::LLVMDisposeBuilder(builder); + + // Let's crash in case that we messed something up above and generated invalid IR. + llvm::LLVMVerifyFunction(outer_fn, llvm::LLVMVerifierFailureAction::LLVMAbortProcessAction); + } +} + // All Builders must have an llfn associated with them #[must_use] pub(crate) struct Builder<'a, 'll, 'tcx> { diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 3cdb5b971d908..f340b06e876cd 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -89,6 +89,11 @@ impl Diagnostic<'_, G> for ParseTargetMachineConfig<'_> { } } +#[derive(Diagnostic)] +#[diag(codegen_llvm_autodiff_without_lto)] +#[note] +pub(crate) struct AutoDiffWithoutLTO; + #[derive(Diagnostic)] #[diag(codegen_llvm_lto_disallowed)] pub(crate) struct LtoDisallowed; @@ -131,6 +136,8 @@ pub enum LlvmError<'a> { PrepareThinLtoModule, #[diag(codegen_llvm_parse_bitcode)] ParseBitcode, + #[diag(codegen_llvm_prepare_autodiff)] + PrepareAutoDiff { src: String, target: String, error: String }, } pub(crate) struct WithLlvmError<'a>(pub LlvmError<'a>, pub String); @@ -152,6 +159,7 @@ impl Diagnostic<'_, G> for WithLlvmError<'_> { } PrepareThinLtoModule => fluent::codegen_llvm_prepare_thin_lto_module_with_llvm_err, ParseBitcode => fluent::codegen_llvm_parse_bitcode_with_llvm_err, + PrepareAutoDiff { .. } => fluent::codegen_llvm_prepare_autodiff_with_llvm_err, }; self.0 .into_diag(dcx, level) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 3dfb86d422dd2..ca4e70690fb35 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -26,9 +26,10 @@ use std::mem::ManuallyDrop; use back::owned_target_machine::OwnedTargetMachine; use back::write::{create_informational_target_machine, create_target_machine}; -use errors::ParseTargetMachineConfig; +use errors::{AutoDiffWithoutLTO, ParseTargetMachineConfig}; pub use llvm_util::target_features; use rustc_ast::expand::allocator::AllocatorKind; +use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule}; use rustc_codegen_ssa::back::write::{ CodegenContext, FatLtoInput, ModuleConfig, TargetMachineFactoryConfig, TargetMachineFactoryFn, @@ -42,7 +43,7 @@ use rustc_middle::dep_graph::{WorkProduct, WorkProductId}; use rustc_middle::ty::TyCtxt; use rustc_middle::util::Providers; use rustc_session::Session; -use rustc_session::config::{OptLevel, OutputFilenames, PrintKind, PrintRequest}; +use rustc_session::config::{Lto, OptLevel, OutputFilenames, PrintKind, PrintRequest}; use rustc_span::symbol::Symbol; mod back { @@ -231,6 +232,19 @@ impl WriteBackendMethods for LlvmCodegenBackend { fn serialize_module(module: ModuleCodegen) -> (String, Self::ModuleBuffer) { (module.name, back::lto::ModuleBuffer::new(module.module_llvm.llmod())) } + /// Generate autodiff rules + fn autodiff( + cgcx: &CodegenContext, + module: &ModuleCodegen, + diff_fncs: Vec, + config: &ModuleConfig, + ) -> Result<(), FatalError> { + if cgcx.lto != Lto::Fat { + let dcx = cgcx.create_dcx(); + return Err(dcx.handle().emit_almost_fatal(AutoDiffWithoutLTO)); + } + back::write::differentiate(module, cgcx, diff_fncs, config) + } } unsafe impl Send for LlvmCodegenBackend {} // Llvm is on a per-thread basis diff --git a/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs new file mode 100644 index 0000000000000..1cef0296fa7f5 --- /dev/null +++ b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs @@ -0,0 +1,38 @@ +#![allow(non_camel_case_types)] + +use libc::{c_char, c_uint, size_t}; + +use super::ffi::{Attribute, BasicBlock, Builder, Metadata, Module, Type, Value}; +extern "C" { + // Enzyme + pub fn LLVMRustHasMetadata(I: &Value, KindID: c_uint) -> bool; + pub fn LLVMRustEraseInstBefore(BB: &BasicBlock, I: &Value); + pub fn LLVMRustGetLastInstruction<'a>(BB: &BasicBlock) -> Option<&'a Value>; + pub fn LLVMRustDIGetInstMetadata(I: &Value) -> &Metadata; + pub fn LLVMRustEraseInstFromParent(V: &Value); + pub fn LLVMRustGetTerminator<'a>(B: &BasicBlock) -> &'a Value; + + pub fn LLVMGetReturnType(T: &Type) -> &Type; + pub fn LLVMDumpModule(M: &Module); + pub fn LLVMCountStructElementTypes(T: &Type) -> c_uint; + pub fn LLVMVerifyFunction(V: &Value, action: LLVMVerifierFailureAction) -> bool; + pub fn LLVMGetParams(Fnc: &Value, parms: *mut &Value); + pub fn LLVMBuildCall2<'a>( + arg1: &Builder<'a>, + ty: &Type, + func: &Value, + args: *mut &Value, + num_args: size_t, + name: *const c_char, + ) -> &'a Value; + pub fn LLVMGetNamedFunction(M: &Module, Name: *const c_char) -> Option<&Value>; + pub fn LLVMIsEnumAttribute(A: &Attribute) -> bool; + pub fn LLVMIsStringAttribute(A: &Attribute) -> bool; +} + +#[repr(C)] +pub enum LLVMVerifierFailureAction { + LLVMAbortProcessAction, + LLVMPrintMessageAction, + LLVMReturnStatusAction, +} diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 909afe35a179b..069f90c6878e2 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -22,8 +22,11 @@ use crate::common::AsCCharPtr; pub mod archive_ro; pub mod diagnostic; +pub mod enzyme_ffi; mod ffi; +pub use self::enzyme_ffi::*; + impl LLVMRustResult { pub fn into_result(self) -> Result<(), ()> { match self { @@ -196,6 +199,10 @@ pub fn set_thread_local_mode(global: &Value, mode: ThreadLocalMode) { } } +pub fn create_md_string<'a>(llcx: &'a Context, s: &str) -> &'a Metadata { + unsafe { LLVMMDStringInContext2(llcx, s.as_c_char_ptr(), s.len()) } +} + impl AttributeKind { /// Create an LLVM Attribute with no associated value. pub fn create_attr(self, llcx: &Context) -> &Attribute { diff --git a/compiler/rustc_codegen_ssa/src/back/lto.rs b/compiler/rustc_codegen_ssa/src/back/lto.rs index ab8b06a05fc74..9fd984b6419ee 100644 --- a/compiler/rustc_codegen_ssa/src/back/lto.rs +++ b/compiler/rustc_codegen_ssa/src/back/lto.rs @@ -1,11 +1,13 @@ use std::ffi::CString; use std::sync::Arc; +use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_data_structures::memmap::Mmap; use rustc_errors::FatalError; use super::write::CodegenContext; use crate::ModuleCodegen; +use crate::back::write::ModuleConfig; use crate::traits::*; pub struct ThinModule { @@ -81,6 +83,23 @@ impl LtoModuleCodegen { LtoModuleCodegen::Thin(ref m) => m.cost(), } } + + /// Run autodiff on Fat LTO module + pub unsafe fn autodiff( + self, + cgcx: &CodegenContext, + diff_fncs: Vec, + config: &ModuleConfig, + ) -> Result, FatalError> { + match &self { + LtoModuleCodegen::Fat(module) => { + B::autodiff(cgcx, &module, diff_fncs, config)?; + } + _ => panic!("autodiff called with non-fat LTO module"), + } + + Ok(self) + } } pub enum SerializedModule { diff --git a/compiler/rustc_codegen_ssa/src/traits/write.rs b/compiler/rustc_codegen_ssa/src/traits/write.rs index aabe9e33c4aa1..97fe614aa10cd 100644 --- a/compiler/rustc_codegen_ssa/src/traits/write.rs +++ b/compiler/rustc_codegen_ssa/src/traits/write.rs @@ -1,3 +1,4 @@ +use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_errors::{DiagCtxtHandle, FatalError}; use rustc_middle::dep_graph::WorkProduct; @@ -61,6 +62,12 @@ pub trait WriteBackendMethods: 'static + Sized + Clone { want_summary: bool, ) -> (String, Self::ThinBuffer); fn serialize_module(module: ModuleCodegen) -> (String, Self::ModuleBuffer); + fn autodiff( + cgcx: &CodegenContext, + module: &ModuleCodegen, + diff_fncs: Vec, + config: &ModuleConfig, + ) -> Result<(), FatalError>; } pub trait ThinBufferMethods: Send + Sync { diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index cd70c3f266920..8c5f9f3d9f291 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -388,6 +388,17 @@ extern "C" void LLVMRustAddCallSiteAttributes(LLVMValueRef Instr, AddAttributes(Call, Index, Attrs, AttrsLen); } +extern "C" LLVMValueRef LLVMRustGetTerminator(LLVMBasicBlockRef BB) { + Instruction *ret = unwrap(BB)->getTerminator(); + return wrap(ret); +} + +extern "C" void LLVMRustEraseInstFromParent(LLVMValueRef Instr) { + if (auto I = dyn_cast(unwrap(Instr))) { + I->eraseFromParent(); + } +} + extern "C" LLVMAttributeRef LLVMRustCreateAttrNoValue(LLVMContextRef C, LLVMRustAttributeKind RustAttr) { return wrap(Attribute::get(*unwrap(C), fromRust(RustAttr))); @@ -954,6 +965,47 @@ extern "C" void LLVMRustAddModuleFlagString( MDString::get(unwrap(M)->getContext(), StringRef(Value, ValueLen))); } +extern "C" LLVMValueRef LLVMRustGetLastInstruction(LLVMBasicBlockRef BB) { + auto Point = unwrap(BB)->rbegin(); + if (Point != unwrap(BB)->rend()) + return wrap(&*Point); + return nullptr; +} + +extern "C" void LLVMRustEraseInstBefore(LLVMBasicBlockRef bb, LLVMValueRef I) { + auto &BB = *unwrap(bb); + auto &Inst = *unwrap(I); + auto It = BB.begin(); + while (&*It != &Inst) + ++It; + // Make sure we found the Instruction. + assert(It != BB.end()); + // We don't want to erase the instruction itself. + It--; + // Delete in rev order to ensure no dangling references. + while (It != BB.begin()) { + auto Prev = std::prev(It); + It->eraseFromParent(); + It = Prev; + } + It->eraseFromParent(); +} + +extern "C" bool LLVMRustHasMetadata(LLVMValueRef inst, unsigned kindID) { + if (auto *I = dyn_cast(unwrap(inst))) { + return I->hasMetadata(kindID); + } + return false; +} + +extern "C" LLVMMetadataRef LLVMRustDIGetInstMetadata(LLVMValueRef x) { + if (auto *I = dyn_cast(unwrap(x))) { + auto *MD = I->getDebugLoc().getAsMDNode(); + return wrap(MD); + } + return nullptr; +} + extern "C" void LLVMRustGlobalAddMetadata(LLVMValueRef Global, unsigned Kind, LLVMMetadataRef MD) { unwrap(Global)->addMetadata(Kind, *unwrap(MD)); From 7fb2fc01a5410163264ce195a9eb1db61fb54d87 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:47:41 -0500 Subject: [PATCH 10/11] feat: clarify how to use `black_box()` Co-authored-by: Ben Kimock --- library/core/src/hint.rs | 92 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 3 deletions(-) diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index c59e4414d3726..9c054b99a27ac 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -310,6 +310,8 @@ pub fn spin_loop() { /// behavior in the calling code. This property makes `black_box` useful for writing code in which /// certain optimizations are not desired, such as benchmarks. /// +///
+/// /// Note however, that `black_box` is only (and can only be) provided on a "best-effort" basis. The /// extent to which it can block optimisations may vary depending upon the platform and code-gen /// backend used. Programs cannot rely on `black_box` for *correctness*, beyond it behaving as the @@ -317,6 +319,8 @@ pub fn spin_loop() { /// This also means that this function does not offer any guarantees for cryptographic or security /// purposes. /// +///
+/// /// [`std::convert::identity`]: crate::convert::identity /// /// # When is this useful? @@ -357,7 +361,7 @@ pub fn spin_loop() { /// ``` /// use std::hint::black_box; /// -/// // Same `contains` function +/// // Same `contains` function. /// fn contains(haystack: &[&str], needle: &str) -> bool { /// haystack.iter().any(|x| x == &needle) /// } @@ -366,8 +370,13 @@ pub fn spin_loop() { /// let haystack = vec!["abc", "def", "ghi", "jkl", "mno"]; /// let needle = "ghi"; /// for _ in 0..10 { -/// // Adjust our benchmark loop contents -/// black_box(contains(black_box(&haystack), black_box(needle))); +/// // Force the compiler to run `contains`, even though it is a pure function whose +/// // results are unused. +/// black_box(contains( +/// // Prevent the compiler from making assumptions about the input. +/// black_box(&haystack), +/// black_box(needle), +/// )); /// } /// } /// ``` @@ -382,6 +391,83 @@ pub fn spin_loop() { /// /// This makes our benchmark much more realistic to how the function would actually be used, where /// arguments are usually not known at compile time and the result is used in some way. +/// +/// # How to use this +/// +/// In practice, `black_box` serves two purposes: +/// +/// 1. It prevents the compiler from making optimizations related to the value returned by `black_box` +/// 2. It forces the value passed to `black_box` to be calculated, even if the return value of `black_box` is unused +/// +/// ``` +/// use std::hint::black_box; +/// +/// let zero = 0; +/// let five = 5; +/// +/// // The compiler will see this and remove the `* five` call, because it knows that multiplying +/// // any integer by 0 will result in 0. +/// let c = zero * five; +/// +/// // Adding `black_box` here disables the compiler's ability to reason about the first operand in the multiplication. +/// // It is forced to assume that it can be any possible number, so it cannot remove the `* five` +/// // operation. +/// let c = black_box(zero) * five; +/// ``` +/// +/// While most cases will not be as clear-cut as the above example, it still illustrates how +/// `black_box` can be used. When benchmarking a function, you usually want to wrap its inputs in +/// `black_box` so the compiler cannot make optimizations that would be unrealistic in real-life +/// use. +/// +/// ``` +/// use std::hint::black_box; +/// +/// // This is a simple function that increments its input by 1. Note that it is pure, meaning it +/// // has no side-effects. This function has no effect if its result is unused. (An example of a +/// // function *with* side-effects is `println!()`.) +/// fn increment(x: u8) -> u8 { +/// x + 1 +/// } +/// +/// // Here, we call `increment` but discard its result. The compiler, seeing this and knowing that +/// // `increment` is pure, will eliminate this function call entirely. This may not be desired, +/// // though, especially if we're trying to track how much time `increment` takes to execute. +/// let _ = increment(black_box(5)); +/// +/// // Here, we force `increment` to be executed. This is because the compiler treats `black_box` +/// // as if it has side-effects, and thus must compute its input. +/// let _ = black_box(increment(black_box(5))); +/// ``` +/// +/// There may be additional situations where you want to wrap the result of a function in +/// `black_box` to force its execution. This is situational though, and may not have any effect +/// (such as when the function returns a zero-sized type such as [`()` unit][unit]). +/// +/// Note that `black_box` has no effect on how its input is treated, only its output. As such, +/// expressions passed to `black_box` may still be optimized: +/// +/// ``` +/// use std::hint::black_box; +/// +/// // The compiler sees this... +/// let y = black_box(5 * 10); +/// +/// // ...as this. As such, it will likely simplify `5 * 10` to just `50`. +/// let _0 = 5 * 10; +/// let y = black_box(_0); +/// ``` +/// +/// In the above example, the `5 * 10` expression is considered distinct from the `black_box` call, +/// and thus is still optimized by the compiler. You can prevent this by moving the multiplication +/// operation outside of `black_box`: +/// +/// ``` +/// use std::hint::black_box; +/// +/// // No assumptions can be made about either operand, so the multiplication is not optimized out. +/// let y = black_box(5) * black_box(10); +/// ``` #[inline] #[stable(feature = "bench_black_box", since = "1.66.0")] #[rustc_const_unstable(feature = "const_black_box", issue = "none")] From 2e412fef75ff2f45186863a76fdd7c5a9ec1f018 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 12 Dec 2024 11:49:31 +1100 Subject: [PATCH 11/11] Remove `Lexer`'s dependency on `Parser`. Lexing precedes parsing, as you'd expect: `Lexer` creates a `TokenStream` and `Parser` then parses that `TokenStream`. But, in a horrendous violation of layering abstractions and common sense, `Lexer` depends on `Parser`! The `Lexer::unclosed_delim_err` method does some error recovery that relies on creating a `Parser` to do some post-processing of the `TokenStream` that the `Lexer` just created. This commit just removes `unclosed_delim_err`. This change removes `Lexer`'s dependency on `Parser`, and also means that `lex_token_tree`'s return value can have a more typical form. The cost is slightly worse error messages in two obscure cases, as shown in these tests: - tests/ui/parser/brace-in-let-chain.rs: there is slightly less explanation in this case involving an extra `{`. - tests/ui/parser/diff-markers/unclosed-delims{,-in-macro}.rs: the diff marker detection is no longer supported (because that detection is implemented in the parser). In my opinion this cost is outweighed by the magnitude of the code cleanup. --- compiler/rustc_parse/src/lexer/mod.rs | 40 ++++---- compiler/rustc_parse/src/lexer/tokentrees.rs | 94 +++---------------- tests/ui/parser/brace-in-let-chain.rs | 4 +- tests/ui/parser/brace-in-let-chain.stderr | 30 +----- .../diff-markers/unclosed-delims-in-macro.rs | 6 +- .../unclosed-delims-in-macro.stderr | 27 ++---- .../ui/parser/diff-markers/unclosed-delims.rs | 14 +-- .../diff-markers/unclosed-delims.stderr | 27 ++---- 8 files changed, 68 insertions(+), 174 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs index 2426eb81678e1..443ddfc94ec2b 100644 --- a/compiler/rustc_parse/src/lexer/mod.rs +++ b/compiler/rustc_parse/src/lexer/mod.rs @@ -69,24 +69,30 @@ pub(crate) fn lex_token_trees<'psess, 'src>( token: Token::dummy(), diag_info: TokenTreeDiagInfo::default(), }; - let (_open_spacing, stream, res) = lexer.lex_token_trees(/* is_delimited */ false); - let unmatched_delims = lexer.diag_info.unmatched_delims; - - if res.is_ok() && unmatched_delims.is_empty() { - Ok(stream) - } else { - // Return error if there are unmatched delimiters or unclosed delimiters. - // We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch - // because the delimiter mismatch is more likely to be the root cause of error - let mut buffer: Vec<_> = unmatched_delims - .into_iter() - .filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess)) - .collect(); - if let Err(errs) = res { - // Add unclosing delimiter or diff marker errors - buffer.extend(errs); + let res = lexer.lex_token_trees(/* is_delimited */ false); + + let mut unmatched_delims: Vec<_> = lexer + .diag_info + .unmatched_delims + .into_iter() + .filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess)) + .collect(); + + match res { + Ok((_open_spacing, stream)) => { + if unmatched_delims.is_empty() { + Ok(stream) + } else { + // Return error if there are unmatched delimiters or unclosed delimiters. + Err(unmatched_delims) + } + } + Err(errs) => { + // We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch + // because the delimiter mismatch is more likely to be the root cause of error + unmatched_delims.extend(errs); + Err(unmatched_delims) } - Err(buffer) } } diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index ee38f16d4ecd5..b3f83a320241e 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -1,12 +1,10 @@ use rustc_ast::token::{self, Delimiter, Token}; use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree}; use rustc_ast_pretty::pprust::token_to_string; -use rustc_errors::{Applicability, Diag}; -use rustc_span::symbol::kw; +use rustc_errors::Diag; use super::diagnostics::{report_suspicious_mismatch_block, same_indentation_level}; use super::{Lexer, UnmatchedDelim}; -use crate::Parser; impl<'psess, 'src> Lexer<'psess, 'src> { // Lex into a token stream. The `Spacing` in the result is that of the @@ -14,7 +12,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> { pub(super) fn lex_token_trees( &mut self, is_delimited: bool, - ) -> (Spacing, TokenStream, Result<(), Vec>>) { + ) -> Result<(Spacing, TokenStream), Vec>> { // Move past the opening delimiter. let open_spacing = self.bump_minimal(); @@ -27,25 +25,25 @@ impl<'psess, 'src> Lexer<'psess, 'src> { debug_assert!(!matches!(delim, Delimiter::Invisible(_))); buf.push(match self.lex_token_tree_open_delim(delim) { Ok(val) => val, - Err(errs) => return (open_spacing, TokenStream::new(buf), Err(errs)), + Err(errs) => return Err(errs), }) } token::CloseDelim(delim) => { // Invisible delimiters cannot occur here because `TokenTreesReader` parses // code directly from strings, with no macro expansion involved. debug_assert!(!matches!(delim, Delimiter::Invisible(_))); - return ( - open_spacing, - TokenStream::new(buf), - if is_delimited { Ok(()) } else { Err(vec![self.close_delim_err(delim)]) }, - ); + return if is_delimited { + Ok((open_spacing, TokenStream::new(buf))) + } else { + Err(vec![self.close_delim_err(delim)]) + }; } token::Eof => { - return ( - open_spacing, - TokenStream::new(buf), - if is_delimited { Err(vec![self.eof_err()]) } else { Ok(()) }, - ); + return if is_delimited { + Err(vec![self.eof_err()]) + } else { + Ok((open_spacing, TokenStream::new(buf))) + }; } _ => { // Get the next normal token. @@ -107,10 +105,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> { // Lex the token trees within the delimiters. // We stop at any delimiter so we can try to recover if the user // uses an incorrect delimiter. - let (open_spacing, tts, res) = self.lex_token_trees(/* is_delimited */ true); - if let Err(errs) = res { - return Err(self.unclosed_delim_err(tts, errs)); - } + let (open_spacing, tts) = self.lex_token_trees(/* is_delimited */ true)?; // Expand to cover the entire delimited token tree. let delim_span = DelimSpan::from_pair(pre_span, self.token.span); @@ -247,67 +242,6 @@ impl<'psess, 'src> Lexer<'psess, 'src> { this_spacing } - fn unclosed_delim_err( - &mut self, - tts: TokenStream, - mut errs: Vec>, - ) -> Vec> { - // If there are unclosed delims, see if there are diff markers and if so, point them - // out instead of complaining about the unclosed delims. - let mut parser = Parser::new(self.psess, tts, None); - let mut diff_errs = vec![]; - // Suggest removing a `{` we think appears in an `if`/`while` condition. - // We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, - // but we have no way of tracking this in the lexer itself, so we piggyback on the parser. - let mut in_cond = false; - while parser.token != token::Eof { - if let Err(diff_err) = parser.err_vcs_conflict_marker() { - diff_errs.push(diff_err); - } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { - in_cond = true; - } else if matches!( - parser.token.kind, - token::CloseDelim(Delimiter::Brace) | token::FatArrow - ) { - // End of the `if`/`while` body, or the end of a `match` guard. - in_cond = false; - } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { - // Store the `&&` and `let` to use their spans later when creating the diagnostic - let maybe_andand = parser.look_ahead(1, |t| t.clone()); - let maybe_let = parser.look_ahead(2, |t| t.clone()); - if maybe_andand == token::OpenDelim(Delimiter::Brace) { - // This might be the beginning of the `if`/`while` body (i.e., the end of the - // condition). - in_cond = false; - } else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) { - let mut err = parser.dcx().struct_span_err( - parser.token.span, - "found a `{` in the middle of a let-chain", - ); - err.span_suggestion( - parser.token.span, - "consider removing this brace to parse the `let` as part of the same chain", - "", - Applicability::MachineApplicable, - ); - err.span_label( - maybe_andand.span.to(maybe_let.span), - "you might have meant to continue the let-chain here", - ); - errs.push(err); - } - } - parser.bump(); - } - if !diff_errs.is_empty() { - for err in errs { - err.cancel(); - } - return diff_errs; - } - errs - } - fn close_delim_err(&mut self, delim: Delimiter) -> Diag<'psess> { // An unexpected closing delimiter (i.e., there is no matching opening delimiter). let token_str = token_to_string(&self.token); diff --git a/tests/ui/parser/brace-in-let-chain.rs b/tests/ui/parser/brace-in-let-chain.rs index 1f34c73a2c3bd..2009bc88d9e8d 100644 --- a/tests/ui/parser/brace-in-let-chain.rs +++ b/tests/ui/parser/brace-in-let-chain.rs @@ -3,7 +3,7 @@ #![feature(let_chains)] fn main() { if let () = () - && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () { && let () = () { } @@ -11,7 +11,7 @@ fn main() { fn quux() { while let () = () - && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () { && let () = () { } diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr index 913a34700dfc9..12af95c278688 100644 --- a/tests/ui/parser/brace-in-let-chain.stderr +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -27,33 +27,5 @@ LL | } LL | } | ^ -error: found a `{` in the middle of a let-chain - --> $DIR/brace-in-let-chain.rs:14:24 - | -LL | && let () = () { - | ^ -LL | && let () = () - | ------ you might have meant to continue the let-chain here - | -help: consider removing this brace to parse the `let` as part of the same chain - | -LL - && let () = () { -LL + && let () = () - | - -error: found a `{` in the middle of a let-chain - --> $DIR/brace-in-let-chain.rs:6:24 - | -LL | && let () = () { - | ^ -LL | && let () = () - | ------ you might have meant to continue the let-chain here - | -help: consider removing this brace to parse the `let` as part of the same chain - | -LL - && let () = () { -LL + && let () = () - | - -error: aborting due to 3 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs index da1774acea542..41a7de03d4b79 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs +++ b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.rs @@ -1,9 +1,11 @@ +// The diff marker detection was removed for this example, because it relied on +// the lexer having a dependency on the parser, which was horrible. + macro_rules! foo { <<<<<<< HEAD - //~^ ERROR encountered diff marker () { ======= () { // >>>>>>> 7a4f13c blah blah blah } -} +} //~ this file contains an unclosed delimiter diff --git a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr index 927821ddfaedb..b33f2b5d1b8b2 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr +++ b/tests/ui/parser/diff-markers/unclosed-delims-in-macro.stderr @@ -1,23 +1,16 @@ -error: encountered diff marker - --> $DIR/unclosed-delims-in-macro.rs:2:1 +error: this file contains an unclosed delimiter + --> $DIR/unclosed-delims-in-macro.rs:11:48 | +LL | macro_rules! foo { + | - unclosed delimiter LL | <<<<<<< HEAD - | ^^^^^^^ between this marker and `=======` is the code that we're merging into +LL | () { + | - this delimiter might not be properly closed... ... -LL | ======= - | ------- between this marker and `>>>>>>>` is the incoming code -LL | () { // -LL | >>>>>>> 7a4f13c blah blah blah - | ^^^^^^^ this marker concludes the conflict region - | - = note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts - to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers - = help: if you're having merge conflicts after pulling new code: - the top section is the code you already had and the bottom section is the remote code - if you're in the middle of a rebase: - the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased - = note: for an explanation on these markers from the `git` documentation: - visit +LL | } + | - ^ + | | + | ...as it matches this but it has different indentation error: aborting due to 1 previous error diff --git a/tests/ui/parser/diff-markers/unclosed-delims.rs b/tests/ui/parser/diff-markers/unclosed-delims.rs index 7d400c3827bb6..827c1eebb9d5f 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims.rs +++ b/tests/ui/parser/diff-markers/unclosed-delims.rs @@ -1,18 +1,12 @@ +// The diff marker detection was removed for this example, because it relied on +// the lexer having a dependency on the parser, which was horrible. + mod tests { #[test] <<<<<<< HEAD -//~^ ERROR encountered diff marker -//~| NOTE between this marker and `=======` - -//~| NOTE conflict markers indicate that -//~| HELP if you're having merge conflicts -//~| NOTE for an explanation on these markers - fn test1() { ======= -//~^ NOTE between this marker and `>>>>>>>` fn test2() { >>>>>>> 7a4f13c blah blah blah -//~^ NOTE this marker concludes the conflict region } -} +} //~ this file contains an unclosed delimiter diff --git a/tests/ui/parser/diff-markers/unclosed-delims.stderr b/tests/ui/parser/diff-markers/unclosed-delims.stderr index 1eab96442b4f2..b2541aa47baed 100644 --- a/tests/ui/parser/diff-markers/unclosed-delims.stderr +++ b/tests/ui/parser/diff-markers/unclosed-delims.stderr @@ -1,23 +1,16 @@ -error: encountered diff marker - --> $DIR/unclosed-delims.rs:3:1 +error: this file contains an unclosed delimiter + --> $DIR/unclosed-delims.rs:12:48 | -LL | <<<<<<< HEAD - | ^^^^^^^ between this marker and `=======` is the code that we're merging into +LL | mod tests { + | - unclosed delimiter ... -LL | ======= - | ------- between this marker and `>>>>>>>` is the incoming code +LL | fn test1() { + | - this delimiter might not be properly closed... ... -LL | >>>>>>> 7a4f13c blah blah blah - | ^^^^^^^ this marker concludes the conflict region - | - = note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts - to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers - = help: if you're having merge conflicts after pulling new code: - the top section is the code you already had and the bottom section is the remote code - if you're in the middle of a rebase: - the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased - = note: for an explanation on these markers from the `git` documentation: - visit +LL | } + | - ^ + | | + | ...as it matches this but it has different indentation error: aborting due to 1 previous error