From d2af7da065286d6169fe28f526fe295cf6083fd8 Mon Sep 17 00:00:00 2001 From: kadiwa Date: Mon, 19 Jun 2023 20:29:01 +0200 Subject: [PATCH 01/30] small code improvements in collect_intra_doc_links --- .../passes/collect_intra_doc_links.rs | 103 +++++++----------- 1 file changed, 39 insertions(+), 64 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1e8ece7e114a3..fa9dcc2e36d5a 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -108,15 +108,13 @@ impl Res { Res::Primitive(_) => return Suggestion::Prefix("prim"), Res::Def(kind, _) => kind, }; - if kind == DefKind::Macro(MacroKind::Bang) { - return Suggestion::Macro; - } else if kind == DefKind::Fn || kind == DefKind::AssocFn { - return Suggestion::Function; - } else if kind == DefKind::Field { - return Suggestion::RemoveDisambiguator; - } let prefix = match kind { + DefKind::Fn | DefKind::AssocFn => return Suggestion::Function, + DefKind::Field => return Suggestion::RemoveDisambiguator, + DefKind::Macro(MacroKind::Bang) => return Suggestion::Macro, + + DefKind::Macro(MacroKind::Derive) => "derive", DefKind::Struct => "struct", DefKind::Enum => "enum", DefKind::Trait => "trait", @@ -126,7 +124,6 @@ impl Res { "const" } DefKind::Static(_) => "static", - DefKind::Macro(MacroKind::Derive) => "derive", // Now handle things that don't have a specific disambiguator _ => match kind .ns() @@ -283,20 +280,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("looking for enum variant {path_str}"); let mut split = path_str.rsplitn(3, "::"); - let variant_field_name = split - .next() - .map(|f| Symbol::intern(f)) - .expect("fold_item should ensure link is non-empty"); - let variant_name = - // we're not sure this is a variant at all, so use the full string - // If there's no second component, the link looks like `[path]`. - // So there's no partial res and we should say the whole link failed to resolve. - split.next().map(|f| Symbol::intern(f)).ok_or_else(no_res)?; - let path = split - .next() - // If there's no third component, we saw `[a::b]` before and it failed to resolve. - // So there's no partial res. - .ok_or_else(no_res)?; + let variant_field_name = Symbol::intern(split.next().unwrap()); + // We're not sure this is a variant at all, so use the full string. + // If there's no second component, the link looks like `[path]`. + // So there's no partial res and we should say the whole link failed to resolve. + let variant_name = Symbol::intern(split.next().ok_or_else(no_res)?); + + // If there's no third component, we saw `[a::b]` before and it failed to resolve. + // So there's no partial res. + let path = split.next().ok_or_else(no_res)?; let ty_res = self.resolve_path(&path, TypeNS, item_id, module_id).ok_or_else(no_res)?; match ty_res { @@ -447,41 +439,29 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } // Try looking for methods and associated items. - let mut split = path_str.rsplitn(2, "::"); - // NB: `split`'s first element is always defined, even if the delimiter was not present. - // NB: `item_str` could be empty when resolving in the root namespace (e.g. `::std`). - let item_str = split.next().unwrap(); - let item_name = Symbol::intern(item_str); - let path_root = split - .next() + // NB: `path_root` could be empty when resolving in the root namespace (e.g. `::std`). + let (path_root, item_str) = path_str.rsplit_once("::").ok_or_else(|| { // If there's no `::`, it's not an associated item. // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved. - .ok_or_else(|| { - debug!("found no `::`, assuming {item_name} was correctly not in scope"); - UnresolvedPath { - item_id, - module_id, - partial_res: None, - unresolved: item_str.into(), - } - })?; + debug!("found no `::`, assuming {path_str} was correctly not in scope"); + UnresolvedPath { item_id, module_id, partial_res: None, unresolved: path_str.into() } + })?; + let item_name = Symbol::intern(item_str); // FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support // links to primitives when `#[rustc_doc_primitive]` is present. It should give an ambiguity // error instead and special case *only* modules with `#[rustc_doc_primitive]`, not all // primitives. - match resolve_primitive(&path_root, TypeNS) - .or_else(|| self.resolve_path(&path_root, TypeNS, item_id, module_id)) - .and_then(|ty_res| { - let candidates = self - .resolve_associated_item(ty_res, item_name, ns, module_id) + match resolve_primitive(path_root, TypeNS) + .or_else(|| self.resolve_path(path_root, TypeNS, item_id, module_id)) + .map(|ty_res| { + self.resolve_associated_item(ty_res, item_name, ns, module_id) .into_iter() .map(|(res, def_id)| (res, Some(def_id))) - .collect::>(); - if !candidates.is_empty() { Some(candidates) } else { None } + .collect::>() }) { - Some(r) => Ok(r), - None => { + Some(r) if !r.is_empty() => Ok(r), + _ => { if ns == Namespace::ValueNS { self.variant_field(path_str, item_id, module_id) .map(|(res, def_id)| vec![(res, Some(def_id))]) @@ -1262,7 +1242,7 @@ impl LinkCollector<'_, '_> { self.report_rawptr_assoc_feature_gate(diag.dox, &diag.link_range, diag.item); return None; } else { - candidates = vec![candidates[0]]; + candidates = vec![*candidate]; } } @@ -1270,8 +1250,10 @@ impl LinkCollector<'_, '_> { // and after removing duplicated kinds, only one remains, the `ambiguity_error` function // won't emit an error. So at this point, we can just take the first candidate as it was // the first retrieved and use it to generate the link. - if candidates.len() > 1 && !ambiguity_error(self.cx, &diag, &key.path_str, &candidates) { - candidates = vec![candidates[0]]; + if let [candidate, _candidate2, ..] = *candidates + && !ambiguity_error(self.cx, &diag, &key.path_str, &candidates) + { + candidates = vec![candidate]; } if let &[(res, def_id)] = candidates.as_slice() { @@ -1321,12 +1303,11 @@ impl LinkCollector<'_, '_> { let mut err = ResolutionFailure::NotResolved(err); for other_ns in [TypeNS, ValueNS, MacroNS] { if other_ns != expected_ns { - if let Ok(res) = - self.resolve(path_str, other_ns, item_id, module_id) - && !res.is_empty() + if let Ok(&[res, ..]) = + self.resolve(path_str, other_ns, item_id, module_id).as_deref() { err = ResolutionFailure::WrongNamespace { - res: full_res(self.cx.tcx, res[0]), + res: full_res(self.cx.tcx, res), expected_ns, }; break; @@ -1747,7 +1728,6 @@ fn report_diagnostic( lint.note(format!( "the link appears in this line:\n\n{line}\n\ {indicator: Option<(&str, &str)> { - let mut splitter = path.rsplitn(2, "::"); - splitter.next().and_then(|right| splitter.next().map(|left| (left, right))) - } // Check if _any_ parent of the path gets resolved. // If so, report it and say the first which failed; if not, say the first path segment didn't resolve. let mut name = path_str; 'outer: loop { - let Some((start, end)) = split(name) else { + // FIXME(jynelson): this might conflict with my `Self` fix in #76467 + let Some((start, end)) = name.rsplit_once("::") else { // avoid bug that marked [Quux::Z] as missing Z, not Quux if partial_res.is_none() { *unresolved = name.into(); @@ -1830,8 +1805,8 @@ fn resolution_failure( for ns in [TypeNS, ValueNS, MacroNS] { if let Ok(v_res) = collector.resolve(start, ns, item_id, module_id) { debug!("found partial_res={v_res:?}"); - if !v_res.is_empty() { - *partial_res = Some(full_res(tcx, v_res[0])); + if let Some(&res) = v_res.first() { + *partial_res = Some(full_res(tcx, res)); *unresolved = end.into(); break 'outer; } From b152de29c59fc0d34ab26b0be445a45e9ffe2dfb Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 5 Jan 2024 13:23:05 +1100 Subject: [PATCH 02/30] coverage: Discard code regions that might cause fatal errors in `llvm-cov` --- .../rustc_mir_transform/src/coverage/mod.rs | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index dcd7014f4fc90..4b91f042f007a 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -330,7 +330,7 @@ fn make_code_region( start_line = source_map.doctest_offset_line(&file.name, start_line); end_line = source_map.doctest_offset_line(&file.name, end_line); - Some(CodeRegion { + check_code_region(CodeRegion { file_name, start_line: start_line as u32, start_col: start_col as u32, @@ -339,6 +339,41 @@ fn make_code_region( }) } +/// If `llvm-cov` sees a code region that is improperly ordered (end < start), +/// it will immediately exit with a fatal error. To prevent that from happening, +/// discard regions that are improperly ordered, or might be interpreted in a +/// way that makes them improperly ordered. +fn check_code_region(code_region: CodeRegion) -> Option { + let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region; + + // Line/column coordinates are supposed to be 1-based. If we ever emit + // coordinates of 0, `llvm-cov` might misinterpret them. + let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0); + // Coverage mappings use the high bit of `end_col` to indicate that a + // region is actually a "gap" region, so make sure it's unset. + let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0; + // If a region is improperly ordered (end < start), `llvm-cov` will exit + // with a fatal error, which is inconvenient for users and hard to debug. + let is_ordered = (start_line, start_col) <= (end_line, end_col); + + if all_nonzero && end_col_has_high_bit_unset && is_ordered { + Some(code_region) + } else { + debug!( + ?code_region, + ?all_nonzero, + ?end_col_has_high_bit_unset, + ?is_ordered, + "Skipping code region that would be misinterpreted or rejected by LLVM" + ); + if cfg!(debug_assertions) { + // If this happens in a debug build, ICE to make it easier to notice. + bug!("Improper code region: {code_region:?}"); + } + None + } +} + fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { // Only instrument functions, methods, and closures (not constants since they are evaluated // at compile time by Miri). From 46652dd254bc9c413d14cec637c7f21ac6601e05 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 17 Jan 2024 14:26:26 +0000 Subject: [PATCH 03/30] llvm: simplify data layout check Don't skip the inconsistent data layout check for custom LLVMs. With #118708, all targets will have a simple test that would trigger this check if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this check won't trigger with our LLVM because each target will have been confirmed to work. With non-builtin targets, this check is probably useful to have because you can change the data layout in your target and if its wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. Signed-off-by: David Wood --- compiler/rustc_codegen_llvm/messages.ftl | 3 ++ compiler/rustc_codegen_llvm/src/context.rs | 38 +++++-------------- compiler/rustc_codegen_llvm/src/errors.rs | 9 +++++ src/bootstrap/src/core/build_steps/compile.rs | 5 --- src/tools/tidy/src/ui_tests.rs | 7 ++-- tests/run-make/target-specs/Makefile | 2 +- tests/ui/codegen/mismatched-data-layout.json | 13 +++++++ tests/ui/codegen/mismatched-data-layouts.rs | 14 +++++++ .../ui/codegen/mismatched-data-layouts.stderr | 4 ++ 9 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 tests/ui/codegen/mismatched-data-layout.json create mode 100644 tests/ui/codegen/mismatched-data-layouts.rs create mode 100644 tests/ui/codegen/mismatched-data-layouts.stderr diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl index 7a86ddc7556a0..d5bc04f594da1 100644 --- a/compiler/rustc_codegen_llvm/messages.ftl +++ b/compiler/rustc_codegen_llvm/messages.ftl @@ -39,6 +39,9 @@ codegen_llvm_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdy codegen_llvm_lto_proc_macro = lto cannot be used for `proc-macro` crate type without `-Zdylib-lto` +codegen_llvm_mismatch_data_layout = + data-layout for target `{$rustc_target}`, `{$rustc_layout}`, differs from LLVM target's `{$llvm_target}` default layout, `{$llvm_layout}` + codegen_llvm_missing_features = add the missing features in a `target_feature` attribute diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 1d1b6e6148dd2..b0f0b650e77f3 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -34,6 +34,7 @@ use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel}; use smallvec::SmallVec; use libc::c_uint; +use std::borrow::Borrow; use std::cell::{Cell, RefCell}; use std::ffi::CStr; use std::str; @@ -147,8 +148,7 @@ pub unsafe fn create_module<'ll>( } // Ensure the data-layout values hardcoded remain the defaults. - if sess.target.is_builtin { - // tm is disposed by its drop impl + { let tm = crate::back::write::create_informational_target_machine(tcx.sess); llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, &tm); @@ -156,33 +156,13 @@ pub unsafe fn create_module<'ll>( let llvm_data_layout = str::from_utf8(CStr::from_ptr(llvm_data_layout).to_bytes()) .expect("got a non-UTF8 data-layout from LLVM"); - // Unfortunately LLVM target specs change over time, and right now we - // don't have proper support to work with any more than one - // `data_layout` than the one that is in the rust-lang/rust repo. If - // this compiler is configured against a custom LLVM, we may have a - // differing data layout, even though we should update our own to use - // that one. - // - // As an interim hack, if CFG_LLVM_ROOT is not an empty string then we - // disable this check entirely as we may be configured with something - // that has a different target layout. - // - // Unsure if this will actually cause breakage when rustc is configured - // as such. - // - // FIXME(#34960) - let cfg_llvm_root = option_env!("CFG_LLVM_ROOT").unwrap_or(""); - let custom_llvm_used = !cfg_llvm_root.trim().is_empty(); - - if !custom_llvm_used && target_data_layout != llvm_data_layout { - bug!( - "data-layout for target `{rustc_target}`, `{rustc_layout}`, \ - differs from LLVM target's `{llvm_target}` default layout, `{llvm_layout}`", - rustc_target = sess.opts.target_triple, - rustc_layout = target_data_layout, - llvm_target = sess.target.llvm_target, - llvm_layout = llvm_data_layout - ); + if target_data_layout != llvm_data_layout { + tcx.dcx().emit_err(crate::errors::MismatchedDataLayout { + rustc_target: sess.opts.target_triple.to_string().as_str(), + rustc_layout: target_data_layout.as_str(), + llvm_target: sess.target.llvm_target.borrow(), + llvm_layout: llvm_data_layout, + }); } } diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 697ce6022984b..d82ff6656f4d3 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -244,3 +244,12 @@ pub(crate) struct CopyBitcode { pub struct UnknownCompression { pub algorithm: &'static str, } + +#[derive(Diagnostic)] +#[diag(codegen_llvm_mismatch_data_layout)] +pub struct MismatchedDataLayout<'a> { + pub rustc_target: &'a str, + pub rustc_layout: &'a str, + pub llvm_target: &'a str, + pub llvm_layout: &'a str, +} diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 190f0fe3cddcf..663ec6fe08645 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1103,16 +1103,11 @@ pub fn rustc_cargo_env( /// Pass down configuration from the LLVM build into the build of /// rustc_llvm and rustc_codegen_llvm. fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { - let target_config = builder.config.target_config.get(&target); - if builder.is_rust_llvm(target) { cargo.env("LLVM_RUSTLLVM", "1"); } let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target }); cargo.env("LLVM_CONFIG", &llvm_config); - if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) { - cargo.env("CFG_LLVM_ROOT", s); - } // Some LLVM linker flags (-L and -l) may be needed to link `rustc_llvm`. Its build script // expects these to be passed via the `LLVM_LINKER_FLAGS` env variable, separated by diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 95cf9f8cb13ad..85553d2e3384a 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -24,9 +24,10 @@ const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ const EXTENSION_EXCEPTION_PATHS: &[&str] = &[ "tests/ui/asm/named-asm-labels.s", // loading an external asm file to test named labels lint - "tests/ui/check-cfg/my-awesome-platform.json", // testing custom targets with cfgs - "tests/ui/commandline-argfile-badutf8.args", // passing args via a file - "tests/ui/commandline-argfile.args", // passing args via a file + "tests/ui/codegen/mismatched-data-layout.json", // testing mismatched data layout w/ custom targets + "tests/ui/check-cfg/my-awesome-platform.json", // testing custom targets with cfgs + "tests/ui/commandline-argfile-badutf8.args", // passing args via a file + "tests/ui/commandline-argfile.args", // passing args via a file "tests/ui/crate-loading/auxiliary/libfoo.rlib", // testing loading a manually created rlib "tests/ui/include-macros/data.bin", // testing including data with the include macros "tests/ui/include-macros/file.txt", // testing including data with the include macros diff --git a/tests/run-make/target-specs/Makefile b/tests/run-make/target-specs/Makefile index 62d5365a73d03..161b66021857b 100644 --- a/tests/run-make/target-specs/Makefile +++ b/tests/run-make/target-specs/Makefile @@ -9,4 +9,4 @@ all: $(RUSTC) -Z unstable-options --target=my-awesome-platform.json --print target-spec-json > $(TMPDIR)/test-platform.json && $(RUSTC) -Z unstable-options --target=$(TMPDIR)/test-platform.json --print target-spec-json | diff -q $(TMPDIR)/test-platform.json - $(RUSTC) foo.rs --target=definitely-not-builtin-target 2>&1 | $(CGREP) 'may not set is_builtin' $(RUSTC) foo.rs --target=endianness-mismatch 2>&1 | $(CGREP) '"data-layout" claims architecture is little-endian' - $(RUSTC) foo.rs --target=mismatching-data-layout --crate-type=lib + $(RUSTC) foo.rs --target=mismatching-data-layout --crate-type=lib 2>&1 | $(CGREP) 'data-layout for target' diff --git a/tests/ui/codegen/mismatched-data-layout.json b/tests/ui/codegen/mismatched-data-layout.json new file mode 100644 index 0000000000000..4cb0602dc75b5 --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layout.json @@ -0,0 +1,13 @@ +{ + "llvm-target": "x86_64-unknown-none-gnu", + "data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128", + "arch": "x86_64", + "target-endian": "little", + "target-pointer-width": "64", + "target-c-int-width": "32", + "os": "unknown", + "linker-flavor": "ld.lld", + "linker": "rust-lld", + "executables": true +} + diff --git a/tests/ui/codegen/mismatched-data-layouts.rs b/tests/ui/codegen/mismatched-data-layouts.rs new file mode 100644 index 0000000000000..047ec155fdca6 --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layouts.rs @@ -0,0 +1,14 @@ +// This test checks that data layout mismatches emit an error. +// +// build-fail +// needs-llvm-components: x86 +// compile-flags: --crate-type=lib --target={{src-base}}/codegen/mismatched-data-layout.json -Z unstable-options +// error-pattern: differs from LLVM target's +// normalize-stderr-test: "`, `[A-Za-z0-9-:]*`" -> "`, `normalized data layout`" +// normalize-stderr-test: "layout, `[A-Za-z0-9-:]*`" -> "layout, `normalized data layout`" + +#![feature(lang_items, no_core, auto_traits)] +#![no_core] + +#[lang = "sized"] +trait Sized {} diff --git a/tests/ui/codegen/mismatched-data-layouts.stderr b/tests/ui/codegen/mismatched-data-layouts.stderr new file mode 100644 index 0000000000000..1fe242266dff7 --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layouts.stderr @@ -0,0 +1,4 @@ +error: data-layout for target `mismatched-data-layout-7814813422914914169`, `normalized data layout`, differs from LLVM target's `x86_64-unknown-none-gnu` default layout, `normalized data layout` + +error: aborting due to 1 previous error + From bdfc64ac984e892f6061deebaee5a9301b8d271f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 21 Jan 2024 12:18:48 +1100 Subject: [PATCH 04/30] coverage: Add a test that uses `#[bench]` --- tests/coverage/bench.cov-map | 16 ++++++++++++++++ tests/coverage/bench.coverage | 9 +++++++++ tests/coverage/bench.rs | 8 ++++++++ 3 files changed, 33 insertions(+) create mode 100644 tests/coverage/bench.cov-map create mode 100644 tests/coverage/bench.coverage create mode 100644 tests/coverage/bench.rs diff --git a/tests/coverage/bench.cov-map b/tests/coverage/bench.cov-map new file mode 100644 index 0000000000000..7cdb1b641bf3d --- /dev/null +++ b/tests/coverage/bench.cov-map @@ -0,0 +1,16 @@ +Function name: bench::my_bench +Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 00, 27] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 8, 1) to (start + 0, 39) + +Function name: bench::my_bench::{closure#0} +Raw bytes (9): 0x[01, 01, 00, 01, 01, 07, 01, 00, 09] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 7, 1) to (start + 0, 9) + diff --git a/tests/coverage/bench.coverage b/tests/coverage/bench.coverage new file mode 100644 index 0000000000000..4642d92cc1911 --- /dev/null +++ b/tests/coverage/bench.coverage @@ -0,0 +1,9 @@ + LL| |#![feature(test)] + LL| |// edition: 2021 + LL| |// compile-flags: --test + LL| | + LL| |extern crate test; + LL| | + LL| 1|#[bench] + LL| 1|fn my_bench(_b: &mut test::Bencher) {} + diff --git a/tests/coverage/bench.rs b/tests/coverage/bench.rs new file mode 100644 index 0000000000000..2dcd7355b2f79 --- /dev/null +++ b/tests/coverage/bench.rs @@ -0,0 +1,8 @@ +#![feature(test)] +// edition: 2021 +// compile-flags: --test + +extern crate test; + +#[bench] +fn my_bench(_b: &mut test::Bencher) {} From 6d7e80c5bc3cc3b176834322afc50dc8dd100599 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 21 Jan 2024 11:26:28 +1100 Subject: [PATCH 05/30] Add `#[coverage(off)]` to closures introduced by `#[test]`/`#[bench]` --- compiler/rustc_builtin_macros/src/lib.rs | 1 + compiler/rustc_builtin_macros/src/test.rs | 21 +++++++++++++++++---- library/core/src/macros/mod.rs | 4 ++-- tests/coverage/bench.cov-map | 8 -------- tests/coverage/bench.coverage | 2 +- tests/coverage/test_harness.cov-map | 8 -------- tests/coverage/test_harness.coverage | 2 +- tests/pretty/tests-are-sorted.pp | 9 ++++++--- 8 files changed, 28 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/lib.rs b/compiler/rustc_builtin_macros/src/lib.rs index f60b73fbe9b13..3e9b06a5b054b 100644 --- a/compiler/rustc_builtin_macros/src/lib.rs +++ b/compiler/rustc_builtin_macros/src/lib.rs @@ -6,6 +6,7 @@ #![doc(rust_logo)] #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![feature(array_windows)] +#![feature(assert_matches)] #![feature(box_patterns)] #![feature(decl_macro)] #![feature(if_let_guard)] diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index 4d44e340ae149..0631f796894b1 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -9,6 +9,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder, Level}; use rustc_expand::base::*; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::{ErrorGuaranteed, FileNameDisplayPreference, Span}; +use std::assert_matches::assert_matches; use std::iter; use thin_vec::{thin_vec, ThinVec}; @@ -182,6 +183,16 @@ pub fn expand_test_or_bench( // creates $name: $expr let field = |name, expr| cx.field_imm(sp, Ident::from_str_and_span(name, sp), expr); + // Adds `#[coverage(off)]` to a closure, so it won't be instrumented in + // `-Cinstrument-coverage` builds. + // This requires `#[allow_internal_unstable(coverage_attribute)]` on the + // corresponding macro declaration in `core::macros`. + let coverage_off = |mut expr: P| { + assert_matches!(expr.kind, ast::ExprKind::Closure(_)); + expr.attrs.push(cx.attr_nested_word(sym::coverage, sym::off, sp)); + expr + }; + let test_fn = if is_bench { // A simple ident for a lambda let b = Ident::from_str_and_span("b", attr_sp); @@ -190,8 +201,9 @@ pub fn expand_test_or_bench( sp, cx.expr_path(test_path("StaticBenchFn")), thin_vec![ + // #[coverage(off)] // |b| self::test::assert_test_result( - cx.lambda1( + coverage_off(cx.lambda1( sp, cx.expr_call( sp, @@ -206,7 +218,7 @@ pub fn expand_test_or_bench( ], ), b, - ), // ) + )), // ) ], ) } else { @@ -214,8 +226,9 @@ pub fn expand_test_or_bench( sp, cx.expr_path(test_path("StaticTestFn")), thin_vec![ + // #[coverage(off)] // || { - cx.lambda0( + coverage_off(cx.lambda0( sp, // test::assert_test_result( cx.expr_call( @@ -230,7 +243,7 @@ pub fn expand_test_or_bench( ), // ) ], ), // } - ), // ) + )), // ) ], ) }; diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index a2437feeeb9cf..9bbaf62a5caaf 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -1596,7 +1596,7 @@ pub(crate) mod builtin { /// /// [the reference]: ../../../reference/attributes/testing.html#the-test-attribute #[stable(feature = "rust1", since = "1.0.0")] - #[allow_internal_unstable(test, rustc_attrs)] + #[allow_internal_unstable(test, rustc_attrs, coverage_attribute)] #[rustc_builtin_macro] pub macro test($item:item) { /* compiler built-in */ @@ -1609,7 +1609,7 @@ pub(crate) mod builtin { soft, reason = "`bench` is a part of custom test frameworks which are unstable" )] - #[allow_internal_unstable(test, rustc_attrs)] + #[allow_internal_unstable(test, rustc_attrs, coverage_attribute)] #[rustc_builtin_macro] pub macro bench($item:item) { /* compiler built-in */ diff --git a/tests/coverage/bench.cov-map b/tests/coverage/bench.cov-map index 7cdb1b641bf3d..aa702a4868185 100644 --- a/tests/coverage/bench.cov-map +++ b/tests/coverage/bench.cov-map @@ -6,11 +6,3 @@ Number of expressions: 0 Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 8, 1) to (start + 0, 39) -Function name: bench::my_bench::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 07, 01, 00, 09] -Number of files: 1 -- file 0 => global file 1 -Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 7, 1) to (start + 0, 9) - diff --git a/tests/coverage/bench.coverage b/tests/coverage/bench.coverage index 4642d92cc1911..64945dc641502 100644 --- a/tests/coverage/bench.coverage +++ b/tests/coverage/bench.coverage @@ -4,6 +4,6 @@ LL| | LL| |extern crate test; LL| | - LL| 1|#[bench] + LL| |#[bench] LL| 1|fn my_bench(_b: &mut test::Bencher) {} diff --git a/tests/coverage/test_harness.cov-map b/tests/coverage/test_harness.cov-map index 6940d2e282430..f75328b11c9ab 100644 --- a/tests/coverage/test_harness.cov-map +++ b/tests/coverage/test_harness.cov-map @@ -6,14 +6,6 @@ Number of expressions: 0 Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 10, 1) to (start + 0, 16) -Function name: test_harness::my_test::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 09, 01, 00, 08] -Number of files: 1 -- file 0 => global file 1 -Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 9, 1) to (start + 0, 8) - Function name: test_harness::unused (unused) Raw bytes (9): 0x[01, 01, 00, 01, 00, 07, 01, 00, 0f] Number of files: 1 diff --git a/tests/coverage/test_harness.coverage b/tests/coverage/test_harness.coverage index ff6009f6fce43..c3f660506fbd8 100644 --- a/tests/coverage/test_harness.coverage +++ b/tests/coverage/test_harness.coverage @@ -6,6 +6,6 @@ LL| |#[allow(dead_code)] LL| 0|fn unused() {} LL| | - LL| 1|#[test] + LL| |#[test] LL| 1|fn my_test() {} diff --git a/tests/pretty/tests-are-sorted.pp b/tests/pretty/tests-are-sorted.pp index fd9386be8f3c3..fbdad0c323f34 100644 --- a/tests/pretty/tests-are-sorted.pp +++ b/tests/pretty/tests-are-sorted.pp @@ -28,7 +28,8 @@ should_panic: test::ShouldPanic::No, test_type: test::TestType::Unknown, }, - testfn: test::StaticTestFn(|| test::assert_test_result(m_test())), + testfn: test::StaticTestFn(#[coverage(off)] || + test::assert_test_result(m_test())), }; fn m_test() {} @@ -51,7 +52,8 @@ should_panic: test::ShouldPanic::No, test_type: test::TestType::Unknown, }, - testfn: test::StaticTestFn(|| test::assert_test_result(z_test())), + testfn: test::StaticTestFn(#[coverage(off)] || + test::assert_test_result(z_test())), }; #[ignore = "not yet implemented"] fn z_test() {} @@ -75,7 +77,8 @@ should_panic: test::ShouldPanic::No, test_type: test::TestType::Unknown, }, - testfn: test::StaticTestFn(|| test::assert_test_result(a_test())), + testfn: test::StaticTestFn(#[coverage(off)] || + test::assert_test_result(a_test())), }; fn a_test() {} #[rustc_main] From 0e3035b512fb020e5f4cee9b151a3e79a86ed144 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 20 Jan 2024 15:11:53 +0100 Subject: [PATCH 06/30] Manually implement derived `NonZero` traits. --- library/core/src/num/nonzero.rs | 109 +++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 8 deletions(-) diff --git a/library/core/src/num/nonzero.rs b/library/core/src/num/nonzero.rs index bda691b16d4a7..2a81146601bea 100644 --- a/library/core/src/num/nonzero.rs +++ b/library/core/src/num/nonzero.rs @@ -1,6 +1,9 @@ //! Definitions of integer that is known not to equal zero. +use crate::cmp::Ordering; use crate::fmt; +use crate::hash::{Hash, Hasher}; +use crate::marker::StructuralPartialEq; use crate::ops::{BitOr, BitOrAssign, Div, Neg, Rem}; use crate::str::FromStr; @@ -31,13 +34,6 @@ pub trait ZeroablePrimitive: Sized + Copy + private::Sealed { type NonZero; } -#[unstable( - feature = "nonzero_internals", - reason = "implementation detail which may disappear or be replaced at any time", - issue = "none" -)] -pub(crate) type NonZero = ::NonZero; - macro_rules! impl_zeroable_primitive { ($NonZero:ident ( $primitive:ty )) => { #[unstable( @@ -71,6 +67,13 @@ impl_zeroable_primitive!(NonZeroI64(i64)); impl_zeroable_primitive!(NonZeroI128(i128)); impl_zeroable_primitive!(NonZeroIsize(isize)); +#[unstable( + feature = "nonzero_internals", + reason = "implementation detail which may disappear or be replaced at any time", + issue = "none" +)] +pub(crate) type NonZero = ::NonZero; + macro_rules! impl_nonzero_fmt { ( #[$stability: meta] ( $( $Trait: ident ),+ ) for $Ty: ident ) => { $( @@ -128,7 +131,7 @@ macro_rules! nonzero_integer { /// /// [null pointer optimization]: crate::option#representation #[$stability] - #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] + #[derive(Copy, Eq)] #[repr(transparent)] #[rustc_layout_scalar_valid_range_start(1)] #[rustc_nonnull_optimization_guaranteed] @@ -494,6 +497,96 @@ macro_rules! nonzero_integer { } } + #[$stability] + impl Clone for $Ty { + #[inline] + fn clone(&self) -> Self { + // SAFETY: The contained value is non-zero. + unsafe { Self(self.0) } + } + } + + #[$stability] + impl PartialEq for $Ty { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } + + #[inline] + fn ne(&self, other: &Self) -> bool { + self.0 != other.0 + } + } + + #[unstable(feature = "structural_match", issue = "31434")] + impl StructuralPartialEq for $Ty {} + + #[$stability] + impl PartialOrd for $Ty { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + self.0.partial_cmp(&other.0) + } + + #[inline] + fn lt(&self, other: &Self) -> bool { + self.0 < other.0 + } + + #[inline] + fn le(&self, other: &Self) -> bool { + self.0 <= other.0 + } + + #[inline] + fn gt(&self, other: &Self) -> bool { + self.0 > other.0 + } + + #[inline] + fn ge(&self, other: &Self) -> bool { + self.0 >= other.0 + } + } + + #[$stability] + impl Ord for $Ty { + #[inline] + fn cmp(&self, other: &Self) -> Ordering { + self.0.cmp(&other.0) + } + + #[inline] + fn max(self, other: Self) -> Self { + // SAFETY: The maximum of two non-zero values is still non-zero. + unsafe { Self(self.0.max(other.0)) } + } + + #[inline] + fn min(self, other: Self) -> Self { + // SAFETY: The minimum of two non-zero values is still non-zero. + unsafe { Self(self.0.min(other.0)) } + } + + #[inline] + fn clamp(self, min: Self, max: Self) -> Self { + // SAFETY: A non-zero value clamped between two non-zero values is still non-zero. + unsafe { Self(self.0.clamp(min.0, max.0)) } + } + } + + #[$stability] + impl Hash for $Ty { + #[inline] + fn hash(&self, state: &mut H) + where + H: Hasher, + { + self.0.hash(state) + } + } + #[stable(feature = "from_nonzero", since = "1.31.0")] impl From<$Ty> for $Int { #[doc = concat!("Converts a `", stringify!($Ty), "` into an `", stringify!($Int), "`")] From f58af9ba28f1bb49f880da43227c65cc10673be2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jan 2024 11:12:30 +0000 Subject: [PATCH 07/30] Add a simpler and more targetted code path for impl trait in assoc items --- .../rustc_hir_analysis/src/collect/type_of.rs | 6 +- .../src/collect/type_of/opaque.rs | 71 +++++++++++++++++-- compiler/rustc_middle/src/query/mod.rs | 9 +++ compiler/rustc_ty_utils/src/opaque_types.rs | 10 ++- 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index bfa9dc42422cb..3674a760cbf9d 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -530,9 +530,13 @@ pub(super) fn type_of_opaque( Ok(ty::EarlyBinder::bind(match tcx.hir_node_by_def_id(def_id) { Node::Item(item) => match item.kind { ItemKind::OpaqueTy(OpaqueTy { - origin: hir::OpaqueTyOrigin::TyAlias { .. }, + origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: false }, .. }) => opaque::find_opaque_ty_constraints_for_tait(tcx, def_id), + ItemKind::OpaqueTy(OpaqueTy { + origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: true }, + .. + }) => opaque::find_opaque_ty_constraints_for_impl_trait_in_assoc_type(tcx, def_id), // Opaque types desugared from `impl Trait`. ItemKind::OpaqueTy(&OpaqueTy { origin: diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index 85093bc12b378..51b2401531c7a 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -23,6 +23,60 @@ pub fn test_opaque_hidden_types(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> res } +/// Checks "defining uses" of opaque `impl Trait` in associated types. +/// These can only be defined by associated items of the same trait. +#[instrument(skip(tcx), level = "debug")] +pub(super) fn find_opaque_ty_constraints_for_impl_trait_in_assoc_type( + tcx: TyCtxt<'_>, + def_id: LocalDefId, +) -> Ty<'_> { + let mut parent_def_id = def_id; + while tcx.def_kind(parent_def_id) == def::DefKind::OpaqueTy { + // Account for `type Alias = impl Trait;` (#116031) + parent_def_id = tcx.local_parent(parent_def_id); + } + let impl_def_id = tcx.local_parent(parent_def_id); + match tcx.def_kind(impl_def_id) { + DefKind::Impl { .. } => {} + other => bug!("invalid impl trait in assoc type parent: {other:?}"), + } + + let mut locator = TaitConstraintLocator { def_id, tcx, found: None, typeck_types: vec![] }; + + for &assoc_id in tcx.associated_item_def_ids(impl_def_id) { + let assoc = tcx.associated_item(assoc_id); + match assoc.kind { + ty::AssocKind::Const | ty::AssocKind::Fn => { + locator.check(assoc_id.expect_local(), true) + } + // Associated types don't have bodies, so they can't constrain hidden types + ty::AssocKind::Type => {} + } + } + + if let Some(hidden) = locator.found { + // Only check against typeck if we didn't already error + if !hidden.ty.references_error() { + for concrete_type in locator.typeck_types { + if concrete_type.ty != tcx.erase_regions(hidden.ty) + && !(concrete_type, hidden).references_error() + { + hidden.report_mismatch(&concrete_type, def_id, tcx).emit(); + } + } + } + + hidden.ty + } else { + let reported = tcx.dcx().emit_err(UnconstrainedOpaqueType { + span: tcx.def_span(def_id), + name: tcx.item_name(parent_def_id.to_def_id()), + what: "impl", + }); + Ty::new_error(tcx, reported) + } +} + /// Checks "defining uses" of opaque `impl Trait` types to ensure that they meet the restrictions /// laid for "higher-order pattern unification". /// This ensures that inference is tractable. @@ -130,7 +184,7 @@ struct TaitConstraintLocator<'tcx> { impl TaitConstraintLocator<'_> { #[instrument(skip(self), level = "debug")] - fn check(&mut self, item_def_id: LocalDefId) { + fn check(&mut self, item_def_id: LocalDefId, impl_trait_in_assoc_type: bool) { // Don't try to check items that cannot possibly constrain the type. if !self.tcx.has_typeck_results(item_def_id) { debug!("no constraint: no typeck results"); @@ -182,7 +236,12 @@ impl TaitConstraintLocator<'_> { continue; } constrained = true; - if !self.tcx.opaque_types_defined_by(item_def_id).contains(&self.def_id) { + let opaque_types_defined_by = if impl_trait_in_assoc_type { + self.tcx.impl_trait_in_assoc_types_defined_by(item_def_id) + } else { + self.tcx.opaque_types_defined_by(item_def_id) + }; + if !opaque_types_defined_by.contains(&self.def_id) { self.tcx.dcx().emit_err(TaitForwardCompat { span: hidden_type.span, item_span: self @@ -240,7 +299,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { } fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { if let hir::ExprKind::Closure(closure) = ex.kind { - self.check(closure.def_id); + self.check(closure.def_id, false); } intravisit::walk_expr(self, ex); } @@ -248,7 +307,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { trace!(?it.owner_id); // The opaque type itself or its children are not within its reveal scope. if it.owner_id.def_id != self.def_id { - self.check(it.owner_id.def_id); + self.check(it.owner_id.def_id, false); intravisit::walk_item(self, it); } } @@ -256,13 +315,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { trace!(?it.owner_id); // The opaque type itself or its children are not within its reveal scope. if it.owner_id.def_id != self.def_id { - self.check(it.owner_id.def_id); + self.check(it.owner_id.def_id, false); intravisit::walk_impl_item(self, it); } } fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) { trace!(?it.owner_id); - self.check(it.owner_id.def_id); + self.check(it.owner_id.def_id, false); intravisit::walk_trait_item(self, it); } fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index a9dc7f5d11a20..23daefd5a65b9 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -343,6 +343,15 @@ rustc_queries! { } } + query impl_trait_in_assoc_types_defined_by( + key: LocalDefId + ) -> &'tcx ty::List { + desc { + |tcx| "computing the opaque types defined by `{}`", + tcx.def_path_str(key.to_def_id()) + } + } + /// Returns the list of bounds that can be used for /// `SelectionCandidate::ProjectionCandidate(_)` and /// `ProjectionTyCandidate::TraitDef`. diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 0bb833c66fa76..eae4c57fc9989 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -272,6 +272,13 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { } } +fn impl_trait_in_assoc_types_defined_by<'tcx>( + tcx: TyCtxt<'tcx>, + item: LocalDefId, +) -> &'tcx ty::List { + opaque_types_defined_by(tcx, item) +} + fn opaque_types_defined_by<'tcx>( tcx: TyCtxt<'tcx>, item: LocalDefId, @@ -321,5 +328,6 @@ fn opaque_types_defined_by<'tcx>( } pub(super) fn provide(providers: &mut Providers) { - *providers = Providers { opaque_types_defined_by, ..*providers }; + *providers = + Providers { opaque_types_defined_by, impl_trait_in_assoc_types_defined_by, ..*providers }; } From ac332bd9163990491d225e2a24ff92af89030dac Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jan 2024 11:16:32 +0000 Subject: [PATCH 08/30] Pull opaque type check into a separate method --- compiler/rustc_ty_utils/src/opaque_types.rs | 125 ++++++++++---------- 1 file changed, 64 insertions(+), 61 deletions(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index eae4c57fc9989..1d9c0405b7d76 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -118,6 +118,69 @@ impl<'tcx> OpaqueTypeCollector<'tcx> { } TaitInBodyFinder { collector: self }.visit_expr(body); } + + fn visit_opaque_ty(&mut self, alias_ty: &ty::AliasTy<'tcx>) { + if !self.seen.insert(alias_ty.def_id.expect_local()) { + return; + } + + // TAITs outside their defining scopes are ignored. + let origin = self.tcx.opaque_type_origin(alias_ty.def_id.expect_local()); + trace!(?origin); + match origin { + rustc_hir::OpaqueTyOrigin::FnReturn(_) | rustc_hir::OpaqueTyOrigin::AsyncFn(_) => {} + rustc_hir::OpaqueTyOrigin::TyAlias { in_assoc_ty } => { + if !in_assoc_ty { + if !self.check_tait_defining_scope(alias_ty.def_id.expect_local()) { + return; + } + } + } + } + + self.opaques.push(alias_ty.def_id.expect_local()); + + let parent_count = self.tcx.generics_of(alias_ty.def_id).parent_count; + // Only check that the parent generics of the TAIT/RPIT are unique. + // the args owned by the opaque are going to always be duplicate + // lifetime params for RPITs, and empty for TAITs. + match self + .tcx + .uses_unique_generic_params(&alias_ty.args[..parent_count], CheckRegions::FromFunction) + { + Ok(()) => { + // FIXME: implement higher kinded lifetime bounds on nested opaque types. They are not + // supported at all, so this is sound to do, but once we want to support them, you'll + // start seeing the error below. + + // Collect opaque types nested within the associated type bounds of this opaque type. + // We use identity args here, because we already know that the opaque type uses + // only generic parameters, and thus substituting would not give us more information. + for (pred, span) in self + .tcx + .explicit_item_bounds(alias_ty.def_id) + .instantiate_identity_iter_copied() + { + trace!(?pred); + self.visit_spanned(span, pred); + } + } + Err(NotUniqueParam::NotParam(arg)) => { + self.tcx.dcx().emit_err(NotParam { + arg, + span: self.span(), + opaque_span: self.tcx.def_span(alias_ty.def_id), + }); + } + Err(NotUniqueParam::DuplicateParam(arg)) => { + self.tcx.dcx().emit_err(DuplicateArg { + arg, + span: self.span(), + opaque_span: self.tcx.def_span(alias_ty.def_id), + }); + } + } + } } impl<'tcx> super::sig_types::SpannedTypeVisitor<'tcx> for OpaqueTypeCollector<'tcx> { @@ -134,67 +197,7 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { t.super_visit_with(self)?; match t.kind() { ty::Alias(ty::Opaque, alias_ty) if alias_ty.def_id.is_local() => { - if !self.seen.insert(alias_ty.def_id.expect_local()) { - return ControlFlow::Continue(()); - } - - // TAITs outside their defining scopes are ignored. - let origin = self.tcx.opaque_type_origin(alias_ty.def_id.expect_local()); - trace!(?origin); - match origin { - rustc_hir::OpaqueTyOrigin::FnReturn(_) - | rustc_hir::OpaqueTyOrigin::AsyncFn(_) => {} - rustc_hir::OpaqueTyOrigin::TyAlias { in_assoc_ty } => { - if !in_assoc_ty { - if !self.check_tait_defining_scope(alias_ty.def_id.expect_local()) { - return ControlFlow::Continue(()); - } - } - } - } - - self.opaques.push(alias_ty.def_id.expect_local()); - - let parent_count = self.tcx.generics_of(alias_ty.def_id).parent_count; - // Only check that the parent generics of the TAIT/RPIT are unique. - // the args owned by the opaque are going to always be duplicate - // lifetime params for RPITs, and empty for TAITs. - match self.tcx.uses_unique_generic_params( - &alias_ty.args[..parent_count], - CheckRegions::FromFunction, - ) { - Ok(()) => { - // FIXME: implement higher kinded lifetime bounds on nested opaque types. They are not - // supported at all, so this is sound to do, but once we want to support them, you'll - // start seeing the error below. - - // Collect opaque types nested within the associated type bounds of this opaque type. - // We use identity args here, because we already know that the opaque type uses - // only generic parameters, and thus substituting would not give us more information. - for (pred, span) in self - .tcx - .explicit_item_bounds(alias_ty.def_id) - .instantiate_identity_iter_copied() - { - trace!(?pred); - self.visit_spanned(span, pred); - } - } - Err(NotUniqueParam::NotParam(arg)) => { - self.tcx.dcx().emit_err(NotParam { - arg, - span: self.span(), - opaque_span: self.tcx.def_span(alias_ty.def_id), - }); - } - Err(NotUniqueParam::DuplicateParam(arg)) => { - self.tcx.dcx().emit_err(DuplicateArg { - arg, - span: self.span(), - opaque_span: self.tcx.def_span(alias_ty.def_id), - }); - } - } + self.visit_opaque_ty(alias_ty); } ty::Alias(ty::Weak, alias_ty) if alias_ty.def_id.is_local() => { self.tcx From f75361fec7aecfcede9cf6f0b97f2b1e7756e47b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jan 2024 11:28:05 +0000 Subject: [PATCH 09/30] Limit impl trait in assoc type defining scope --- compiler/rustc_ty_utils/src/opaque_types.rs | 80 ++++++++++++++++++- .../hidden_behind_struct_field2.rs | 8 +- .../hidden_behind_struct_field2.stderr | 15 ++++ 3 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.stderr diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 1d9c0405b7d76..ef67317a601ac 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -275,11 +275,89 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { } } +struct ImplTraitInAssocTypeCollector<'tcx>(OpaqueTypeCollector<'tcx>); + +impl<'tcx> super::sig_types::SpannedTypeVisitor<'tcx> for ImplTraitInAssocTypeCollector<'tcx> { + #[instrument(skip(self), ret, level = "trace")] + fn visit(&mut self, span: Span, value: impl TypeVisitable>) -> ControlFlow { + let old = self.0.span; + self.0.span = Some(span); + value.visit_with(self); + self.0.span = old; + + ControlFlow::Continue(()) + } +} + +impl<'tcx> TypeVisitor> for ImplTraitInAssocTypeCollector<'tcx> { + #[instrument(skip(self), ret, level = "trace")] + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + t.super_visit_with(self)?; + match t.kind() { + ty::Alias(ty::Opaque, alias_ty) if alias_ty.def_id.is_local() => { + self.0.visit_opaque_ty(alias_ty); + } + ty::Alias(ty::Projection, alias_ty) => { + // This avoids having to do normalization of `Self::AssocTy` by only + // supporting the case of a method defining opaque types from assoc types + // in the same impl block. + let parent_trait_ref = self + .0 + .parent_trait_ref() + .expect("impl trait in assoc type collector used on non-assoc item"); + // If the trait ref of the associated item and the impl differs, + // then we can't use the impl's identity substitutions below, so + // just skip. + if alias_ty.trait_ref(self.0.tcx) == parent_trait_ref { + let parent = self.0.parent().expect("we should have a parent here"); + + for &assoc in self.0.tcx.associated_items(parent).in_definition_order() { + trace!(?assoc); + if assoc.trait_item_def_id != Some(alias_ty.def_id) { + continue; + } + + // If the type is further specializable, then the type_of + // is not actually correct below. + if !assoc.defaultness(self.0.tcx).is_final() { + continue; + } + + let impl_args = alias_ty.args.rebase_onto( + self.0.tcx, + parent_trait_ref.def_id, + ty::GenericArgs::identity_for_item(self.0.tcx, parent), + ); + + if check_args_compatible(self.0.tcx, assoc, impl_args) { + return self + .0 + .tcx + .type_of(assoc.def_id) + .instantiate(self.0.tcx, impl_args) + .visit_with(self); + } else { + self.0.tcx.dcx().span_delayed_bug( + self.0.tcx.def_span(assoc.def_id), + "item had incorrect args", + ); + } + } + } + } + _ => trace!(kind=?t.kind()), + } + ControlFlow::Continue(()) + } +} + fn impl_trait_in_assoc_types_defined_by<'tcx>( tcx: TyCtxt<'tcx>, item: LocalDefId, ) -> &'tcx ty::List { - opaque_types_defined_by(tcx, item) + let mut collector = ImplTraitInAssocTypeCollector(OpaqueTypeCollector::new(tcx, item)); + super::sig_types::walk_types(tcx, item, &mut collector); + tcx.mk_local_def_ids(&collector.0.opaques) } fn opaque_types_defined_by<'tcx>( diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs index e440dce5e514f..4c881dd133086 100644 --- a/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs +++ b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs @@ -1,9 +1,8 @@ -//! This test shows that we can even follow projections -//! into associated types of the same impl if they are -//! indirectly mentioned in a struct field. +//! This test shows that we do not treat opaque types +//! as defined by a method if the opaque type is +//! only indirectly mentioned in a struct field. #![feature(impl_trait_in_assoc_type)] -// check-pass struct Bar; @@ -16,6 +15,7 @@ impl Trait for Bar { type Assoc = impl std::fmt::Debug; fn foo() -> Foo { Foo { field: () } + //~^ ERROR: item constrains opaque type that is not in its signature } } diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.stderr b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.stderr new file mode 100644 index 0000000000000..5c53dfa3a7500 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.stderr @@ -0,0 +1,15 @@ +error: item constrains opaque type that is not in its signature + --> $DIR/hidden_behind_struct_field2.rs:17:22 + | +LL | Foo { field: () } + | ^^ + | + = note: this item must mention the opaque type in its signature in order to be able to register hidden types +note: this item must mention the opaque type in its signature in order to be able to register hidden types + --> $DIR/hidden_behind_struct_field2.rs:16:8 + | +LL | fn foo() -> Foo { + | ^^^ + +error: aborting due to 1 previous error + From 4e0769956bf5c966fccb54acbb0866f029c79818 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jan 2024 14:12:59 +0000 Subject: [PATCH 10/30] Add some tests --- tests/ui/impl-trait/rpit/early_bound.rs | 13 ++++++ tests/ui/impl-trait/rpit/early_bound.stderr | 26 ++++++++++++ ...pl_trait_in_trait_defined_outside_trait.rs | 38 +++++++++++++++++ ...rait_in_trait_defined_outside_trait.stderr | 41 +++++++++++++++++++ ...l_trait_in_trait_defined_outside_trait2.rs | 22 ++++++++++ ...ait_in_trait_defined_outside_trait2.stderr | 17 ++++++++ ...l_trait_in_trait_defined_outside_trait3.rs | 38 +++++++++++++++++ .../in-assoc-ty-early-bound.rs | 17 ++++++++ .../in-assoc-ty-early-bound.stderr | 13 ++++++ .../in-assoc-ty-early-bound2.rs | 21 ++++++++++ .../in-assoc-ty-early-bound2.stderr | 22 ++++++++++ 11 files changed, 268 insertions(+) create mode 100644 tests/ui/impl-trait/rpit/early_bound.rs create mode 100644 tests/ui/impl-trait/rpit/early_bound.stderr create mode 100644 tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.rs create mode 100644 tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.stderr create mode 100644 tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.rs create mode 100644 tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.stderr create mode 100644 tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait3.rs create mode 100644 tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.rs create mode 100644 tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.stderr create mode 100644 tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.rs create mode 100644 tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.stderr diff --git a/tests/ui/impl-trait/rpit/early_bound.rs b/tests/ui/impl-trait/rpit/early_bound.rs new file mode 100644 index 0000000000000..03bd64d4d76d0 --- /dev/null +++ b/tests/ui/impl-trait/rpit/early_bound.rs @@ -0,0 +1,13 @@ +use std::convert::identity; + +fn test<'a: 'a>(n: bool) -> impl Sized + 'a { + //~^ ERROR concrete type differs from previous defining opaque type use + let true = n else { loop {} }; + let _ = || { + let _ = identity::<&'a ()>(test(false)); + //~^ ERROR hidden type for `impl Sized + 'a` captures lifetime that does not appear in bounds + }; + loop {} +} + +fn main() {} diff --git a/tests/ui/impl-trait/rpit/early_bound.stderr b/tests/ui/impl-trait/rpit/early_bound.stderr new file mode 100644 index 0000000000000..815368f250e35 --- /dev/null +++ b/tests/ui/impl-trait/rpit/early_bound.stderr @@ -0,0 +1,26 @@ +error[E0700]: hidden type for `impl Sized + 'a` captures lifetime that does not appear in bounds + --> $DIR/early_bound.rs:7:17 + | +LL | fn test<'a: 'a>(n: bool) -> impl Sized + 'a { + | -- --------------- opaque type defined here + | | + | hidden type `&'a ()` captures the lifetime `'a` as defined here +... +LL | let _ = identity::<&'a ()>(test(false)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: concrete type differs from previous defining opaque type use + --> $DIR/early_bound.rs:3:29 + | +LL | fn test<'a: 'a>(n: bool) -> impl Sized + 'a { + | ^^^^^^^^^^^^^^^ expected `&()`, got `()` + | +note: previous use here + --> $DIR/early_bound.rs:7:36 + | +LL | let _ = identity::<&'a ()>(test(false)); + | ^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0700`. diff --git a/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.rs b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.rs new file mode 100644 index 0000000000000..a788563ab779e --- /dev/null +++ b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.rs @@ -0,0 +1,38 @@ +//! Check that we cannot instantiate a hidden type in the body +//! of an assoc fn or const unless mentioned in the signature. + +#![feature(impl_trait_in_assoc_type)] + +trait Trait: Sized { + type Assoc; + fn foo(); + fn bar() -> Self::Assoc; +} + +impl Trait for () { + type Assoc = impl std::fmt::Debug; + fn foo() { + let x: Self::Assoc = 42; //~ ERROR: mismatched types + } + fn bar() -> Self::Assoc { + "" + } +} + +trait Trait2: Sized { + type Assoc; + const FOO: (); + fn bar() -> Self::Assoc; +} + +impl Trait2 for () { + type Assoc = impl std::fmt::Debug; + const FOO: () = { + let x: Self::Assoc = 42; //~ ERROR: mismatched types + }; + fn bar() -> Self::Assoc { + "" + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.stderr b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.stderr new file mode 100644 index 0000000000000..1d7a97c536713 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait.stderr @@ -0,0 +1,41 @@ +error[E0308]: mismatched types + --> $DIR/impl_trait_in_trait_defined_outside_trait.rs:15:30 + | +LL | type Assoc = impl std::fmt::Debug; + | -------------------- the expected opaque type +LL | fn foo() { +LL | let x: Self::Assoc = 42; + | ----------- ^^ expected opaque type, found integer + | | + | expected due to this + | + = note: expected opaque type `<() as Trait>::Assoc` + found type `{integer}` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/impl_trait_in_trait_defined_outside_trait.rs:14:8 + | +LL | fn foo() { + | ^^^ + +error[E0308]: mismatched types + --> $DIR/impl_trait_in_trait_defined_outside_trait.rs:31:30 + | +LL | type Assoc = impl std::fmt::Debug; + | -------------------- the expected opaque type +LL | const FOO: () = { +LL | let x: Self::Assoc = 42; + | ----------- ^^ expected opaque type, found integer + | | + | expected due to this + | + = note: expected opaque type `<() as Trait2>::Assoc` + found type `{integer}` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/impl_trait_in_trait_defined_outside_trait.rs:30:11 + | +LL | const FOO: () = { + | ^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.rs b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.rs new file mode 100644 index 0000000000000..77cdca198daea --- /dev/null +++ b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.rs @@ -0,0 +1,22 @@ +//! Check that we cannot instantiate a hidden type from another assoc type. + +#![feature(impl_trait_in_assoc_type)] + +trait Trait: Sized { + type Assoc; + type Foo; + fn foo() -> Self::Assoc; +} + +impl Trait for () { + type Assoc = impl std::fmt::Debug; + type Foo = [(); { + let x: Self::Assoc = 42; //~ ERROR: mismatched types + 3 + }]; + fn foo() -> Self::Assoc { + "" + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.stderr b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.stderr new file mode 100644 index 0000000000000..708c3f28d2d2e --- /dev/null +++ b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait2.stderr @@ -0,0 +1,17 @@ +error[E0308]: mismatched types + --> $DIR/impl_trait_in_trait_defined_outside_trait2.rs:14:30 + | +LL | type Assoc = impl std::fmt::Debug; + | -------------------- the expected opaque type +LL | type Foo = [(); { +LL | let x: Self::Assoc = 42; + | ----------- ^^ expected opaque type, found integer + | | + | expected due to this + | + = note: expected opaque type `<() as Trait>::Assoc` + found type `{integer}` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait3.rs b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait3.rs new file mode 100644 index 0000000000000..dfcf733653376 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/impl_trait_in_trait_defined_outside_trait3.rs @@ -0,0 +1,38 @@ +//! Check that non-defining assoc items can use the opaque type +//! opaquely. + +// check-pass + +#![feature(impl_trait_in_assoc_type)] + +trait Trait: Sized { + type Assoc; + fn foo(); + fn bar() -> Self::Assoc; +} + +impl Trait for () { + type Assoc = impl std::fmt::Debug; + fn foo() { + let x: Self::Assoc = Self::bar(); + } + fn bar() -> Self::Assoc { + "" + } +} + +trait Trait2: Sized { + type Assoc; + const FOO: (); + const BAR: Self::Assoc; +} + +impl Trait2 for () { + type Assoc = impl Copy; + const FOO: () = { + let x: Self::Assoc = Self::BAR; + }; + const BAR: Self::Assoc = ""; +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.rs b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.rs new file mode 100644 index 0000000000000..baeba1d3de635 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.rs @@ -0,0 +1,17 @@ +#![feature(impl_trait_in_assoc_type)] + +trait Foo { + type Assoc<'a, 'b>; + fn bar<'a: 'a, 'b: 'b>(_: &'a ()) -> Self::Assoc<'a, 'b>; +} + +impl Foo for () { + type Assoc<'a, 'b> = impl Sized; + fn bar<'a: 'a, 'b: 'b>(x: &'a ()) -> Self::Assoc<'a, 'b> { + let closure = |x: &'a ()| -> Self::Assoc<'b, 'a> { x }; + //~^ ERROR `<() as Foo>::Assoc<'b, 'a>` captures lifetime that does not appear in bounds + x + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.stderr b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.stderr new file mode 100644 index 0000000000000..a7d3e7f0be403 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound.stderr @@ -0,0 +1,13 @@ +error[E0700]: hidden type for `<() as Foo>::Assoc<'b, 'a>` captures lifetime that does not appear in bounds + --> $DIR/in-assoc-ty-early-bound.rs:11:60 + | +LL | type Assoc<'a, 'b> = impl Sized; + | ---------- opaque type defined here +LL | fn bar<'a: 'a, 'b: 'b>(x: &'a ()) -> Self::Assoc<'a, 'b> { + | -- hidden type `&'a ()` captures the lifetime `'a` as defined here +LL | let closure = |x: &'a ()| -> Self::Assoc<'b, 'a> { x }; + | ^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0700`. diff --git a/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.rs b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.rs new file mode 100644 index 0000000000000..7452000b65dc8 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.rs @@ -0,0 +1,21 @@ +#![feature(impl_trait_in_assoc_type)] + +trait Foo { + type Assoc<'a>; + fn bar<'a: 'a>(); +} + +impl Foo for () { + type Assoc<'a> = impl Sized; //~ ERROR unconstrained opaque type + fn bar<'a: 'a>() + where + Self::Assoc<'a>:, + { + let _ = |x: &'a ()| { + let _: Self::Assoc<'a> = x; + //~^ ERROR `<() as Foo>::Assoc<'a>` captures lifetime that does not appear in bound + }; + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.stderr b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.stderr new file mode 100644 index 0000000000000..1274a8b60dea0 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/in-assoc-ty-early-bound2.stderr @@ -0,0 +1,22 @@ +error[E0700]: hidden type for `<() as Foo>::Assoc<'a>` captures lifetime that does not appear in bounds + --> $DIR/in-assoc-ty-early-bound2.rs:15:20 + | +LL | type Assoc<'a> = impl Sized; + | ---------- opaque type defined here +LL | fn bar<'a: 'a>() + | -- hidden type `&'a ()` captures the lifetime `'a` as defined here +... +LL | let _: Self::Assoc<'a> = x; + | ^^^^^^^^^^^^^^^ + +error: unconstrained opaque type + --> $DIR/in-assoc-ty-early-bound2.rs:9:22 + | +LL | type Assoc<'a> = impl Sized; + | ^^^^^^^^^^ + | + = note: `Assoc` must be used in combination with a concrete type within the same impl + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0700`. From 1829aa631fe3df4db1217922ec420e7c57501d50 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 16 Jan 2024 09:21:54 +0000 Subject: [PATCH 11/30] Use an enum instead of a bool --- .../src/collect/type_of/opaque.rs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index 51b2401531c7a..79cb384c5bded 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -47,7 +47,7 @@ pub(super) fn find_opaque_ty_constraints_for_impl_trait_in_assoc_type( let assoc = tcx.associated_item(assoc_id); match assoc.kind { ty::AssocKind::Const | ty::AssocKind::Fn => { - locator.check(assoc_id.expect_local(), true) + locator.check(assoc_id.expect_local(), ImplTraitSource::AssocTy) } // Associated types don't have bodies, so they can't constrain hidden types ty::AssocKind::Type => {} @@ -182,9 +182,15 @@ struct TaitConstraintLocator<'tcx> { typeck_types: Vec>, } +#[derive(Debug)] +enum ImplTraitSource { + AssocTy, + TyAlias, +} + impl TaitConstraintLocator<'_> { #[instrument(skip(self), level = "debug")] - fn check(&mut self, item_def_id: LocalDefId, impl_trait_in_assoc_type: bool) { + fn check(&mut self, item_def_id: LocalDefId, source: ImplTraitSource) { // Don't try to check items that cannot possibly constrain the type. if !self.tcx.has_typeck_results(item_def_id) { debug!("no constraint: no typeck results"); @@ -236,10 +242,11 @@ impl TaitConstraintLocator<'_> { continue; } constrained = true; - let opaque_types_defined_by = if impl_trait_in_assoc_type { - self.tcx.impl_trait_in_assoc_types_defined_by(item_def_id) - } else { - self.tcx.opaque_types_defined_by(item_def_id) + let opaque_types_defined_by = match source { + ImplTraitSource::AssocTy => { + self.tcx.impl_trait_in_assoc_types_defined_by(item_def_id) + } + ImplTraitSource::TyAlias => self.tcx.opaque_types_defined_by(item_def_id), }; if !opaque_types_defined_by.contains(&self.def_id) { self.tcx.dcx().emit_err(TaitForwardCompat { @@ -299,7 +306,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { } fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { if let hir::ExprKind::Closure(closure) = ex.kind { - self.check(closure.def_id, false); + self.check(closure.def_id, ImplTraitSource::TyAlias); } intravisit::walk_expr(self, ex); } @@ -307,7 +314,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { trace!(?it.owner_id); // The opaque type itself or its children are not within its reveal scope. if it.owner_id.def_id != self.def_id { - self.check(it.owner_id.def_id, false); + self.check(it.owner_id.def_id, ImplTraitSource::TyAlias); intravisit::walk_item(self, it); } } @@ -315,13 +322,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { trace!(?it.owner_id); // The opaque type itself or its children are not within its reveal scope. if it.owner_id.def_id != self.def_id { - self.check(it.owner_id.def_id, false); + self.check(it.owner_id.def_id, ImplTraitSource::TyAlias); intravisit::walk_impl_item(self, it); } } fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) { trace!(?it.owner_id); - self.check(it.owner_id.def_id, false); + self.check(it.owner_id.def_id, ImplTraitSource::TyAlias); intravisit::walk_trait_item(self, it); } fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { From 5e5d1350b600ad8922a7c4bee44611bea597d082 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 16 Jan 2024 10:17:58 +0000 Subject: [PATCH 12/30] Check that we forbid nested items, but not nested closures --- .../itiat-allow-nested-closures.bad.stderr | 23 ++++++++++++++++ .../itiat-allow-nested-closures.rs | 26 +++++++++++++++++++ .../itiat-forbid-nested-items.rs | 20 ++++++++++++++ .../itiat-forbid-nested-items.stderr | 22 ++++++++++++++++ 4 files changed, 91 insertions(+) create mode 100644 tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.bad.stderr create mode 100644 tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.rs create mode 100644 tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.rs create mode 100644 tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.stderr diff --git a/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.bad.stderr b/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.bad.stderr new file mode 100644 index 0000000000000..4acc47eaef220 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.bad.stderr @@ -0,0 +1,23 @@ +error[E0308]: mismatched types + --> $DIR/itiat-allow-nested-closures.rs:21:22 + | +LL | type Assoc = impl Sized; + | ---------- the found opaque type +... +LL | let _: i32 = closure(); + | --- ^^^^^^^^^ expected `i32`, found opaque type + | | + | expected due to this + +error[E0308]: mismatched types + --> $DIR/itiat-allow-nested-closures.rs:22:9 + | +LL | fn bar() -> Self::Assoc { + | ----------- expected `()` because of return type +... +LL | 1i32 + | ^^^^ expected `()`, found `i32` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.rs b/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.rs new file mode 100644 index 0000000000000..55994d6a3259d --- /dev/null +++ b/tests/ui/type-alias-impl-trait/itiat-allow-nested-closures.rs @@ -0,0 +1,26 @@ +#![feature(impl_trait_in_assoc_type)] + +// revisions: ok bad +// [ok] check-pass + +trait Foo { + type Assoc; + fn bar() -> Self::Assoc; +} + +impl Foo for () { + type Assoc = impl Sized; + fn bar() -> Self::Assoc { + let closure = || -> Self::Assoc { + #[cfg(ok)] + let x: Self::Assoc = 42_i32; + #[cfg(bad)] + let x: Self::Assoc = (); + x + }; + let _: i32 = closure(); //[bad]~ ERROR mismatched types + 1i32 //[bad]~ ERROR mismatched types + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.rs b/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.rs new file mode 100644 index 0000000000000..8c9d780c1119d --- /dev/null +++ b/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.rs @@ -0,0 +1,20 @@ +#![feature(impl_trait_in_assoc_type)] + +trait Foo { + type Assoc; + fn bar() -> Self::Assoc; +} + +impl Foo for () { + type Assoc = impl Sized; + fn bar() -> Self::Assoc { + fn foo() -> <() as Foo>::Assoc { + let x: <() as Foo>::Assoc = 42_i32; //~ ERROR mismatched types + x + }; + let _: i32 = foo(); + 1i32 + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.stderr b/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.stderr new file mode 100644 index 0000000000000..c177201431a61 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/itiat-forbid-nested-items.stderr @@ -0,0 +1,22 @@ +error[E0308]: mismatched types + --> $DIR/itiat-forbid-nested-items.rs:12:41 + | +LL | type Assoc = impl Sized; + | ---------- the expected opaque type +... +LL | let x: <() as Foo>::Assoc = 42_i32; + | ------------------ ^^^^^^ expected opaque type, found `i32` + | | + | expected due to this + | + = note: expected opaque type `<() as Foo>::Assoc` + found type `i32` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/itiat-forbid-nested-items.rs:11:12 + | +LL | fn foo() -> <() as Foo>::Assoc { + | ^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. From 21e5beae3cc4ffd2adea9ae7b4a9d8b84a4bc0a8 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 22 Jan 2024 10:10:00 -0600 Subject: [PATCH 13/30] Use debug_assert instead of expanded equivalent --- compiler/rustc_mir_transform/src/coverage/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 4b91f042f007a..afeacbf2232d3 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -366,10 +366,8 @@ fn check_code_region(code_region: CodeRegion) -> Option { ?is_ordered, "Skipping code region that would be misinterpreted or rejected by LLVM" ); - if cfg!(debug_assertions) { - // If this happens in a debug build, ICE to make it easier to notice. - bug!("Improper code region: {code_region:?}"); - } + // If this happens in a debug build, ICE to make it easier to notice. + debug_assert!(false, "Improper code region: {code_region:?}"); None } } From f700ee4e709abb2243acd0fb8b9928160896c2bd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 19 Jan 2024 20:04:14 +0000 Subject: [PATCH 14/30] Do not normalize closure signature when building FnOnce shim --- compiler/rustc_codegen_cranelift/src/base.rs | 1 - compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 1 - .../rustc_const_eval/src/interpret/cast.rs | 3 +-- compiler/rustc_middle/src/ty/instance.rs | 18 ++++++++---------- compiler/rustc_monomorphize/src/collector.rs | 3 +-- compiler/rustc_smir/src/rustc_smir/context.rs | 5 ++++- compiler/rustc_ty_utils/src/instance.rs | 7 ++++++- 7 files changed, 20 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 881c0c0b56b6f..0afd6d0e670b3 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -682,7 +682,6 @@ fn codegen_stmt<'tcx>( args, ty::ClosureKind::FnOnce, ) - .expect("failed to normalize and resolve closure during codegen") .polymorphize(fx.tcx); let func_ref = fx.get_function_ref(instance); let func_addr = fx.bcx.ins().func_addr(fx.pointer_type, func_ref); diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 02b51dfe5bf7f..266505d3f2691 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -435,7 +435,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { args, ty::ClosureKind::FnOnce, ) - .expect("failed to normalize and resolve closure during codegen") .polymorphize(bx.cx().tcx()); OperandValue::Immediate(bx.cx().get_fn_addr(instance)) } diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index d296ff5928b36..0cb5c634b22ba 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -117,8 +117,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { def_id, args, ty::ClosureKind::FnOnce, - ) - .ok_or_else(|| err_inval!(TooGeneric))?; + ); let fn_ptr = self.fn_ptr(FnVal::Instance(instance)); self.write_pointer(fn_ptr, dest)?; } diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index dd41cb5a61f44..89c5b8a6b684e 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -528,12 +528,12 @@ impl<'tcx> Instance<'tcx> { def_id: DefId, args: ty::GenericArgsRef<'tcx>, requested_kind: ty::ClosureKind, - ) -> Option> { + ) -> Instance<'tcx> { let actual_kind = args.as_closure().kind(); match needs_fn_once_adapter_shim(actual_kind, requested_kind) { Ok(true) => Instance::fn_once_adapter_instance(tcx, def_id, args), - _ => Some(Instance::new(def_id, args)), + _ => Instance::new(def_id, args), } } @@ -548,7 +548,7 @@ impl<'tcx> Instance<'tcx> { tcx: TyCtxt<'tcx>, closure_did: DefId, args: ty::GenericArgsRef<'tcx>, - ) -> Option> { + ) -> Instance<'tcx> { let fn_once = tcx.require_lang_item(LangItem::FnOnce, None); let call_once = tcx .associated_items(fn_once) @@ -562,14 +562,12 @@ impl<'tcx> Instance<'tcx> { let self_ty = Ty::new_closure(tcx, closure_did, args); - let sig = args.as_closure().sig(); - let sig = - tcx.try_normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), sig).ok()?; - assert_eq!(sig.inputs().len(), 1); - let args = tcx.mk_args_trait(self_ty, [sig.inputs()[0].into()]); + let tupled_inputs_ty = args.as_closure().sig().map_bound(|sig| sig.inputs()[0]); + let tupled_inputs_ty = tcx.instantiate_bound_regions_with_erased(tupled_inputs_ty); + let args = tcx.mk_args_trait(self_ty, [tupled_inputs_ty.into()]); - debug!(?self_ty, ?sig); - Some(Instance { def, args }) + debug!(?self_ty, args=?tupled_inputs_ty.tuple_fields()); + Instance { def, args } } /// Depending on the kind of `InstanceDef`, the MIR body associated with an diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 20e70f87b75e8..1dbb8c4f42481 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -783,8 +783,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { def_id, args, ty::ClosureKind::FnOnce, - ) - .expect("failed to normalize and resolve closure during codegen"); + ); if should_codegen_locally(self.tcx, &instance) { self.output.push(create_fn_mono_item(self.tcx, instance, span)); } diff --git a/compiler/rustc_smir/src/rustc_smir/context.rs b/compiler/rustc_smir/src/rustc_smir/context.rs index 10085f659b370..94a1fb33f99c5 100644 --- a/compiler/rustc_smir/src/rustc_smir/context.rs +++ b/compiler/rustc_smir/src/rustc_smir/context.rs @@ -464,7 +464,10 @@ impl<'tcx> Context for TablesWrapper<'tcx> { let def_id = def.0.internal(&mut *tables, tcx); let args_ref = args.internal(&mut *tables, tcx); let closure_kind = kind.internal(&mut *tables, tcx); - Instance::resolve_closure(tables.tcx, def_id, args_ref, closure_kind).stable(&mut *tables) + Some( + Instance::resolve_closure(tables.tcx, def_id, args_ref, closure_kind) + .stable(&mut *tables), + ) } fn eval_instance(&self, def: InstanceDef, const_ty: Ty) -> Result { diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 81d5304b81265..18eb5314e8980 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -322,7 +322,12 @@ fn resolve_associated_item<'tcx>( match *rcvr_args.type_at(0).kind() { ty::Closure(closure_def_id, args) => { let trait_closure_kind = tcx.fn_trait_kind_from_def_id(trait_id).unwrap(); - Instance::resolve_closure(tcx, closure_def_id, args, trait_closure_kind) + Some(Instance::resolve_closure( + tcx, + closure_def_id, + args, + trait_closure_kind, + )) } ty::FnDef(..) | ty::FnPtr(..) => Some(Instance { def: ty::InstanceDef::FnPtrShim(trait_item_id, rcvr_args.type_at(0)), From 390ef9ba0297ae5ba5aacdf0be0d0c47be8d166a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Jan 2024 20:51:19 +0000 Subject: [PATCH 15/30] Fix incorrect suggestion for boxing tail expression in blocks --- compiler/rustc_hir_typeck/src/demand.rs | 8 ++++++- ...uggest-box-on-divergent-if-else-arms.fixed | 14 +++++++++++ .../suggest-box-on-divergent-if-else-arms.rs | 14 +++++++++++ ...ggest-box-on-divergent-if-else-arms.stderr | 24 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 9850892bd36af..b6dfc34d3ac19 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -44,7 +44,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { || self.suggest_non_zero_new_unwrap(err, expr, expected, expr_ty) || self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) || self.suggest_no_capture_closure(err, expected, expr_ty) - || self.suggest_boxing_when_appropriate(err, expr.span, expr.hir_id, expected, expr_ty) + || self.suggest_boxing_when_appropriate( + err, + expr.peel_blocks().span, + expr.hir_id, + expected, + expr_ty, + ) || self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected) || self.suggest_copied_cloned_or_as_ref(err, expr, expr_ty, expected) || self.suggest_clone_for_ref(err, expr, expr_ty, expected) diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed new file mode 100644 index 0000000000000..d8031083f6cf8 --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} +struct Struct; +impl Trait for Struct {} +fn foo() -> Box { + Box::new(Struct) +} +fn main() { + let _ = if true { + foo() + } else { + Box::new(Struct) //~ ERROR E0308 + }; +} diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs new file mode 100644 index 0000000000000..53e718a56635b --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} +struct Struct; +impl Trait for Struct {} +fn foo() -> Box { + Box::new(Struct) +} +fn main() { + let _ = if true { + foo() + } else { + Struct //~ ERROR E0308 + }; +} diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr new file mode 100644 index 0000000000000..183f48ca1fb78 --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -0,0 +1,24 @@ +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:12:9 + | +LL | let _ = if true { + | _____________- +LL | | foo() + | | ----- expected because of this +LL | | } else { +LL | | Struct + | | ^^^^^^ expected `Box`, found `Struct` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Box` + found struct `Struct` + = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html +help: store this in the heap by calling `Box::new` + | +LL | Box::new(Struct) + | +++++++++ + + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. From ac56a2b564a3e15b8377e72294a3d565a1c8c659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Jan 2024 20:53:41 +0000 Subject: [PATCH 16/30] Suggest boxing if then expr if that solves divergent arms When encountering ```rust let _ = if true { Struct } else { foo() // -> Box }; ``` if `Struct` implements `Trait`, suggest boxing the then arm tail expression. Part of #102629. --- .../infer/error_reporting/note_and_explain.rs | 32 +++++++++++++++++++ ...uggest-box-on-divergent-if-else-arms.fixed | 5 +++ .../suggest-box-on-divergent-if-else-arms.rs | 5 +++ ...ggest-box-on-divergent-if-else-arms.stderr | 22 ++++++++++++- 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index afb3c5c1e5656..a0dfaf33a6e85 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -330,6 +330,38 @@ impl Trait for X { ); } } + (ty::Adt(_, _), ty::Adt(def, args)) + if let ObligationCauseCode::IfExpression(cause) = cause.code() + && let hir::Node::Block(blk) = self.tcx.hir_node(cause.then_id) + && let Some(then) = blk.expr + && def.is_box() + && let boxed_ty = args.type_at(0) + && let ty::Dynamic(t, _, _) = boxed_ty.kind() + && let Some(def_id) = t.principal_def_id() + && let mut impl_def_ids = vec![] + && let _ = + tcx.for_each_relevant_impl(def_id, values.expected, |did| { + impl_def_ids.push(did) + }) + && let [_] = &impl_def_ids[..] => + { + // We have divergent if/else arms where the expected value is a type that + // implements the trait of the found boxed trait object. + diag.multipart_suggestion( + format!( + "`{}` implements `{}` so you can box it to coerce to the trait \ + object `{}`", + values.expected, + tcx.item_name(def_id), + values.found, + ), + vec![ + (then.span.shrink_to_lo(), "Box::new(".to_string()), + (then.span.shrink_to_hi(), ")".to_string()), + ], + MachineApplicable, + ); + } _ => {} } debug!( diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed index d8031083f6cf8..dfdf6b24bf7e0 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -6,6 +6,11 @@ fn foo() -> Box { Box::new(Struct) } fn main() { + let _ = if true { + Box::new(Struct) + } else { + foo() //~ ERROR E0308 + }; let _ = if true { foo() } else { diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs index 53e718a56635b..e138ad9b33647 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -6,6 +6,11 @@ fn foo() -> Box { Box::new(Struct) } fn main() { + let _ = if true { + Struct + } else { + foo() //~ ERROR E0308 + }; let _ = if true { foo() } else { diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr index 183f48ca1fb78..98c9d880f97aa 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -3,6 +3,26 @@ error[E0308]: `if` and `else` have incompatible types | LL | let _ = if true { | _____________- +LL | | Struct + | | ------ expected because of this +LL | | } else { +LL | | foo() + | | ^^^^^ expected `Struct`, found `Box` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Struct` + found struct `Box` +help: `Struct` implements `Trait` so you can box it to coerce to the trait object `Box` + | +LL | Box::new(Struct) + | +++++++++ + + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:17:9 + | +LL | let _ = if true { + | _____________- LL | | foo() | | ----- expected because of this LL | | } else { @@ -19,6 +39,6 @@ help: store this in the heap by calling `Box::new` LL | Box::new(Struct) | +++++++++ + -error: aborting due to 1 previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0308`. From 9525751ba3d752ba1602b6c87aec44583526fa71 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 17 Jan 2024 23:14:14 +0300 Subject: [PATCH 17/30] linker: Refactor APIs for linking dynamic libraries Rename `link_(dylib,framework)`, remove `link_rust_dylib`. --- compiler/rustc_codegen_ssa/src/back/link.rs | 17 ++- compiler/rustc_codegen_ssa/src/back/linker.rs | 109 ++++-------------- 2 files changed, 33 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 959653c932653..411f727a9f87b 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1265,7 +1265,7 @@ fn link_sanitizer_runtime( let path = find_sanitizer_runtime(sess, &filename); let rpath = path.to_str().expect("non-utf8 component in path"); linker.args(&["-Wl,-rpath", "-Xlinker", rpath]); - linker.link_dylib(&filename, false, true); + linker.link_dylib_by_name(&filename, false, true); } else if sess.target.is_like_msvc && flavor == LinkerFlavor::Msvc(Lld::No) && name == "asan" { // MSVC provides the `/INFERASANLIBS` argument to automatically find the // compatible ASAN library. @@ -2526,7 +2526,7 @@ fn add_native_libs_from_crate( } NativeLibKind::Dylib { as_needed } => { if link_dynamic { - cmd.link_dylib(name, verbatim, as_needed.unwrap_or(true)) + cmd.link_dylib_by_name(name, verbatim, as_needed.unwrap_or(true)) } } NativeLibKind::Unspecified => { @@ -2538,13 +2538,13 @@ fn add_native_libs_from_crate( } } else { if link_dynamic { - cmd.link_dylib(name, verbatim, true); + cmd.link_dylib_by_name(name, verbatim, true); } } } NativeLibKind::Framework { as_needed } => { if link_dynamic { - cmd.link_framework(name, as_needed.unwrap_or(true)) + cmd.link_framework_by_name(name, verbatim, as_needed.unwrap_or(true)) } } NativeLibKind::RawDylib => { @@ -2859,13 +2859,20 @@ fn add_dynamic_crate(cmd: &mut dyn Linker, sess: &Session, cratepath: &Path) { // Just need to tell the linker about where the library lives and // what its name is let parent = cratepath.parent(); + // When producing a dll, the MSVC linker may not actually emit a + // `foo.lib` file if the dll doesn't actually export any symbols, so we + // check to see if the file is there and just omit linking to it if it's + // not present. + if sess.target.is_like_msvc && !cratepath.with_extension("dll.lib").exists() { + return; + } if let Some(dir) = parent { cmd.include_path(&rehome_sysroot_lib_dir(sess, dir)); } let stem = cratepath.file_stem().unwrap().to_str().unwrap(); // Convert library file-stem into a cc -l argument. let prefix = if stem.starts_with("lib") && !sess.target.is_like_windows { 3 } else { 0 }; - cmd.link_rust_dylib(&stem[prefix..], parent.unwrap_or_else(|| Path::new(""))); + cmd.link_dylib_by_name(&stem[prefix..], false, true); } fn relevant_lib(sess: &Session, lib: &NativeLib) -> bool { diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 90f5027c26494..9a854bb7101dd 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -166,9 +166,10 @@ pub fn get_linker<'a>( pub trait Linker { fn cmd(&mut self) -> &mut Command; fn set_output_kind(&mut self, output_kind: LinkOutputKind, out_filename: &Path); - fn link_dylib(&mut self, lib: &str, verbatim: bool, as_needed: bool); - fn link_rust_dylib(&mut self, lib: &str, path: &Path); - fn link_framework(&mut self, framework: &str, as_needed: bool); + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, as_needed: bool); + fn link_framework_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { + bug!("framework linked with unsupported linker") + } fn link_staticlib(&mut self, lib: &str, verbatim: bool); fn link_rlib(&mut self, lib: &Path); fn link_whole_rlib(&mut self, lib: &Path); @@ -432,8 +433,8 @@ impl<'a> Linker for GccLinker<'a> { } } - fn link_dylib(&mut self, lib: &str, verbatim: bool, as_needed: bool) { - if self.sess.target.os == "illumos" && lib == "c" { + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, as_needed: bool) { + if self.sess.target.os == "illumos" && name == "c" { // libc will be added via late_link_args on illumos so that it will // appear last in the library search order. // FIXME: This should be replaced by a more complete and generic @@ -454,7 +455,7 @@ impl<'a> Linker for GccLinker<'a> { } } self.hint_dynamic(); - self.cmd.arg(format!("-l{}{lib}", if verbatim && self.is_gnu { ":" } else { "" },)); + self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },)); if !as_needed { if self.sess.target.is_like_osx { // See above FIXME comment @@ -493,20 +494,15 @@ impl<'a> Linker for GccLinker<'a> { self.linker_args(&["-z", "norelro"]); } - fn link_rust_dylib(&mut self, lib: &str, _path: &Path) { - self.hint_dynamic(); - self.cmd.arg(format!("-l{lib}")); - } - - fn link_framework(&mut self, framework: &str, as_needed: bool) { + fn link_framework_by_name(&mut self, name: &str, _verbatim: bool, as_needed: bool) { self.hint_dynamic(); if !as_needed { // FIXME(81490): ld64 as of macOS 11 supports the -needed_framework // flag but we have no way to detect that here. - // self.cmd.arg("-needed_framework").arg(framework); + // self.cmd.arg("-needed_framework").arg(name); self.sess.dcx().emit_warn(errors::Ld64UnimplementedModifier); } - self.cmd.arg("-framework").arg(framework); + self.cmd.arg("-framework").arg(name); } // Here we explicitly ask that the entire archive is included into the @@ -845,19 +841,8 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg("/OPT:NOREF,NOICF"); } - fn link_dylib(&mut self, lib: &str, verbatim: bool, _as_needed: bool) { - self.cmd.arg(format!("{}{}", lib, if verbatim { "" } else { ".lib" })); - } - - fn link_rust_dylib(&mut self, lib: &str, path: &Path) { - // When producing a dll, the MSVC linker may not actually emit a - // `foo.lib` file if the dll doesn't actually export any symbols, so we - // check to see if the file is there and just omit linking to it if it's - // not present. - let name = format!("{lib}.dll.lib"); - if path.join(&name).exists() { - self.cmd.arg(name); - } + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { + self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); } fn link_staticlib(&mut self, lib: &str, verbatim: bool) { @@ -899,9 +884,6 @@ impl<'a> Linker for MsvcLinker<'a> { fn framework_path(&mut self, _path: &Path) { bug!("frameworks are not supported on windows") } - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - bug!("frameworks are not supported on windows") - } fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, _search_path: &[PathBuf]) { self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", lib, if verbatim { "" } else { ".lib" })); @@ -1073,9 +1055,9 @@ impl<'a> Linker for EmLinker<'a> { self.cmd.arg(path); } - fn link_dylib(&mut self, lib: &str, verbatim: bool, _as_needed: bool) { + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { // Emscripten always links statically - self.link_staticlib(lib, verbatim); + self.link_staticlib(name, verbatim); } fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, _search_path: &[PathBuf]) { @@ -1088,10 +1070,6 @@ impl<'a> Linker for EmLinker<'a> { self.link_rlib(lib); } - fn link_rust_dylib(&mut self, lib: &str, _path: &Path) { - self.link_dylib(lib, false, true); - } - fn link_rlib(&mut self, lib: &Path) { self.add_object(lib); } @@ -1112,10 +1090,6 @@ impl<'a> Linker for EmLinker<'a> { bug!("frameworks are not supported on Emscripten") } - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - bug!("frameworks are not supported on Emscripten") - } - fn gc_sections(&mut self, _keep_metadata: bool) { // noop } @@ -1249,8 +1223,8 @@ impl<'a> Linker for WasmLd<'a> { } } - fn link_dylib(&mut self, lib: &str, _verbatim: bool, _as_needed: bool) { - self.cmd.arg("-l").arg(lib); + fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) { + self.cmd.arg("-l").arg(name); } fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { @@ -1283,14 +1257,6 @@ impl<'a> Linker for WasmLd<'a> { fn no_relro(&mut self) {} - fn link_rust_dylib(&mut self, lib: &str, _path: &Path) { - self.cmd.arg("-l").arg(lib); - } - - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - panic!("frameworks not supported") - } - fn link_whole_staticlib(&mut self, lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { self.cmd.arg("--whole-archive").arg("-l").arg(lib).arg("--no-whole-archive"); } @@ -1398,7 +1364,7 @@ pub struct L4Bender<'a> { } impl<'a> Linker for L4Bender<'a> { - fn link_dylib(&mut self, _lib: &str, _verbatim: bool, _as_needed: bool) { + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("dylibs are not supported on L4Re"); } fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { @@ -1442,14 +1408,6 @@ impl<'a> Linker for L4Bender<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn link_rust_dylib(&mut self, _: &str, _: &Path) { - panic!("Rust dylibs not supported"); - } - - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - bug!("frameworks not supported on L4Re"); - } - fn link_whole_staticlib(&mut self, lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { self.hint_static(); self.cmd.arg("--whole-archive").arg(format!("-l{lib}")); @@ -1571,9 +1529,9 @@ impl<'a> AixLinker<'a> { } impl<'a> Linker for AixLinker<'a> { - fn link_dylib(&mut self, lib: &str, _verbatim: bool, _as_needed: bool) { + fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) { self.hint_dynamic(); - self.cmd.arg(format!("-l{lib}")); + self.cmd.arg(format!("-l{name}")); } fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { @@ -1626,15 +1584,6 @@ impl<'a> Linker for AixLinker<'a> { } } - fn link_rust_dylib(&mut self, lib: &str, _: &Path) { - self.hint_dynamic(); - self.cmd.arg(format!("-l{lib}")); - } - - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - bug!("frameworks not supported on AIX"); - } - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, search_path: &[PathBuf]) { self.hint_static(); let lib = find_native_static_library(lib, verbatim, search_path, self.sess); @@ -1844,11 +1793,7 @@ impl<'a> Linker for PtxLinker<'a> { self.cmd.arg("-o").arg(path); } - fn link_dylib(&mut self, _lib: &str, _verbatim: bool, _as_needed: bool) { - panic!("external dylibs not supported") - } - - fn link_rust_dylib(&mut self, _lib: &str, _path: &Path) { + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { panic!("external dylibs not supported") } @@ -1864,10 +1809,6 @@ impl<'a> Linker for PtxLinker<'a> { panic!("frameworks not supported") } - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - panic!("frameworks not supported") - } - fn full_relro(&mut self) {} fn partial_relro(&mut self) {} @@ -1942,11 +1883,7 @@ impl<'a> Linker for BpfLinker<'a> { self.cmd.arg("-o").arg(path); } - fn link_dylib(&mut self, _lib: &str, _verbatim: bool, _as_needed: bool) { - panic!("external dylibs not supported") - } - - fn link_rust_dylib(&mut self, _lib: &str, _path: &Path) { + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { panic!("external dylibs not supported") } @@ -1962,10 +1899,6 @@ impl<'a> Linker for BpfLinker<'a> { panic!("frameworks not supported") } - fn link_framework(&mut self, _framework: &str, _as_needed: bool) { - panic!("frameworks not supported") - } - fn full_relro(&mut self) {} fn partial_relro(&mut self) {} From 2305e28051dc4947657d2d2d00b2d7993118bcee Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2024 00:28:56 +0300 Subject: [PATCH 18/30] linker: Refactor APIs for linking static libraries Rename `link(_whole)(staticlib,rlib)` to something more suitable. --- compiler/rustc_codegen_ssa/src/back/link.rs | 14 +- compiler/rustc_codegen_ssa/src/back/linker.rs | 169 +++++++++++------- 2 files changed, 114 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 411f727a9f87b..18fa8a41c4622 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1273,7 +1273,7 @@ fn link_sanitizer_runtime( } else { let filename = format!("librustc{channel}_rt.{name}.a"); let path = find_sanitizer_runtime(sess, &filename).join(&filename); - linker.link_whole_rlib(&path); + linker.link_whole_staticlib_by_path(&path); } } @@ -2506,20 +2506,20 @@ fn add_native_libs_from_crate( // If rlib contains native libs as archives, they are unpacked to tmpdir. let path = tmpdir.join(filename.as_str()); if whole_archive { - cmd.link_whole_rlib(&path); + cmd.link_whole_staticlib_by_path(&path); } else { - cmd.link_rlib(&path); + cmd.link_staticlib_by_path(&path); } } } else { if whole_archive { - cmd.link_whole_staticlib( + cmd.link_whole_staticlib_by_name( name, verbatim, search_paths.get_or_init(|| archive_search_paths(sess)), ); } else { - cmd.link_staticlib(name, verbatim) + cmd.link_staticlib_by_name(name, verbatim) } } } @@ -2534,7 +2534,7 @@ fn add_native_libs_from_crate( // link kind is unspecified. if !link_output_kind.can_link_dylib() && !sess.target.crt_static_allows_dylibs { if link_static { - cmd.link_staticlib(name, verbatim) + cmd.link_staticlib_by_name(name, verbatim) } } else { if link_dynamic { @@ -2791,7 +2791,7 @@ fn add_static_crate<'a>( } else { fix_windows_verbatim_for_gcc(path) }; - cmd.link_rlib(&rlib_path); + cmd.link_staticlib_by_path(&rlib_path); }; if !are_upstream_rust_objects_already_included(sess) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 9a854bb7101dd..4360c10edc30a 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -170,10 +170,15 @@ pub trait Linker { fn link_framework_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("framework linked with unsupported linker") } - fn link_staticlib(&mut self, lib: &str, verbatim: bool); - fn link_rlib(&mut self, lib: &Path); - fn link_whole_rlib(&mut self, lib: &Path); - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, search_path: &[PathBuf]); + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool); + fn link_staticlib_by_path(&mut self, path: &Path); + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + search_paths: &[PathBuf], + ); + fn link_whole_staticlib_by_path(&mut self, path: &Path); fn include_path(&mut self, path: &Path); fn framework_path(&mut self, path: &Path); fn output_filename(&mut self, path: &Path); @@ -464,13 +469,13 @@ impl<'a> Linker for GccLinker<'a> { } } } - fn link_staticlib(&mut self, lib: &str, verbatim: bool) { + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { self.hint_static(); - self.cmd.arg(format!("-l{}{lib}", if verbatim && self.is_gnu { ":" } else { "" },)); + self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },)); } - fn link_rlib(&mut self, lib: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); - self.cmd.arg(lib); + self.cmd.arg(path); } fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); @@ -511,29 +516,34 @@ impl<'a> Linker for GccLinker<'a> { // don't otherwise explicitly reference them. This can occur for // libraries which are just providing bindings, libraries with generic // functions, etc. - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + search_paths: &[PathBuf], + ) { self.hint_static(); let target = &self.sess.target; if !target.is_like_osx { self.linker_arg("--whole-archive"); - self.cmd.arg(format!("-l{}{lib}", if verbatim && self.is_gnu { ":" } else { "" },)); + self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" })); self.linker_arg("--no-whole-archive"); } else { // -force_load is the macOS equivalent of --whole-archive, but it // involves passing the full path to the library to link. self.linker_arg("-force_load"); - let lib = find_native_static_library(lib, verbatim, search_path, self.sess); + let lib = find_native_static_library(name, verbatim, search_paths, self.sess); self.linker_arg(&lib); } } - fn link_whole_rlib(&mut self, lib: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); if self.sess.target.is_like_osx { self.linker_arg("-force_load"); - self.linker_arg(&lib); + self.linker_arg(&path); } else { - self.linker_args(&[OsString::from("--whole-archive"), lib.into()]); + self.linker_args(&[OsString::from("--whole-archive"), path.into()]); self.linker_arg("--no-whole-archive"); } } @@ -817,8 +827,8 @@ impl<'a> Linker for MsvcLinker<'a> { } } - fn link_rlib(&mut self, lib: &Path) { - self.cmd.arg(lib); + fn link_staticlib_by_path(&mut self, path: &Path) { + self.cmd.arg(path); } fn add_object(&mut self, path: &Path) { self.cmd.arg(path); @@ -845,8 +855,8 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); } - fn link_staticlib(&mut self, lib: &str, verbatim: bool) { - self.cmd.arg(format!("{}{}", lib, if verbatim { "" } else { ".lib" })); + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { + self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); } fn full_relro(&mut self) { @@ -885,10 +895,15 @@ impl<'a> Linker for MsvcLinker<'a> { bug!("frameworks are not supported on windows") } - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, _search_path: &[PathBuf]) { - self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", lib, if verbatim { "" } else { ".lib" })); + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + _search_paths: &[PathBuf], + ) { + self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); } - fn link_whole_rlib(&mut self, path: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { let mut arg = OsString::from("/WHOLEARCHIVE:"); arg.push(path); self.cmd.arg(arg); @@ -1043,8 +1058,8 @@ impl<'a> Linker for EmLinker<'a> { self.cmd.arg("-L").arg(path); } - fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { - self.cmd.arg("-l").arg(lib); + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { + self.cmd.arg("-l").arg(name); } fn output_filename(&mut self, path: &Path) { @@ -1057,21 +1072,26 @@ impl<'a> Linker for EmLinker<'a> { fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { // Emscripten always links statically - self.link_staticlib(name, verbatim); + self.link_staticlib_by_name(name, verbatim); } - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, _search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + _search_paths: &[PathBuf], + ) { // not supported? - self.link_staticlib(lib, verbatim); + self.link_staticlib_by_name(name, verbatim); } - fn link_whole_rlib(&mut self, lib: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { // not supported? - self.link_rlib(lib); + self.link_staticlib_by_path(path); } - fn link_rlib(&mut self, lib: &Path) { - self.add_object(lib); + fn link_staticlib_by_path(&mut self, path: &Path) { + self.add_object(path); } fn full_relro(&mut self) { @@ -1227,12 +1247,12 @@ impl<'a> Linker for WasmLd<'a> { self.cmd.arg("-l").arg(name); } - fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { - self.cmd.arg("-l").arg(lib); + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { + self.cmd.arg("-l").arg(name); } - fn link_rlib(&mut self, lib: &Path) { - self.cmd.arg(lib); + fn link_staticlib_by_path(&mut self, path: &Path) { + self.cmd.arg(path); } fn include_path(&mut self, path: &Path) { @@ -1257,12 +1277,17 @@ impl<'a> Linker for WasmLd<'a> { fn no_relro(&mut self) {} - fn link_whole_staticlib(&mut self, lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { - self.cmd.arg("--whole-archive").arg("-l").arg(lib).arg("--no-whole-archive"); + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { + self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); } - fn link_whole_rlib(&mut self, lib: &Path) { - self.cmd.arg("--whole-archive").arg(lib).arg("--no-whole-archive"); + fn link_whole_staticlib_by_path(&mut self, path: &Path) { + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); } fn gc_sections(&mut self, _keep_metadata: bool) { @@ -1367,13 +1392,13 @@ impl<'a> Linker for L4Bender<'a> { fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("dylibs are not supported on L4Re"); } - fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { self.hint_static(); - self.cmd.arg(format!("-PC{lib}")); + self.cmd.arg(format!("-PC{name}")); } - fn link_rlib(&mut self, lib: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); - self.cmd.arg(lib); + self.cmd.arg(path); } fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); @@ -1408,15 +1433,20 @@ impl<'a> Linker for L4Bender<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn link_whole_staticlib(&mut self, lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { self.hint_static(); - self.cmd.arg("--whole-archive").arg(format!("-l{lib}")); + self.cmd.arg("--whole-archive").arg(format!("-l{name}")); self.cmd.arg("--no-whole-archive"); } - fn link_whole_rlib(&mut self, lib: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); - self.cmd.arg("--whole-archive").arg(lib).arg("--no-whole-archive"); + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); } fn gc_sections(&mut self, keep_metadata: bool) { @@ -1534,14 +1564,14 @@ impl<'a> Linker for AixLinker<'a> { self.cmd.arg(format!("-l{name}")); } - fn link_staticlib(&mut self, lib: &str, _verbatim: bool) { + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { self.hint_static(); - self.cmd.arg(format!("-l{lib}")); + self.cmd.arg(format!("-l{name}")); } - fn link_rlib(&mut self, lib: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); - self.cmd.arg(lib); + self.cmd.arg(path); } fn include_path(&mut self, path: &Path) { @@ -1584,15 +1614,20 @@ impl<'a> Linker for AixLinker<'a> { } } - fn link_whole_staticlib(&mut self, lib: &str, verbatim: bool, search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + search_paths: &[PathBuf], + ) { self.hint_static(); - let lib = find_native_static_library(lib, verbatim, search_path, self.sess); + let lib = find_native_static_library(name, verbatim, search_paths, self.sess); self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); } - fn link_whole_rlib(&mut self, lib: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); - self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); + self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); } fn gc_sections(&mut self, _keep_metadata: bool) { @@ -1759,11 +1794,11 @@ impl<'a> Linker for PtxLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn link_rlib(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg("--rlib").arg(path); } - fn link_whole_rlib(&mut self, path: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg("--rlib").arg(path); } @@ -1797,11 +1832,16 @@ impl<'a> Linker for PtxLinker<'a> { panic!("external dylibs not supported") } - fn link_staticlib(&mut self, _lib: &str, _verbatim: bool) { + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { panic!("staticlibs not supported") } - fn link_whole_staticlib(&mut self, _lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + _name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { panic!("staticlibs not supported") } @@ -1848,11 +1888,11 @@ impl<'a> Linker for BpfLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn link_rlib(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg(path); } - fn link_whole_rlib(&mut self, path: &Path) { + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg(path); } @@ -1887,11 +1927,16 @@ impl<'a> Linker for BpfLinker<'a> { panic!("external dylibs not supported") } - fn link_staticlib(&mut self, _lib: &str, _verbatim: bool) { + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { panic!("staticlibs not supported") } - fn link_whole_staticlib(&mut self, _lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { + fn link_whole_staticlib_by_name( + &mut self, + _name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { panic!("staticlibs not supported") } From d7712c6e28bf48056171513212420c9ecdd08d5f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2024 15:47:41 +0300 Subject: [PATCH 19/30] linker: Group library linking methods together and sort them consistently --- compiler/rustc_codegen_ssa/src/back/linker.rs | 351 +++++++++--------- 1 file changed, 180 insertions(+), 171 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 4360c10edc30a..2bddd473702e8 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -171,13 +171,13 @@ pub trait Linker { bug!("framework linked with unsupported linker") } fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool); - fn link_staticlib_by_path(&mut self, path: &Path); fn link_whole_staticlib_by_name( &mut self, name: &str, verbatim: bool, search_paths: &[PathBuf], ); + fn link_staticlib_by_path(&mut self, path: &Path); fn link_whole_staticlib_by_path(&mut self, path: &Path); fn include_path(&mut self, path: &Path); fn framework_path(&mut self, path: &Path); @@ -469,35 +469,6 @@ impl<'a> Linker for GccLinker<'a> { } } } - fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { - self.hint_static(); - self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },)); - } - fn link_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - self.cmd.arg(path); - } - fn include_path(&mut self, path: &Path) { - self.cmd.arg("-L").arg(path); - } - fn framework_path(&mut self, path: &Path) { - self.cmd.arg("-F").arg(path); - } - fn output_filename(&mut self, path: &Path) { - self.cmd.arg("-o").arg(path); - } - fn add_object(&mut self, path: &Path) { - self.cmd.arg(path); - } - fn full_relro(&mut self) { - self.linker_args(&["-z", "relro", "-z", "now"]); - } - fn partial_relro(&mut self) { - self.linker_args(&["-z", "relro"]); - } - fn no_relro(&mut self) { - self.linker_args(&["-z", "norelro"]); - } fn link_framework_by_name(&mut self, name: &str, _verbatim: bool, as_needed: bool) { self.hint_dynamic(); @@ -510,6 +481,11 @@ impl<'a> Linker for GccLinker<'a> { self.cmd.arg("-framework").arg(name); } + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { + self.hint_static(); + self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },)); + } + // Here we explicitly ask that the entire archive is included into the // result artifact. For more details see #15460, but the gist is that // the linker will strip away any unused objects in the archive if we @@ -537,6 +513,11 @@ impl<'a> Linker for GccLinker<'a> { } } + fn link_staticlib_by_path(&mut self, path: &Path) { + self.hint_static(); + self.cmd.arg(path); + } + fn link_whole_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); if self.sess.target.is_like_osx { @@ -548,6 +529,28 @@ impl<'a> Linker for GccLinker<'a> { } } + fn include_path(&mut self, path: &Path) { + self.cmd.arg("-L").arg(path); + } + fn framework_path(&mut self, path: &Path) { + self.cmd.arg("-F").arg(path); + } + fn output_filename(&mut self, path: &Path) { + self.cmd.arg("-o").arg(path); + } + fn add_object(&mut self, path: &Path) { + self.cmd.arg(path); + } + fn full_relro(&mut self) { + self.linker_args(&["-z", "relro", "-z", "now"]); + } + fn partial_relro(&mut self) { + self.linker_args(&["-z", "relro"]); + } + fn no_relro(&mut self) { + self.linker_args(&["-z", "norelro"]); + } + fn gc_sections(&mut self, keep_metadata: bool) { // The dead_strip option to the linker specifies that functions and data // unreachable by the entry point will be removed. This is quite useful @@ -827,9 +830,33 @@ impl<'a> Linker for MsvcLinker<'a> { } } + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { + self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); + } + + fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { + self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); + } + + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + _search_paths: &[PathBuf], + ) { + self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg(path); } + + fn link_whole_staticlib_by_path(&mut self, path: &Path) { + let mut arg = OsString::from("/WHOLEARCHIVE:"); + arg.push(path); + self.cmd.arg(arg); + } + fn add_object(&mut self, path: &Path) { self.cmd.arg(path); } @@ -851,14 +878,6 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg("/OPT:NOREF,NOICF"); } - fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { - self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); - } - - fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { - self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); - } - fn full_relro(&mut self) { // noop } @@ -895,19 +914,6 @@ impl<'a> Linker for MsvcLinker<'a> { bug!("frameworks are not supported on windows") } - fn link_whole_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - _search_paths: &[PathBuf], - ) { - self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); - } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - let mut arg = OsString::from("/WHOLEARCHIVE:"); - arg.push(path); - self.cmd.arg(arg); - } fn optimize(&mut self) { // Needs more investigation of `/OPT` arguments } @@ -1054,27 +1060,15 @@ impl<'a> Linker for EmLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn include_path(&mut self, path: &Path) { - self.cmd.arg("-L").arg(path); + fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { + // Emscripten always links statically + self.link_staticlib_by_name(name, verbatim); } fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { self.cmd.arg("-l").arg(name); } - fn output_filename(&mut self, path: &Path) { - self.cmd.arg("-o").arg(path); - } - - fn add_object(&mut self, path: &Path) { - self.cmd.arg(path); - } - - fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { - // Emscripten always links statically - self.link_staticlib_by_name(name, verbatim); - } - fn link_whole_staticlib_by_name( &mut self, name: &str, @@ -1085,13 +1079,25 @@ impl<'a> Linker for EmLinker<'a> { self.link_staticlib_by_name(name, verbatim); } + fn link_staticlib_by_path(&mut self, path: &Path) { + self.add_object(path); + } + fn link_whole_staticlib_by_path(&mut self, path: &Path) { // not supported? self.link_staticlib_by_path(path); } - fn link_staticlib_by_path(&mut self, path: &Path) { - self.add_object(path); + fn include_path(&mut self, path: &Path) { + self.cmd.arg("-L").arg(path); + } + + fn output_filename(&mut self, path: &Path) { + self.cmd.arg("-o").arg(path); + } + + fn add_object(&mut self, path: &Path) { + self.cmd.arg(path); } fn full_relro(&mut self) { @@ -1251,10 +1257,23 @@ impl<'a> Linker for WasmLd<'a> { self.cmd.arg("-l").arg(name); } + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { + self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg(path); } + fn link_whole_staticlib_by_path(&mut self, path: &Path) { + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + } + fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); } @@ -1277,19 +1296,6 @@ impl<'a> Linker for WasmLd<'a> { fn no_relro(&mut self) {} - fn link_whole_staticlib_by_name( - &mut self, - name: &str, - _verbatim: bool, - _search_paths: &[PathBuf], - ) { - self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); - } - fn gc_sections(&mut self, _keep_metadata: bool) { self.cmd.arg("--gc-sections"); } @@ -1389,17 +1395,42 @@ pub struct L4Bender<'a> { } impl<'a> Linker for L4Bender<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } + + fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("dylibs are not supported on L4Re"); } + fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { self.hint_static(); self.cmd.arg(format!("-PC{name}")); } + + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { + self.hint_static(); + self.cmd.arg("--whole-archive").arg(format!("-l{name}")); + self.cmd.arg("--no-whole-archive"); + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); self.cmd.arg(path); } + + fn link_whole_staticlib_by_path(&mut self, path: &Path) { + self.hint_static(); + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + } + fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); } @@ -1427,28 +1458,6 @@ impl<'a> Linker for L4Bender<'a> { self.cmd.arg("-z").arg("norelro"); } - fn cmd(&mut self) -> &mut Command { - &mut self.cmd - } - - fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - - fn link_whole_staticlib_by_name( - &mut self, - name: &str, - _verbatim: bool, - _search_paths: &[PathBuf], - ) { - self.hint_static(); - self.cmd.arg("--whole-archive").arg(format!("-l{name}")); - self.cmd.arg("--no-whole-archive"); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); - } - fn gc_sections(&mut self, keep_metadata: bool) { if !keep_metadata { self.cmd.arg("--gc-sections"); @@ -1559,6 +1568,24 @@ impl<'a> AixLinker<'a> { } impl<'a> Linker for AixLinker<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } + + fn set_output_kind(&mut self, output_kind: LinkOutputKind, out_filename: &Path) { + match output_kind { + LinkOutputKind::DynamicDylib => { + self.hint_dynamic(); + self.build_dylib(out_filename); + } + LinkOutputKind::StaticDylib => { + self.hint_static(); + self.build_dylib(out_filename); + } + _ => {} + } + } + fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) { self.hint_dynamic(); self.cmd.arg(format!("-l{name}")); @@ -1569,11 +1596,27 @@ impl<'a> Linker for AixLinker<'a> { self.cmd.arg(format!("-l{name}")); } + fn link_whole_staticlib_by_name( + &mut self, + name: &str, + verbatim: bool, + search_paths: &[PathBuf], + ) { + self.hint_static(); + let lib = find_native_static_library(name, verbatim, search_paths, self.sess); + self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.hint_static(); self.cmd.arg(path); } + fn link_whole_staticlib_by_path(&mut self, path: &Path) { + self.hint_static(); + self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); + } + fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); } @@ -1596,40 +1639,6 @@ impl<'a> Linker for AixLinker<'a> { fn no_relro(&mut self) {} - fn cmd(&mut self) -> &mut Command { - &mut self.cmd - } - - fn set_output_kind(&mut self, output_kind: LinkOutputKind, out_filename: &Path) { - match output_kind { - LinkOutputKind::DynamicDylib => { - self.hint_dynamic(); - self.build_dylib(out_filename); - } - LinkOutputKind::StaticDylib => { - self.hint_static(); - self.build_dylib(out_filename); - } - _ => {} - } - } - - fn link_whole_staticlib_by_name( - &mut self, - name: &str, - verbatim: bool, - search_paths: &[PathBuf], - ) { - self.hint_static(); - let lib = find_native_static_library(name, verbatim, search_paths, self.sess); - self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); - } - fn gc_sections(&mut self, _keep_metadata: bool) { self.cmd.arg("-bgc"); } @@ -1794,6 +1803,23 @@ impl<'a> Linker for PtxLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { + panic!("external dylibs not supported") + } + + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { + panic!("staticlibs not supported") + } + + fn link_whole_staticlib_by_name( + &mut self, + _name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { + panic!("staticlibs not supported") + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg("--rlib").arg(path); } @@ -1828,23 +1854,6 @@ impl<'a> Linker for PtxLinker<'a> { self.cmd.arg("-o").arg(path); } - fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { - panic!("external dylibs not supported") - } - - fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { - panic!("staticlibs not supported") - } - - fn link_whole_staticlib_by_name( - &mut self, - _name: &str, - _verbatim: bool, - _search_paths: &[PathBuf], - ) { - panic!("staticlibs not supported") - } - fn framework_path(&mut self, _path: &Path) { panic!("frameworks not supported") } @@ -1888,6 +1897,23 @@ impl<'a> Linker for BpfLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} + fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { + panic!("external dylibs not supported") + } + + fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { + panic!("staticlibs not supported") + } + + fn link_whole_staticlib_by_name( + &mut self, + _name: &str, + _verbatim: bool, + _search_paths: &[PathBuf], + ) { + panic!("staticlibs not supported") + } + fn link_staticlib_by_path(&mut self, path: &Path) { self.cmd.arg(path); } @@ -1923,23 +1949,6 @@ impl<'a> Linker for BpfLinker<'a> { self.cmd.arg("-o").arg(path); } - fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { - panic!("external dylibs not supported") - } - - fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { - panic!("staticlibs not supported") - } - - fn link_whole_staticlib_by_name( - &mut self, - _name: &str, - _verbatim: bool, - _search_paths: &[PathBuf], - ) { - panic!("staticlibs not supported") - } - fn framework_path(&mut self, _path: &Path) { panic!("frameworks not supported") } From 0ae891e6b48418115cca424d2a59a1eee34452e4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2024 16:13:13 +0300 Subject: [PATCH 20/30] linker: Do not collect search paths unless necessary --- compiler/rustc_codegen_ssa/src/back/link.rs | 25 +++++++++++-------- compiler/rustc_codegen_ssa/src/back/linker.rs | 25 +++++++++++-------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 18fa8a41c4622..5507f04fea1f7 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -52,6 +52,15 @@ use std::path::{Path, PathBuf}; use std::process::{ExitStatus, Output, Stdio}; use std::{env, fmt, fs, io, mem, str}; +#[derive(Default)] +pub struct SearchPaths(OnceCell>); + +impl SearchPaths { + pub(super) fn get(&self, sess: &Session) -> &[PathBuf] { + self.0.get_or_init(|| archive_search_paths(sess)) + } +} + pub fn ensure_removed(dcx: &DiagCtxt, path: &Path) { if let Err(e) = fs::remove_file(path) { if e.kind() != io::ErrorKind::NotFound { @@ -2445,7 +2454,7 @@ fn add_native_libs_from_crate( archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, tmpdir: &Path, - search_paths: &OnceCell>, + search_paths: &SearchPaths, bundled_libs: &FxHashSet, cnum: CrateNum, link_static: bool, @@ -2513,11 +2522,7 @@ fn add_native_libs_from_crate( } } else { if whole_archive { - cmd.link_whole_staticlib_by_name( - name, - verbatim, - search_paths.get_or_init(|| archive_search_paths(sess)), - ); + cmd.link_whole_staticlib_by_name(name, verbatim, search_paths); } else { cmd.link_staticlib_by_name(name, verbatim) } @@ -2581,7 +2586,7 @@ fn add_local_native_libraries( } } - let search_paths = OnceCell::new(); + let search_paths = SearchPaths::default(); // All static and dynamic native library dependencies are linked to the local crate. let link_static = true; let link_dynamic = true; @@ -2623,7 +2628,7 @@ fn add_upstream_rust_crates<'a>( .find(|(ty, _)| *ty == crate_type) .expect("failed to find crate type in dependency format list"); - let search_paths = OnceCell::new(); + let search_paths = SearchPaths::default(); for &cnum in &codegen_results.crate_info.used_crates { // We may not pass all crates through to the linker. Some crates may appear statically in // an existing dylib, meaning we'll pick up all the symbols from the dylib. @@ -2698,7 +2703,7 @@ fn add_upstream_native_libraries( tmpdir: &Path, link_output_kind: LinkOutputKind, ) { - let search_path = OnceCell::new(); + let search_paths = SearchPaths::default(); for &cnum in &codegen_results.crate_info.used_crates { // Static libraries are not linked here, they are linked in `add_upstream_rust_crates`. // FIXME: Merge this function to `add_upstream_rust_crates` so that all native libraries @@ -2720,7 +2725,7 @@ fn add_upstream_native_libraries( archive_builder_builder, codegen_results, tmpdir, - &search_path, + &search_paths, &Default::default(), cnum, link_static, diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 2bddd473702e8..4b38be6f1889a 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1,5 +1,6 @@ use super::command::Command; use super::symbol_export; +use crate::back::link::SearchPaths; use crate::errors; use rustc_span::symbol::sym; @@ -175,7 +176,7 @@ pub trait Linker { &mut self, name: &str, verbatim: bool, - search_paths: &[PathBuf], + search_paths: &SearchPaths, ); fn link_staticlib_by_path(&mut self, path: &Path); fn link_whole_staticlib_by_path(&mut self, path: &Path); @@ -496,7 +497,7 @@ impl<'a> Linker for GccLinker<'a> { &mut self, name: &str, verbatim: bool, - search_paths: &[PathBuf], + search_paths: &SearchPaths, ) { self.hint_static(); let target = &self.sess.target; @@ -508,7 +509,8 @@ impl<'a> Linker for GccLinker<'a> { // -force_load is the macOS equivalent of --whole-archive, but it // involves passing the full path to the library to link. self.linker_arg("-force_load"); - let lib = find_native_static_library(name, verbatim, search_paths, self.sess); + let lib = + find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); self.linker_arg(&lib); } } @@ -842,7 +844,7 @@ impl<'a> Linker for MsvcLinker<'a> { &mut self, name: &str, verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); } @@ -1073,7 +1075,7 @@ impl<'a> Linker for EmLinker<'a> { &mut self, name: &str, verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { // not supported? self.link_staticlib_by_name(name, verbatim); @@ -1261,7 +1263,7 @@ impl<'a> Linker for WasmLd<'a> { &mut self, name: &str, _verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); } @@ -1414,7 +1416,7 @@ impl<'a> Linker for L4Bender<'a> { &mut self, name: &str, _verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { self.hint_static(); self.cmd.arg("--whole-archive").arg(format!("-l{name}")); @@ -1600,10 +1602,11 @@ impl<'a> Linker for AixLinker<'a> { &mut self, name: &str, verbatim: bool, - search_paths: &[PathBuf], + search_paths: &SearchPaths, ) { self.hint_static(); - let lib = find_native_static_library(name, verbatim, search_paths, self.sess); + let lib = + find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); } @@ -1815,7 +1818,7 @@ impl<'a> Linker for PtxLinker<'a> { &mut self, _name: &str, _verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { panic!("staticlibs not supported") } @@ -1909,7 +1912,7 @@ impl<'a> Linker for BpfLinker<'a> { &mut self, _name: &str, _verbatim: bool, - _search_paths: &[PathBuf], + _search_paths: &SearchPaths, ) { panic!("staticlibs not supported") } From e77af0ff26565ff1318cdc45d8ea381e01973ba5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2024 17:41:18 +0300 Subject: [PATCH 21/30] linker: Merge `link_staticlib_*` and `link_whole_staticlib_*` --- compiler/rustc_codegen_ssa/src/back/link.rs | 18 +- compiler/rustc_codegen_ssa/src/back/linker.rs | 198 +++++++----------- 2 files changed, 86 insertions(+), 130 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 5507f04fea1f7..e3b4189b3f5a3 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1282,7 +1282,7 @@ fn link_sanitizer_runtime( } else { let filename = format!("librustc{channel}_rt.{name}.a"); let path = find_sanitizer_runtime(sess, &filename).join(&filename); - linker.link_whole_staticlib_by_path(&path); + linker.link_staticlib_by_path(&path, true); } } @@ -2514,18 +2514,10 @@ fn add_native_libs_from_crate( if let Some(filename) = lib.filename { // If rlib contains native libs as archives, they are unpacked to tmpdir. let path = tmpdir.join(filename.as_str()); - if whole_archive { - cmd.link_whole_staticlib_by_path(&path); - } else { - cmd.link_staticlib_by_path(&path); - } + cmd.link_staticlib_by_path(&path, whole_archive); } } else { - if whole_archive { - cmd.link_whole_staticlib_by_name(name, verbatim, search_paths); - } else { - cmd.link_staticlib_by_name(name, verbatim) - } + cmd.link_staticlib_by_name(name, verbatim, whole_archive, search_paths); } } } @@ -2539,7 +2531,7 @@ fn add_native_libs_from_crate( // link kind is unspecified. if !link_output_kind.can_link_dylib() && !sess.target.crt_static_allows_dylibs { if link_static { - cmd.link_staticlib_by_name(name, verbatim) + cmd.link_staticlib_by_name(name, verbatim, false, search_paths); } } else { if link_dynamic { @@ -2796,7 +2788,7 @@ fn add_static_crate<'a>( } else { fix_windows_verbatim_for_gcc(path) }; - cmd.link_staticlib_by_path(&rlib_path); + cmd.link_staticlib_by_path(&rlib_path, false); }; if !are_upstream_rust_objects_already_included(sess) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 4b38be6f1889a..092b66e386867 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -171,15 +171,14 @@ pub trait Linker { fn link_framework_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) { bug!("framework linked with unsupported linker") } - fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool); - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, verbatim: bool, + whole_archive: bool, search_paths: &SearchPaths, ); - fn link_staticlib_by_path(&mut self, path: &Path); - fn link_whole_staticlib_by_path(&mut self, path: &Path); + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool); fn include_path(&mut self, path: &Path); fn framework_path(&mut self, path: &Path); fn output_filename(&mut self, path: &Path); @@ -482,26 +481,17 @@ impl<'a> Linker for GccLinker<'a> { self.cmd.arg("-framework").arg(name); } - fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { - self.hint_static(); - self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },)); - } - - // Here we explicitly ask that the entire archive is included into the - // result artifact. For more details see #15460, but the gist is that - // the linker will strip away any unused objects in the archive if we - // don't otherwise explicitly reference them. This can occur for - // libraries which are just providing bindings, libraries with generic - // functions, etc. - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, verbatim: bool, + whole_archive: bool, search_paths: &SearchPaths, ) { self.hint_static(); - let target = &self.sess.target; - if !target.is_like_osx { + if !whole_archive { + self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" })); + } else if !self.sess.target.is_like_osx { self.linker_arg("--whole-archive"); self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" })); self.linker_arg("--no-whole-archive"); @@ -515,14 +505,11 @@ impl<'a> Linker for GccLinker<'a> { } } - fn link_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { self.hint_static(); - self.cmd.arg(path); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - if self.sess.target.is_like_osx { + if !whole_archive { + self.cmd.arg(path); + } else if self.sess.target.is_like_osx { self.linker_arg("-force_load"); self.linker_arg(&path); } else { @@ -836,27 +823,28 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); } - fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool) { - self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, verbatim: bool, + whole_archive: bool, _search_paths: &SearchPaths, ) { - self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); - } - - fn link_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg(path); + if !whole_archive { + self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); + } else { + self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); + } } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - let mut arg = OsString::from("/WHOLEARCHIVE:"); - arg.push(path); - self.cmd.arg(arg); + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { + if !whole_archive { + self.cmd.arg(path); + } else { + let mut arg = OsString::from("/WHOLEARCHIVE:"); + arg.push(path); + self.cmd.arg(arg); + } } fn add_object(&mut self, path: &Path) { @@ -1062,34 +1050,25 @@ impl<'a> Linker for EmLinker<'a> { fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {} - fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, _as_needed: bool) { + fn link_dylib_by_name(&mut self, name: &str, _verbatim: bool, _as_needed: bool) { // Emscripten always links statically - self.link_staticlib_by_name(name, verbatim); - } - - fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { self.cmd.arg("-l").arg(name); } - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, - verbatim: bool, + _verbatim: bool, + _whole_archive: bool, _search_paths: &SearchPaths, ) { - // not supported? - self.link_staticlib_by_name(name, verbatim); + self.cmd.arg("-l").arg(name); } - fn link_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, _whole_archive: bool) { self.add_object(path); } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - // not supported? - self.link_staticlib_by_path(path); - } - fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); } @@ -1255,25 +1234,26 @@ impl<'a> Linker for WasmLd<'a> { self.cmd.arg("-l").arg(name); } - fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { - self.cmd.arg("-l").arg(name); - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, _verbatim: bool, + whole_archive: bool, _search_paths: &SearchPaths, ) { - self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); - } - - fn link_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg(path); + if !whole_archive { + self.cmd.arg("-l").arg(name); + } else { + self.cmd.arg("--whole-archive").arg("-l").arg(name).arg("--no-whole-archive"); + } } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { + if !whole_archive { + self.cmd.arg(path); + } else { + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + } } fn include_path(&mut self, path: &Path) { @@ -1407,30 +1387,29 @@ impl<'a> Linker for L4Bender<'a> { bug!("dylibs are not supported on L4Re"); } - fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { - self.hint_static(); - self.cmd.arg(format!("-PC{name}")); - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, _verbatim: bool, + whole_archive: bool, _search_paths: &SearchPaths, ) { self.hint_static(); - self.cmd.arg("--whole-archive").arg(format!("-l{name}")); - self.cmd.arg("--no-whole-archive"); - } - - fn link_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - self.cmd.arg(path); + if !whole_archive { + self.cmd.arg(format!("-PC{name}")); + } else { + self.cmd.arg("--whole-archive").arg(format!("-l{name}")); + self.cmd.arg("--no-whole-archive"); + } } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { self.hint_static(); - self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + if !whole_archive { + self.cmd.arg(path); + } else { + self.cmd.arg("--whole-archive").arg(path).arg("--no-whole-archive"); + } } fn include_path(&mut self, path: &Path) { @@ -1593,31 +1572,30 @@ impl<'a> Linker for AixLinker<'a> { self.cmd.arg(format!("-l{name}")); } - fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool) { - self.hint_static(); - self.cmd.arg(format!("-l{name}")); - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, name: &str, verbatim: bool, + whole_archive: bool, search_paths: &SearchPaths, ) { self.hint_static(); - let lib = - find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); - self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); - } - - fn link_staticlib_by_path(&mut self, path: &Path) { - self.hint_static(); - self.cmd.arg(path); + if !whole_archive { + self.cmd.arg(format!("-l{name}")); + } else { + let lib = + find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); + self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); + } } - fn link_whole_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { self.hint_static(); - self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); + if !whole_archive { + self.cmd.arg(path); + } else { + self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); + } } fn include_path(&mut self, path: &Path) { @@ -1810,24 +1788,17 @@ impl<'a> Linker for PtxLinker<'a> { panic!("external dylibs not supported") } - fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { - panic!("staticlibs not supported") - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, _name: &str, _verbatim: bool, + _whole_archive: bool, _search_paths: &SearchPaths, ) { panic!("staticlibs not supported") } - fn link_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg("--rlib").arg(path); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, _whole_archive: bool) { self.cmd.arg("--rlib").arg(path); } @@ -1904,24 +1875,17 @@ impl<'a> Linker for BpfLinker<'a> { panic!("external dylibs not supported") } - fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool) { - panic!("staticlibs not supported") - } - - fn link_whole_staticlib_by_name( + fn link_staticlib_by_name( &mut self, _name: &str, _verbatim: bool, + _whole_archive: bool, _search_paths: &SearchPaths, ) { panic!("staticlibs not supported") } - fn link_staticlib_by_path(&mut self, path: &Path) { - self.cmd.arg(path); - } - - fn link_whole_staticlib_by_path(&mut self, path: &Path) { + fn link_staticlib_by_path(&mut self, path: &Path, _whole_archive: bool) { self.cmd.arg(path); } From 42bcccc3ffafa851a118ff1c3a98998c64a682e5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2024 18:09:59 +0300 Subject: [PATCH 22/30] linker: Cleanup implementations of `link_staticlib_*` --- compiler/rustc_codegen_ssa/src/back/linker.rs | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 092b66e386867..9f06f398288f2 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -489,19 +489,19 @@ impl<'a> Linker for GccLinker<'a> { search_paths: &SearchPaths, ) { self.hint_static(); + let colon = if verbatim && self.is_gnu { ":" } else { "" }; if !whole_archive { - self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" })); - } else if !self.sess.target.is_like_osx { - self.linker_arg("--whole-archive"); - self.cmd.arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" })); - self.linker_arg("--no-whole-archive"); - } else { + self.cmd.arg(format!("-l{colon}{name}")); + } else if self.sess.target.is_like_osx { // -force_load is the macOS equivalent of --whole-archive, but it // involves passing the full path to the library to link. self.linker_arg("-force_load"); - let lib = - find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); - self.linker_arg(&lib); + let search_paths = search_paths.get(self.sess); + self.linker_arg(find_native_static_library(name, verbatim, search_paths, self.sess)); + } else { + self.linker_arg("--whole-archive"); + self.cmd.arg(format!("-l{colon}{name}")); + self.linker_arg("--no-whole-archive"); } } @@ -511,9 +511,10 @@ impl<'a> Linker for GccLinker<'a> { self.cmd.arg(path); } else if self.sess.target.is_like_osx { self.linker_arg("-force_load"); - self.linker_arg(&path); + self.linker_arg(path); } else { - self.linker_args(&[OsString::from("--whole-archive"), path.into()]); + self.linker_arg("--whole-archive"); + self.linker_arg(path); self.linker_arg("--no-whole-archive"); } } @@ -830,11 +831,9 @@ impl<'a> Linker for MsvcLinker<'a> { whole_archive: bool, _search_paths: &SearchPaths, ) { - if !whole_archive { - self.cmd.arg(format!("{}{}", name, if verbatim { "" } else { ".lib" })); - } else { - self.cmd.arg(format!("/WHOLEARCHIVE:{}{}", name, if verbatim { "" } else { ".lib" })); - } + let prefix = if whole_archive { "/WHOLEARCHIVE:" } else { "" }; + let suffix = if verbatim { "" } else { ".lib" }; + self.cmd.arg(format!("{prefix}{name}{suffix}")); } fn link_staticlib_by_path(&mut self, path: &Path, whole_archive: bool) { @@ -1066,7 +1065,7 @@ impl<'a> Linker for EmLinker<'a> { } fn link_staticlib_by_path(&mut self, path: &Path, _whole_archive: bool) { - self.add_object(path); + self.cmd.arg(path); } fn include_path(&mut self, path: &Path) { @@ -1398,8 +1397,7 @@ impl<'a> Linker for L4Bender<'a> { if !whole_archive { self.cmd.arg(format!("-PC{name}")); } else { - self.cmd.arg("--whole-archive").arg(format!("-l{name}")); - self.cmd.arg("--no-whole-archive"); + self.cmd.arg("--whole-archive").arg(format!("-l{name}")).arg("--no-whole-archive"); } } @@ -1583,9 +1581,10 @@ impl<'a> Linker for AixLinker<'a> { if !whole_archive { self.cmd.arg(format!("-l{name}")); } else { - let lib = - find_native_static_library(name, verbatim, search_paths.get(self.sess), self.sess); - self.cmd.arg(format!("-bkeepfile:{}", lib.to_str().unwrap())); + let mut arg = OsString::from("-bkeepfile:"); + let search_path = search_paths.get(self.sess); + arg.push(find_native_static_library(name, verbatim, search_path, self.sess)); + self.cmd.arg(arg); } } @@ -1594,7 +1593,9 @@ impl<'a> Linker for AixLinker<'a> { if !whole_archive { self.cmd.arg(path); } else { - self.cmd.arg(format!("-bkeepfile:{}", path.to_str().unwrap())); + let mut arg = OsString::from("-bkeepfile:"); + arg.push(path); + self.cmd.arg(arg); } } From 161c674ef051b93978b96f6df70f03094144c537 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 20 Jan 2024 19:01:57 +0000 Subject: [PATCH 23/30] Add Assume custom MIR. --- .../src/build/custom/parse/instruction.rs | 4 ++ compiler/rustc_span/src/symbol.rs | 1 + library/core/src/intrinsics/mir.rs | 2 + .../assume.assume_constant.built.after.mir | 10 +++++ .../assume.assume_local.built.after.mir | 10 +++++ .../assume.assume_place.built.after.mir | 10 +++++ tests/mir-opt/building/custom/assume.rs | 44 +++++++++++++++++++ 7 files changed, 81 insertions(+) create mode 100644 tests/mir-opt/building/custom/assume.assume_constant.built.after.mir create mode 100644 tests/mir-opt/building/custom/assume.assume_local.built.after.mir create mode 100644 tests/mir-opt/building/custom/assume.assume_place.built.after.mir create mode 100644 tests/mir-opt/building/custom/assume.rs diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 5428333a11611..c669d3fd6230d 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -20,6 +20,10 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { @call(mir_storage_dead, args) => { Ok(StatementKind::StorageDead(self.parse_local(args[0])?)) }, + @call(mir_assume, args) => { + let op = self.parse_operand(args[0])?; + Ok(StatementKind::Intrinsic(Box::new(NonDivergingIntrinsic::Assume(op)))) + }, @call(mir_deinit, args) => { Ok(StatementKind::Deinit(Box::new(self.parse_place(args[0])?))) }, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 7b0138d50baed..6d10fdf49d7f6 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1027,6 +1027,7 @@ symbols! { minnumf32, minnumf64, mips_target_feature, + mir_assume, mir_basic_block, mir_call, mir_cast_transmute, diff --git a/library/core/src/intrinsics/mir.rs b/library/core/src/intrinsics/mir.rs index 334e32b26b184..d348e31609d48 100644 --- a/library/core/src/intrinsics/mir.rs +++ b/library/core/src/intrinsics/mir.rs @@ -357,6 +357,8 @@ define!("mir_unwind_resume", define!("mir_storage_live", fn StorageLive(local: T)); define!("mir_storage_dead", fn StorageDead(local: T)); +#[cfg(not(bootstrap))] +define!("mir_assume", fn Assume(operand: bool)); define!("mir_deinit", fn Deinit(place: T)); define!("mir_checked", fn Checked(binop: T) -> (T, bool)); define!("mir_len", fn Len(place: T) -> usize); diff --git a/tests/mir-opt/building/custom/assume.assume_constant.built.after.mir b/tests/mir-opt/building/custom/assume.assume_constant.built.after.mir new file mode 100644 index 0000000000000..8e70d0a1e9b11 --- /dev/null +++ b/tests/mir-opt/building/custom/assume.assume_constant.built.after.mir @@ -0,0 +1,10 @@ +// MIR for `assume_constant` after built + +fn assume_constant() -> () { + let mut _0: (); + + bb0: { + assume(const true); + return; + } +} diff --git a/tests/mir-opt/building/custom/assume.assume_local.built.after.mir b/tests/mir-opt/building/custom/assume.assume_local.built.after.mir new file mode 100644 index 0000000000000..7ea1fcd30c2ec --- /dev/null +++ b/tests/mir-opt/building/custom/assume.assume_local.built.after.mir @@ -0,0 +1,10 @@ +// MIR for `assume_local` after built + +fn assume_local(_1: bool) -> () { + let mut _0: (); + + bb0: { + assume(_1); + return; + } +} diff --git a/tests/mir-opt/building/custom/assume.assume_place.built.after.mir b/tests/mir-opt/building/custom/assume.assume_place.built.after.mir new file mode 100644 index 0000000000000..ce914618d3dce --- /dev/null +++ b/tests/mir-opt/building/custom/assume.assume_place.built.after.mir @@ -0,0 +1,10 @@ +// MIR for `assume_place` after built + +fn assume_place(_1: (bool, u8)) -> () { + let mut _0: (); + + bb0: { + assume((_1.0: bool)); + return; + } +} diff --git a/tests/mir-opt/building/custom/assume.rs b/tests/mir-opt/building/custom/assume.rs new file mode 100644 index 0000000000000..a477e12f0e03d --- /dev/null +++ b/tests/mir-opt/building/custom/assume.rs @@ -0,0 +1,44 @@ +// skip-filecheck +#![feature(custom_mir, core_intrinsics)] + +extern crate core; +use core::intrinsics::mir::*; + +// EMIT_MIR assume.assume_local.built.after.mir +#[custom_mir(dialect = "built")] +fn assume_local(x: bool) { + mir!( + { + Assume(x); + Return() + } + ) +} + +// EMIT_MIR assume.assume_place.built.after.mir +#[custom_mir(dialect = "built")] +fn assume_place(p: (bool, u8)) { + mir!( + { + Assume(p.0); + Return() + } + ) +} + +// EMIT_MIR assume.assume_constant.built.after.mir +#[custom_mir(dialect = "built")] +fn assume_constant() { + mir!( + { + Assume(true); + Return() + } + ) +} + +fn main() { + assume_local(true); + assume_place((true, 50)); + assume_constant(); +} From d7a7be4049672d3a3b5a1a8380fbe843b52775c2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 20 Jan 2024 19:11:45 +0000 Subject: [PATCH 24/30] Add test for jump-threading assume. --- ...ding.assume.JumpThreading.panic-abort.diff | 39 +++++++++++++++ ...ing.assume.JumpThreading.panic-unwind.diff | 39 +++++++++++++++ tests/mir-opt/jump_threading.rs | 48 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 tests/mir-opt/jump_threading.assume.JumpThreading.panic-abort.diff create mode 100644 tests/mir-opt/jump_threading.assume.JumpThreading.panic-unwind.diff diff --git a/tests/mir-opt/jump_threading.assume.JumpThreading.panic-abort.diff b/tests/mir-opt/jump_threading.assume.JumpThreading.panic-abort.diff new file mode 100644 index 0000000000000..f1f0106fdbce9 --- /dev/null +++ b/tests/mir-opt/jump_threading.assume.JumpThreading.panic-abort.diff @@ -0,0 +1,39 @@ +- // MIR for `assume` before JumpThreading ++ // MIR for `assume` after JumpThreading + + fn assume(_1: u8, _2: bool) -> u8 { + let mut _0: u8; + + bb0: { + switchInt(_1) -> [7: bb1, otherwise: bb2]; + } + + bb1: { + assume(_2); +- goto -> bb3; ++ goto -> bb6; + } + + bb2: { + goto -> bb3; + } + + bb3: { + switchInt(_2) -> [0: bb4, otherwise: bb5]; + } + + bb4: { + _0 = const 4_u8; + return; + } + + bb5: { + _0 = const 5_u8; + return; ++ } ++ ++ bb6: { ++ goto -> bb5; + } + } + diff --git a/tests/mir-opt/jump_threading.assume.JumpThreading.panic-unwind.diff b/tests/mir-opt/jump_threading.assume.JumpThreading.panic-unwind.diff new file mode 100644 index 0000000000000..f1f0106fdbce9 --- /dev/null +++ b/tests/mir-opt/jump_threading.assume.JumpThreading.panic-unwind.diff @@ -0,0 +1,39 @@ +- // MIR for `assume` before JumpThreading ++ // MIR for `assume` after JumpThreading + + fn assume(_1: u8, _2: bool) -> u8 { + let mut _0: u8; + + bb0: { + switchInt(_1) -> [7: bb1, otherwise: bb2]; + } + + bb1: { + assume(_2); +- goto -> bb3; ++ goto -> bb6; + } + + bb2: { + goto -> bb3; + } + + bb3: { + switchInt(_2) -> [0: bb4, otherwise: bb5]; + } + + bb4: { + _0 = const 4_u8; + return; + } + + bb5: { + _0 = const 5_u8; + return; ++ } ++ ++ bb6: { ++ goto -> bb5; + } + } + diff --git a/tests/mir-opt/jump_threading.rs b/tests/mir-opt/jump_threading.rs index 7c2fa42828be6..a66fe8b57e718 100644 --- a/tests/mir-opt/jump_threading.rs +++ b/tests/mir-opt/jump_threading.rs @@ -468,6 +468,52 @@ fn aggregate(x: u8) -> u8 { } } +/// Verify that we can leverage the existence of an `Assume` terminator. +#[custom_mir(dialect = "runtime", phase = "post-cleanup")] +fn assume(a: u8, b: bool) -> u8 { + // CHECK-LABEL: fn assume( + mir!( + { + // CHECK: bb0: { + // CHECK-NEXT: switchInt(_1) -> [7: bb1, otherwise: bb2] + match a { 7 => bb1, _ => bb2 } + } + bb1 = { + // CHECK: bb1: { + // CHECK-NEXT: assume(_2); + // CHECK-NEXT: goto -> bb6; + Assume(b); + Goto(bb3) + } + bb2 = { + // CHECK: bb2: { + // CHECK-NEXT: goto -> bb3; + Goto(bb3) + } + bb3 = { + // CHECK: bb3: { + // CHECK-NEXT: switchInt(_2) -> [0: bb4, otherwise: bb5]; + match b { false => bb4, _ => bb5 } + } + bb4 = { + // CHECK: bb4: { + // CHECK-NEXT: _0 = const 4_u8; + // CHECK-NEXT: return; + RET = 4; + Return() + } + bb5 = { + // CHECK: bb5: { + // CHECK-NEXT: _0 = const 5_u8; + // CHECK-NEXT: return; + RET = 5; + Return() + } + // CHECK: bb6: { + // CHECK-NEXT: goto -> bb5; + ) +} + fn main() { // CHECK-LABEL: fn main( too_complex(Ok(0)); @@ -481,6 +527,7 @@ fn main() { renumbered_bb(true); disappearing_bb(7); aggregate(7); + assume(7, false); } // EMIT_MIR jump_threading.too_complex.JumpThreading.diff @@ -494,3 +541,4 @@ fn main() { // EMIT_MIR jump_threading.renumbered_bb.JumpThreading.diff // EMIT_MIR jump_threading.disappearing_bb.JumpThreading.diff // EMIT_MIR jump_threading.aggregate.JumpThreading.diff +// EMIT_MIR jump_threading.assume.JumpThreading.diff From afaac75ac76cfbc38066d0474f8ca69d92ca184d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 20 Jan 2024 19:11:59 +0000 Subject: [PATCH 25/30] Do not thread through Assert terminator. --- .../rustc_mir_transform/src/jump_threading.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_transform/src/jump_threading.rs b/compiler/rustc_mir_transform/src/jump_threading.rs index e87f68a09057b..7a70ed5cb7f0e 100644 --- a/compiler/rustc_mir_transform/src/jump_threading.rs +++ b/compiler/rustc_mir_transform/src/jump_threading.rs @@ -566,11 +566,6 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { cost: &CostChecker<'_, 'tcx>, depth: usize, ) { - let register_opportunity = |c: Condition| { - debug!(?bb, ?c.target, "register"); - self.opportunities.push(ThreadingOpportunity { chain: vec![bb], target: c.target }) - }; - let term = self.body.basic_blocks[bb].terminator(); let place_to_flood = match term.kind { // We come from a target, so those are not possible. @@ -592,16 +587,8 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { // Flood the overwritten place, and progress through. TerminatorKind::Drop { place: destination, .. } | TerminatorKind::Call { destination, .. } => Some(destination), - // Treat as an `assume(cond == expected)`. - TerminatorKind::Assert { ref cond, expected, .. } => { - if let Some(place) = cond.place() - && let Some(conditions) = state.try_get(place.as_ref(), self.map) - { - let expected = if expected { ScalarInt::TRUE } else { ScalarInt::FALSE }; - conditions.iter_matches(expected).for_each(register_opportunity); - } - None - } + // Ignore, as this can be a no-op at codegen time. + TerminatorKind::Assert { .. } => None, }; // We can recurse through this terminator. From f3682a1304200d554d2d36abf21376861b9ae14a Mon Sep 17 00:00:00 2001 From: "HTGAzureX1212." <39023054+HTGAzureX1212@users.noreply.github.com> Date: Tue, 23 Jan 2024 10:56:33 +0800 Subject: [PATCH 26/30] add list of characters to uncommon codepoints lint --- compiler/rustc_errors/src/diagnostic_impls.rs | 8 ++++++++ compiler/rustc_lint/messages.ftl | 2 +- compiler/rustc_lint/src/lints.rs | 4 +++- compiler/rustc_lint/src/non_ascii_idents.rs | 11 ++++++++++- tests/ui/lexer/lex-emoji-identifiers.stderr | 2 +- .../lint-uncommon-codepoints.stderr | 6 +++--- 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 39252dea28303..f6679ae9bb352 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -110,6 +110,14 @@ impl IntoDiagnosticArg for char { } } +impl IntoDiagnosticArg for Vec { + fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { + DiagnosticArgValue::StrListSepByAnd( + self.into_iter().map(|c| Cow::Owned(format!("{c:?}"))).collect(), + ) + } +} + impl IntoDiagnosticArg for Symbol { fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { self.to_ident_string().into_diagnostic_arg() diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 33f96139f2011..ac456c69c57db 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -240,7 +240,7 @@ lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of lint_identifier_non_ascii_char = identifier contains non-ASCII characters -lint_identifier_uncommon_codepoints = identifier contains uncommon Unicode codepoints +lint_identifier_uncommon_codepoints = identifier contains uncommon Unicode codepoints: {$codepoints} lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 94ecc7d95877b..7d63fad304447 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1107,7 +1107,9 @@ pub struct IdentifierNonAsciiChar; #[derive(LintDiagnostic)] #[diag(lint_identifier_uncommon_codepoints)] -pub struct IdentifierUncommonCodepoints; +pub struct IdentifierUncommonCodepoints { + pub codepoints: Vec, +} #[derive(LintDiagnostic)] #[diag(lint_confusable_identifier_pair)] diff --git a/compiler/rustc_lint/src/non_ascii_idents.rs b/compiler/rustc_lint/src/non_ascii_idents.rs index 00f87a5af80fc..ec11f7a6130de 100644 --- a/compiler/rustc_lint/src/non_ascii_idents.rs +++ b/compiler/rustc_lint/src/non_ascii_idents.rs @@ -190,7 +190,16 @@ impl EarlyLintPass for NonAsciiIdents { if check_uncommon_codepoints && !symbol_str.chars().all(GeneralSecurityProfile::identifier_allowed) { - cx.emit_span_lint(UNCOMMON_CODEPOINTS, sp, IdentifierUncommonCodepoints); + cx.emit_span_lint( + UNCOMMON_CODEPOINTS, + sp, + IdentifierUncommonCodepoints { + codepoints: symbol_str + .chars() + .filter(|c| !GeneralSecurityProfile::identifier_allowed(*c)) + .collect(), + }, + ); } } diff --git a/tests/ui/lexer/lex-emoji-identifiers.stderr b/tests/ui/lexer/lex-emoji-identifiers.stderr index 747825fa2a988..568bde254fb07 100644 --- a/tests/ui/lexer/lex-emoji-identifiers.stderr +++ b/tests/ui/lexer/lex-emoji-identifiers.stderr @@ -40,7 +40,7 @@ error: identifiers cannot contain emoji: `folded🙏🏿` LL | let folded🙏🏿 = "modifier sequence"; | ^^^^^^^^^^ -warning: identifier contains uncommon Unicode codepoints +warning: identifier contains uncommon Unicode codepoints: '\u{fe0f}' --> $DIR/lex-emoji-identifiers.rs:6:9 | LL | let key1️⃣ = "keycap sequence"; diff --git a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr index 0533da03068ae..4df13014f7c85 100644 --- a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr +++ b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr @@ -1,4 +1,4 @@ -error: identifier contains uncommon Unicode codepoints +error: identifier contains uncommon Unicode codepoints: 'µ' --> $DIR/lint-uncommon-codepoints.rs:3:7 | LL | const µ: f64 = 0.000001; @@ -10,13 +10,13 @@ note: the lint level is defined here LL | #![deny(uncommon_codepoints)] | ^^^^^^^^^^^^^^^^^^^ -error: identifier contains uncommon Unicode codepoints +error: identifier contains uncommon Unicode codepoints: 'ij' --> $DIR/lint-uncommon-codepoints.rs:6:4 | LL | fn dijkstra() {} | ^^^^^^^ -error: identifier contains uncommon Unicode codepoints +error: identifier contains uncommon Unicode codepoints: 'ㇻ', 'ㇲ', and 'ㇳ' --> $DIR/lint-uncommon-codepoints.rs:9:9 | LL | let ㇻㇲㇳ = "rust"; From 34f4f3da4f3e2ac8fa086c4d3a3c89d49d23a263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jan 2024 04:42:26 +0000 Subject: [PATCH 27/30] Suggest boxing both arms of if expr if that solves divergent arms involving `impl Trait` When encountering the following ```rust // run-rustfix trait Trait {} struct Struct; impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } fn bar() -> impl Trait { Struct } fn main() { let _ = if true { Struct } else { foo() //~ ERROR E0308 }; let _ = if true { foo() } else { Struct //~ ERROR E0308 }; let _ = if true { Struct } else { bar() // impl Trait }; let _ = if true { bar() // impl Trait } else { Struct }; } ``` suggest boxing both arms ```rust let _ = if true { Box::new(Struct) as Box } else { Box::new(bar()) }; let _ = if true { Box::new(bar()) as Box } else { Box::new(Struct) }; ``` --- .../infer/error_reporting/note_and_explain.rs | 86 +++++++++++++++---- ...uggest-box-on-divergent-if-else-arms.fixed | 13 +++ .../suggest-box-on-divergent-if-else-arms.rs | 13 +++ ...ggest-box-on-divergent-if-else-arms.stderr | 56 +++++++++++- 4 files changed, 149 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index a0dfaf33a6e85..01cd3c57925b3 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -294,8 +294,9 @@ impl Trait for X { ); } } - (ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) - if alias.def_id.is_local() + (_, ty::Alias(ty::Opaque, opaque_ty)) + | (ty::Alias(ty::Opaque, opaque_ty), _) => { + if opaque_ty.def_id.is_local() && matches!( tcx.def_kind(body_owner_def_id), DefKind::Fn @@ -303,21 +304,74 @@ impl Trait for X { | DefKind::Const | DefKind::AssocFn | DefKind::AssocConst - ) => - { - if tcx.is_type_alias_impl_trait(alias.def_id) { - if !tcx + ) + && tcx.is_type_alias_impl_trait(opaque_ty.def_id) + && !tcx .opaque_types_defined_by(body_owner_def_id.expect_local()) - .contains(&alias.def_id.expect_local()) - { - let sp = tcx - .def_ident_span(body_owner_def_id) - .unwrap_or_else(|| tcx.def_span(body_owner_def_id)); - diag.span_note( - sp, - "\ - this item must have the opaque type in its signature \ - in order to be able to register hidden types", + .contains(&opaque_ty.def_id.expect_local()) + { + let sp = tcx + .def_ident_span(body_owner_def_id) + .unwrap_or_else(|| tcx.def_span(body_owner_def_id)); + diag.span_note( + sp, + "this item must have the opaque type in its signature in order to \ + be able to register hidden types", + ); + } + // If two if arms can be coerced to a trait object, provide a structured + // suggestion. + let ObligationCauseCode::IfExpression(cause) = cause.code() else { + return; + }; + let hir::Node::Block(blk) = self.tcx.hir_node(cause.then_id) else { + return; + }; + let Some(then) = blk.expr else { + return; + }; + let hir::Node::Block(blk) = self.tcx.hir_node(cause.else_id) else { + return; + }; + let Some(else_) = blk.expr else { + return; + }; + let expected = match values.found.kind() { + ty::Alias(..) => values.expected, + _ => values.found, + }; + let preds = tcx.explicit_item_bounds(opaque_ty.def_id); + for (pred, _span) in preds.skip_binder() { + let ty::ClauseKind::Trait(trait_predicate) = pred.kind().skip_binder() + else { + continue; + }; + if trait_predicate.polarity != ty::ImplPolarity::Positive { + continue; + } + let def_id = trait_predicate.def_id(); + let mut impl_def_ids = vec![]; + tcx.for_each_relevant_impl(def_id, expected, |did| { + impl_def_ids.push(did) + }); + if let [_] = &impl_def_ids[..] { + let trait_name = tcx.item_name(def_id); + diag.multipart_suggestion( + format!( + "`{expected}` implements `{trait_name}` so you can box \ + both arms and coerce to the trait object \ + `Box`", + ), + vec![ + (then.span.shrink_to_lo(), "Box::new(".to_string()), + ( + then.span.shrink_to_hi(), + format!(") as Box", tcx.def_path_str(def_id)), + ), + (else_.span.shrink_to_lo(), "Box::new(".to_string()), + (else_.span.shrink_to_hi(), ")".to_string()), + ], + MachineApplicable, ); } } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed index dfdf6b24bf7e0..9ce46bc1a65b2 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -5,6 +5,9 @@ impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } +fn bar() -> impl Trait { + Struct +} fn main() { let _ = if true { Box::new(Struct) @@ -16,4 +19,14 @@ fn main() { } else { Box::new(Struct) //~ ERROR E0308 }; + let _ = if true { + Box::new(Struct) as Box + } else { + Box::new(bar()) //~ ERROR E0308 + }; + let _ = if true { + Box::new(bar()) as Box + } else { + Box::new(Struct) //~ ERROR E0308 + }; } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs index e138ad9b33647..7f65a3bb59d31 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -5,6 +5,9 @@ impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } +fn bar() -> impl Trait { + Struct +} fn main() { let _ = if true { Struct @@ -16,4 +19,14 @@ fn main() { } else { Struct //~ ERROR E0308 }; + let _ = if true { + Struct + } else { + bar() //~ ERROR E0308 + }; + let _ = if true { + bar() + } else { + Struct //~ ERROR E0308 + }; } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr index 98c9d880f97aa..c58bf60e7d656 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -1,5 +1,5 @@ error[E0308]: `if` and `else` have incompatible types - --> $DIR/suggest-box-on-divergent-if-else-arms.rs:12:9 + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:15:9 | LL | let _ = if true { | _____________- @@ -19,7 +19,7 @@ LL | Box::new(Struct) | +++++++++ + error[E0308]: `if` and `else` have incompatible types - --> $DIR/suggest-box-on-divergent-if-else-arms.rs:17:9 + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:20:9 | LL | let _ = if true { | _____________- @@ -39,6 +39,56 @@ help: store this in the heap by calling `Box::new` LL | Box::new(Struct) | +++++++++ + -error: aborting due to 2 previous errors +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:25:9 + | +LL | fn bar() -> impl Trait { + | ---------- the found opaque type +... +LL | let _ = if true { + | _____________- +LL | | Struct + | | ------ expected because of this +LL | | } else { +LL | | bar() + | | ^^^^^ expected `Struct`, found opaque type +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Struct` + found opaque type `impl Trait` +help: `Struct` implements `Trait` so you can box both arms and coerce to the trait object `Box` + | +LL ~ Box::new(Struct) as Box +LL | } else { +LL ~ Box::new(bar()) + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:30:9 + | +LL | fn bar() -> impl Trait { + | ---------- the expected opaque type +... +LL | let _ = if true { + | _____________- +LL | | bar() + | | ----- expected because of this +LL | | } else { +LL | | Struct + | | ^^^^^^ expected opaque type, found `Struct` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected opaque type `impl Trait` + found struct `Struct` +help: `Struct` implements `Trait` so you can box both arms and coerce to the trait object `Box` + | +LL ~ Box::new(bar()) as Box +LL | } else { +LL ~ Box::new(Struct) + | + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`. From 3a07333a8aa873fcb75d50541f7f209e2a04f80f Mon Sep 17 00:00:00 2001 From: "HTGAzureX1212." <39023054+HTGAzureX1212@users.noreply.github.com> Date: Tue, 23 Jan 2024 21:16:24 +0800 Subject: [PATCH 28/30] address requested changes --- compiler/rustc_lint/messages.ftl | 5 ++++- compiler/rustc_lint/src/lints.rs | 1 + compiler/rustc_lint/src/non_ascii_idents.rs | 13 +++++++------ tests/ui/lexer/lex-emoji-identifiers.rs | 2 +- tests/ui/lexer/lex-emoji-identifiers.stderr | 2 +- .../lint-uncommon-codepoints.rs | 4 ++-- .../lint-uncommon-codepoints.stderr | 4 ++-- 7 files changed, 18 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index ac456c69c57db..b4506990d4fca 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -240,7 +240,10 @@ lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of lint_identifier_non_ascii_char = identifier contains non-ASCII characters -lint_identifier_uncommon_codepoints = identifier contains uncommon Unicode codepoints: {$codepoints} +lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> + [one] an uncommon Unicode codepoint + *[other] uncommon Unicode codepoints +}: {$codepoints} lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 7d63fad304447..e19bb1cb62f5a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1109,6 +1109,7 @@ pub struct IdentifierNonAsciiChar; #[diag(lint_identifier_uncommon_codepoints)] pub struct IdentifierUncommonCodepoints { pub codepoints: Vec, + pub codepoints_len: usize, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/non_ascii_idents.rs b/compiler/rustc_lint/src/non_ascii_idents.rs index ec11f7a6130de..f78b32ce5e77b 100644 --- a/compiler/rustc_lint/src/non_ascii_idents.rs +++ b/compiler/rustc_lint/src/non_ascii_idents.rs @@ -190,15 +190,16 @@ impl EarlyLintPass for NonAsciiIdents { if check_uncommon_codepoints && !symbol_str.chars().all(GeneralSecurityProfile::identifier_allowed) { + let codepoints: Vec<_> = symbol_str + .chars() + .filter(|c| !GeneralSecurityProfile::identifier_allowed(*c)) + .collect(); + let codepoints_len = codepoints.len(); + cx.emit_span_lint( UNCOMMON_CODEPOINTS, sp, - IdentifierUncommonCodepoints { - codepoints: symbol_str - .chars() - .filter(|c| !GeneralSecurityProfile::identifier_allowed(*c)) - .collect(), - }, + IdentifierUncommonCodepoints { codepoints, codepoints_len }, ); } } diff --git a/tests/ui/lexer/lex-emoji-identifiers.rs b/tests/ui/lexer/lex-emoji-identifiers.rs index decf2f0058721..bbc088521b7bd 100644 --- a/tests/ui/lexer/lex-emoji-identifiers.rs +++ b/tests/ui/lexer/lex-emoji-identifiers.rs @@ -4,7 +4,7 @@ fn invalid_emoji_usages() { let wireless🛜 = "basic emoji"; //~ ERROR: identifiers cannot contain emoji // FIXME let key1️⃣ = "keycap sequence"; //~ ERROR: unknown start of token - //~^ WARN: identifier contains uncommon Unicode codepoints + //~^ WARN: identifier contains an uncommon Unicode codepoint let flag🇺🇳 = "flag sequence"; //~ ERROR: identifiers cannot contain emoji let wales🏴 = "tag sequence"; //~ ERROR: identifiers cannot contain emoji let folded🙏🏿 = "modifier sequence"; //~ ERROR: identifiers cannot contain emoji diff --git a/tests/ui/lexer/lex-emoji-identifiers.stderr b/tests/ui/lexer/lex-emoji-identifiers.stderr index 568bde254fb07..679b7422bc150 100644 --- a/tests/ui/lexer/lex-emoji-identifiers.stderr +++ b/tests/ui/lexer/lex-emoji-identifiers.stderr @@ -40,7 +40,7 @@ error: identifiers cannot contain emoji: `folded🙏🏿` LL | let folded🙏🏿 = "modifier sequence"; | ^^^^^^^^^^ -warning: identifier contains uncommon Unicode codepoints: '\u{fe0f}' +warning: identifier contains an uncommon Unicode codepoint: '\u{fe0f}' --> $DIR/lex-emoji-identifiers.rs:6:9 | LL | let key1️⃣ = "keycap sequence"; diff --git a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs index ed8e7ddddc597..c3459930a94c0 100644 --- a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs +++ b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs @@ -1,9 +1,9 @@ #![deny(uncommon_codepoints)] -const µ: f64 = 0.000001; //~ ERROR identifier contains uncommon Unicode codepoints +const µ: f64 = 0.000001; //~ ERROR identifier contains an uncommon Unicode codepoint //~| WARNING should have an upper case name -fn dijkstra() {} //~ ERROR identifier contains uncommon Unicode codepoints +fn dijkstra() {} //~ ERROR identifier contains an uncommon Unicode codepoint fn main() { let ㇻㇲㇳ = "rust"; //~ ERROR identifier contains uncommon Unicode codepoints diff --git a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr index 4df13014f7c85..bae5ac654d354 100644 --- a/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr +++ b/tests/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr @@ -1,4 +1,4 @@ -error: identifier contains uncommon Unicode codepoints: 'µ' +error: identifier contains an uncommon Unicode codepoint: 'µ' --> $DIR/lint-uncommon-codepoints.rs:3:7 | LL | const µ: f64 = 0.000001; @@ -10,7 +10,7 @@ note: the lint level is defined here LL | #![deny(uncommon_codepoints)] | ^^^^^^^^^^^^^^^^^^^ -error: identifier contains uncommon Unicode codepoints: 'ij' +error: identifier contains an uncommon Unicode codepoint: 'ij' --> $DIR/lint-uncommon-codepoints.rs:6:4 | LL | fn dijkstra() {} From da1d0c4a6913dded1deaba602f677674dcbbe21f Mon Sep 17 00:00:00 2001 From: "HTGAzureX1212." <39023054+HTGAzureX1212@users.noreply.github.com> Date: Tue, 23 Jan 2024 21:17:06 +0800 Subject: [PATCH 29/30] tidy --- compiler/rustc_lint/messages.ftl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index b4506990d4fca..5652a34103b09 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -240,7 +240,7 @@ lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of lint_identifier_non_ascii_char = identifier contains non-ASCII characters -lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> +lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> [one] an uncommon Unicode codepoint *[other] uncommon Unicode codepoints }: {$codepoints} From 851d4c4e249f38c82846737af9cd80cccc5b2f02 Mon Sep 17 00:00:00 2001 From: bohan Date: Sun, 21 Jan 2024 19:17:28 +0800 Subject: [PATCH 30/30] add several resolution test cases --- tests/ui/imports/ambiguous-2.rs | 1 + tests/ui/imports/ambiguous-4.rs | 2 +- .../glob-conflict-cross-crate-2-extern.rs | 10 +++++++ .../auxiliary/issue-114682-2-extern.rs | 17 +++++++++++ .../auxiliary/issue-114682-3-extern.rs | 16 +++++++++++ .../auxiliary/issue-114682-4-extern.rs | 10 +++++++ .../auxiliary/issue-114682-5-extern-1.rs | 1 + .../auxiliary/issue-114682-5-extern-2.rs | 13 +++++++++ .../auxiliary/issue-114682-6-extern.rs | 9 ++++++ tests/ui/imports/extern-with-ambiguous-2.rs | 2 ++ tests/ui/imports/extern-with-ambiguous-3.rs | 2 ++ ...rate.rs => glob-conflict-cross-crate-1.rs} | 4 +++ ...err => glob-conflict-cross-crate-1.stderr} | 4 +-- .../ui/imports/glob-conflict-cross-crate-2.rs | 10 +++++++ .../glob-conflict-cross-crate-2.stderr | 9 ++++++ .../ui/imports/glob-conflict-cross-crate-3.rs | 16 +++++++++++ tests/ui/imports/issue-114682-1.rs | 25 +++++++++++++++++ tests/ui/imports/issue-114682-1.stderr | 28 +++++++++++++++++++ tests/ui/imports/issue-114682-2.rs | 19 +++++++++++++ tests/ui/imports/issue-114682-2.stderr | 9 ++++++ tests/ui/imports/issue-114682-3.rs | 24 ++++++++++++++++ tests/ui/imports/issue-114682-4.rs | 13 +++++++++ tests/ui/imports/issue-114682-5.rs | 15 ++++++++++ tests/ui/imports/issue-114682-6.rs | 13 +++++++++ 24 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 tests/ui/imports/auxiliary/glob-conflict-cross-crate-2-extern.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-2-extern.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-3-extern.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-4-extern.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-5-extern-1.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-5-extern-2.rs create mode 100644 tests/ui/imports/auxiliary/issue-114682-6-extern.rs rename tests/ui/imports/{glob-conflict-cross-crate.rs => glob-conflict-cross-crate-1.rs} (54%) rename tests/ui/imports/{glob-conflict-cross-crate.stderr => glob-conflict-cross-crate-1.stderr} (82%) create mode 100644 tests/ui/imports/glob-conflict-cross-crate-2.rs create mode 100644 tests/ui/imports/glob-conflict-cross-crate-2.stderr create mode 100644 tests/ui/imports/glob-conflict-cross-crate-3.rs create mode 100644 tests/ui/imports/issue-114682-1.rs create mode 100644 tests/ui/imports/issue-114682-1.stderr create mode 100644 tests/ui/imports/issue-114682-2.rs create mode 100644 tests/ui/imports/issue-114682-2.stderr create mode 100644 tests/ui/imports/issue-114682-3.rs create mode 100644 tests/ui/imports/issue-114682-4.rs create mode 100644 tests/ui/imports/issue-114682-5.rs create mode 100644 tests/ui/imports/issue-114682-6.rs diff --git a/tests/ui/imports/ambiguous-2.rs b/tests/ui/imports/ambiguous-2.rs index 7b38f3006b104..2918feb059107 100644 --- a/tests/ui/imports/ambiguous-2.rs +++ b/tests/ui/imports/ambiguous-2.rs @@ -6,4 +6,5 @@ extern crate ambiguous_1; fn main() { ambiguous_1::id(); + //^ FIXME: `id` should be identified as an ambiguous item. } diff --git a/tests/ui/imports/ambiguous-4.rs b/tests/ui/imports/ambiguous-4.rs index 24ae33784c526..1e8f5be5a882e 100644 --- a/tests/ui/imports/ambiguous-4.rs +++ b/tests/ui/imports/ambiguous-4.rs @@ -5,5 +5,5 @@ extern crate ambiguous_4_extern; fn main() { ambiguous_4_extern::id(); - // `warning_ambiguous` had been lost at metadata. + //^ FIXME: `id` should be identified as an ambiguous item. } diff --git a/tests/ui/imports/auxiliary/glob-conflict-cross-crate-2-extern.rs b/tests/ui/imports/auxiliary/glob-conflict-cross-crate-2-extern.rs new file mode 100644 index 0000000000000..5dec6d4699496 --- /dev/null +++ b/tests/ui/imports/auxiliary/glob-conflict-cross-crate-2-extern.rs @@ -0,0 +1,10 @@ +mod a { + pub type C = i8; +} + +mod b { + pub type C = i16; +} + +pub use a::*; +pub use b::*; diff --git a/tests/ui/imports/auxiliary/issue-114682-2-extern.rs b/tests/ui/imports/auxiliary/issue-114682-2-extern.rs new file mode 100644 index 0000000000000..df2af78916682 --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-2-extern.rs @@ -0,0 +1,17 @@ +macro_rules! m { + () => { + pub fn max() {} + pub(crate) mod max {} + }; +} + +mod d { + m! {} +} + +mod e { + pub type max = i32; +} + +pub use self::d::*; +pub use self::e::*; diff --git a/tests/ui/imports/auxiliary/issue-114682-3-extern.rs b/tests/ui/imports/auxiliary/issue-114682-3-extern.rs new file mode 100644 index 0000000000000..999b66342fe00 --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-3-extern.rs @@ -0,0 +1,16 @@ +mod gio { + pub trait SettingsExt { + fn abc(&self) {} + } + impl SettingsExt for T {} +} + +mod gtk { + pub trait SettingsExt { + fn efg(&self) {} + } + impl SettingsExt for T {} +} + +pub use gtk::*; +pub use gio::*; diff --git a/tests/ui/imports/auxiliary/issue-114682-4-extern.rs b/tests/ui/imports/auxiliary/issue-114682-4-extern.rs new file mode 100644 index 0000000000000..86663f11b31eb --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-4-extern.rs @@ -0,0 +1,10 @@ +mod a { + pub type Result = std::result::Result; +} + +mod b { + pub type Result = std::result::Result; +} + +pub use a::*; +pub use b::*; diff --git a/tests/ui/imports/auxiliary/issue-114682-5-extern-1.rs b/tests/ui/imports/auxiliary/issue-114682-5-extern-1.rs new file mode 100644 index 0000000000000..ebf6493f9f718 --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-5-extern-1.rs @@ -0,0 +1 @@ +pub struct Url; diff --git a/tests/ui/imports/auxiliary/issue-114682-5-extern-2.rs b/tests/ui/imports/auxiliary/issue-114682-5-extern-2.rs new file mode 100644 index 0000000000000..9dbefdd531be3 --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-5-extern-2.rs @@ -0,0 +1,13 @@ +// edition: 2018 +// aux-build: issue-114682-5-extern-1.rs +// compile-flags: --extern issue_114682_5_extern_1 + +pub mod p { + pub use crate::types::*; + pub use crate::*; +} +mod types { + pub mod issue_114682_5_extern_1 {} +} + +pub use issue_114682_5_extern_1; diff --git a/tests/ui/imports/auxiliary/issue-114682-6-extern.rs b/tests/ui/imports/auxiliary/issue-114682-6-extern.rs new file mode 100644 index 0000000000000..caf3c4e35a0e9 --- /dev/null +++ b/tests/ui/imports/auxiliary/issue-114682-6-extern.rs @@ -0,0 +1,9 @@ +mod a { + pub fn log() {} +} +mod b { + pub fn log() {} +} + +pub use self::a::*; +pub use self::b::*; diff --git a/tests/ui/imports/extern-with-ambiguous-2.rs b/tests/ui/imports/extern-with-ambiguous-2.rs index 68c623c1c4a65..b7c9cccdb6402 100644 --- a/tests/ui/imports/extern-with-ambiguous-2.rs +++ b/tests/ui/imports/extern-with-ambiguous-2.rs @@ -12,5 +12,7 @@ mod s { use s::*; use extern_with_ambiguous_2_extern::*; use error::*; +//^ FIXME: An ambiguity error should be thrown for `error`, +// as there is ambiguity present within `extern-with-ambiguous-2-extern.rs`. fn main() {} diff --git a/tests/ui/imports/extern-with-ambiguous-3.rs b/tests/ui/imports/extern-with-ambiguous-3.rs index 282c1d569b0cc..44a9a2a00a453 100644 --- a/tests/ui/imports/extern-with-ambiguous-3.rs +++ b/tests/ui/imports/extern-with-ambiguous-3.rs @@ -13,5 +13,7 @@ mod s { use s::*; use extern_with_ambiguous_3_extern::*; use error::*; +//^ FIXME: An ambiguity error should be thrown for `error`, +// as there is ambiguity present within `extern-with-ambiguous-3-extern.rs`. fn main() {} diff --git a/tests/ui/imports/glob-conflict-cross-crate.rs b/tests/ui/imports/glob-conflict-cross-crate-1.rs similarity index 54% rename from tests/ui/imports/glob-conflict-cross-crate.rs rename to tests/ui/imports/glob-conflict-cross-crate-1.rs index d84c243f2139c..832e6c888a646 100644 --- a/tests/ui/imports/glob-conflict-cross-crate.rs +++ b/tests/ui/imports/glob-conflict-cross-crate-1.rs @@ -4,5 +4,9 @@ extern crate glob_conflict; fn main() { glob_conflict::f(); //~ ERROR cannot find function `f` in crate `glob_conflict` + //^ FIXME: `glob_conflict::f` should raise an + // ambiguity error instead of a not found error. glob_conflict::glob::f(); //~ ERROR cannot find function `f` in module `glob_conflict::glob` + //^ FIXME: `glob_conflict::glob::f` should raise an + // ambiguity error instead of a not found error. } diff --git a/tests/ui/imports/glob-conflict-cross-crate.stderr b/tests/ui/imports/glob-conflict-cross-crate-1.stderr similarity index 82% rename from tests/ui/imports/glob-conflict-cross-crate.stderr rename to tests/ui/imports/glob-conflict-cross-crate-1.stderr index 0e3b4222fe44f..758087107f397 100644 --- a/tests/ui/imports/glob-conflict-cross-crate.stderr +++ b/tests/ui/imports/glob-conflict-cross-crate-1.stderr @@ -1,11 +1,11 @@ error[E0425]: cannot find function `f` in crate `glob_conflict` - --> $DIR/glob-conflict-cross-crate.rs:6:20 + --> $DIR/glob-conflict-cross-crate-1.rs:6:20 | LL | glob_conflict::f(); | ^ not found in `glob_conflict` error[E0425]: cannot find function `f` in module `glob_conflict::glob` - --> $DIR/glob-conflict-cross-crate.rs:7:26 + --> $DIR/glob-conflict-cross-crate-1.rs:9:26 | LL | glob_conflict::glob::f(); | ^ not found in `glob_conflict::glob` diff --git a/tests/ui/imports/glob-conflict-cross-crate-2.rs b/tests/ui/imports/glob-conflict-cross-crate-2.rs new file mode 100644 index 0000000000000..6ba71ad30ac54 --- /dev/null +++ b/tests/ui/imports/glob-conflict-cross-crate-2.rs @@ -0,0 +1,10 @@ +// aux-build:glob-conflict-cross-crate-2-extern.rs + +extern crate glob_conflict_cross_crate_2_extern; + +use glob_conflict_cross_crate_2_extern::*; + +fn main() { + let _a: C = 1; //~ ERROR cannot find type `C` in this scope + //^ FIXME: `C` should be identified as an ambiguous item. +} diff --git a/tests/ui/imports/glob-conflict-cross-crate-2.stderr b/tests/ui/imports/glob-conflict-cross-crate-2.stderr new file mode 100644 index 0000000000000..aebb2d59d063a --- /dev/null +++ b/tests/ui/imports/glob-conflict-cross-crate-2.stderr @@ -0,0 +1,9 @@ +error[E0412]: cannot find type `C` in this scope + --> $DIR/glob-conflict-cross-crate-2.rs:8:13 + | +LL | let _a: C = 1; + | ^ not found in this scope + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0412`. diff --git a/tests/ui/imports/glob-conflict-cross-crate-3.rs b/tests/ui/imports/glob-conflict-cross-crate-3.rs new file mode 100644 index 0000000000000..535d87d8ea284 --- /dev/null +++ b/tests/ui/imports/glob-conflict-cross-crate-3.rs @@ -0,0 +1,16 @@ +// check-pass +// aux-build:glob-conflict-cross-crate-2-extern.rs + +extern crate glob_conflict_cross_crate_2_extern; + +mod a { + pub type C = i32; +} + +use glob_conflict_cross_crate_2_extern::*; +use a::*; + +fn main() { + let _a: C = 1; + //^ FIXME: `C` should be identified as an ambiguous item. +} diff --git a/tests/ui/imports/issue-114682-1.rs b/tests/ui/imports/issue-114682-1.rs new file mode 100644 index 0000000000000..88fe05e51444d --- /dev/null +++ b/tests/ui/imports/issue-114682-1.rs @@ -0,0 +1,25 @@ +// https://github.com/rust-lang/rust/pull/114682#discussion_r1420534109 + +#![feature(decl_macro)] + +macro_rules! mac { + () => { + pub macro A() { + println!("non import") + } + } +} + +mod m { + pub macro A() { + println!("import") + } +} + +pub use m::*; +mac!(); + +fn main() { + A!(); + //~^ ERROR `A` is ambiguous +} diff --git a/tests/ui/imports/issue-114682-1.stderr b/tests/ui/imports/issue-114682-1.stderr new file mode 100644 index 0000000000000..85fb7f7919e4e --- /dev/null +++ b/tests/ui/imports/issue-114682-1.stderr @@ -0,0 +1,28 @@ +error[E0659]: `A` is ambiguous + --> $DIR/issue-114682-1.rs:23:5 + | +LL | A!(); + | ^ ambiguous name + | + = note: ambiguous because of a conflict between a name from a glob import and a macro-expanded name in the same module during import or macro resolution +note: `A` could refer to the macro defined here + --> $DIR/issue-114682-1.rs:7:9 + | +LL | / pub macro A() { +LL | | println!("non import") +LL | | } + | |_________^ +... +LL | mac!(); + | ------ in this macro invocation +note: `A` could also refer to the macro imported here + --> $DIR/issue-114682-1.rs:19:9 + | +LL | pub use m::*; + | ^^^^ + = help: consider adding an explicit import of `A` to disambiguate + = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0659`. diff --git a/tests/ui/imports/issue-114682-2.rs b/tests/ui/imports/issue-114682-2.rs new file mode 100644 index 0000000000000..491105e62efea --- /dev/null +++ b/tests/ui/imports/issue-114682-2.rs @@ -0,0 +1,19 @@ +// aux-build: issue-114682-2-extern.rs +// https://github.com/rust-lang/rust/pull/114682#issuecomment-1879998900 + +extern crate issue_114682_2_extern; + +use issue_114682_2_extern::max; + +type A = issue_114682_2_extern::max; +//~^ ERROR: expected type, found function `issue_114682_2_extern::max` +// FIXME: +// The above error was emitted due to `(Mod(issue_114682_2_extern), Namespace(Type), Ident(max))` +// being identified as an ambiguous item. +// However, there are two points worth discussing: +// First, should this ambiguous item be omitted considering the maximum visibility +// of `issue_114682_2_extern::m::max` in the type namespace is only within the extern crate. +// Second, if we retain the ambiguous item of the extern crate, should it be treated +// as an ambiguous item within the local crate for the same reasoning? + +fn main() {} diff --git a/tests/ui/imports/issue-114682-2.stderr b/tests/ui/imports/issue-114682-2.stderr new file mode 100644 index 0000000000000..972bcecb56bc2 --- /dev/null +++ b/tests/ui/imports/issue-114682-2.stderr @@ -0,0 +1,9 @@ +error[E0573]: expected type, found function `issue_114682_2_extern::max` + --> $DIR/issue-114682-2.rs:8:10 + | +LL | type A = issue_114682_2_extern::max; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ not a type + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0573`. diff --git a/tests/ui/imports/issue-114682-3.rs b/tests/ui/imports/issue-114682-3.rs new file mode 100644 index 0000000000000..0f658bfe1597a --- /dev/null +++ b/tests/ui/imports/issue-114682-3.rs @@ -0,0 +1,24 @@ +// check-pass +// aux-build: issue-114682-3-extern.rs +// https://github.com/rust-lang/rust/pull/114682#issuecomment-1880625909 + +extern crate issue_114682_3_extern; + +use issue_114682_3_extern::*; + +mod auto { + pub trait SettingsExt { + fn ext(&self) {} + } + + impl SettingsExt for T {} +} + +pub use self::auto::*; + +fn main() { + let a: u8 = 1; + a.ext(); + //^ FIXME: it should report `ext` not found because `SettingsExt` + // is an ambiguous item in `issue-114682-3-extern.rs`. +} diff --git a/tests/ui/imports/issue-114682-4.rs b/tests/ui/imports/issue-114682-4.rs new file mode 100644 index 0000000000000..97615c1041049 --- /dev/null +++ b/tests/ui/imports/issue-114682-4.rs @@ -0,0 +1,13 @@ +// check-pass +// aux-build: issue-114682-4-extern.rs +// https://github.com/rust-lang/rust/pull/114682#issuecomment-1880755441 + +extern crate issue_114682_4_extern; + +use issue_114682_4_extern::*; + +fn a() -> Result { // FIXME: `Result` should be identified as an ambiguous item. + Ok(1) +} + +fn main() {} diff --git a/tests/ui/imports/issue-114682-5.rs b/tests/ui/imports/issue-114682-5.rs new file mode 100644 index 0000000000000..eb5ac10495bee --- /dev/null +++ b/tests/ui/imports/issue-114682-5.rs @@ -0,0 +1,15 @@ +// check-pass +// edition: 2018 +// aux-build: issue-114682-5-extern-1.rs +// aux-build: issue-114682-5-extern-2.rs +// compile-flags: --extern issue_114682_5_extern_1 +// https://github.com/rust-lang/rust/pull/114682#issuecomment-1880755441 + +extern crate issue_114682_5_extern_2; + +use issue_114682_5_extern_2::p::*; +use issue_114682_5_extern_1::Url; +// FIXME: The `issue_114682_5_extern_1` should be considered an ambiguous item, +// as it has already been recognized as ambiguous in `issue_114682_5_extern_2`. + +fn main() {} diff --git a/tests/ui/imports/issue-114682-6.rs b/tests/ui/imports/issue-114682-6.rs new file mode 100644 index 0000000000000..29a7d9e942643 --- /dev/null +++ b/tests/ui/imports/issue-114682-6.rs @@ -0,0 +1,13 @@ +// check-pass +// aux-build: issue-114682-6-extern.rs +// https://github.com/rust-lang/rust/pull/114682#issuecomment-1880755441 + +extern crate issue_114682_6_extern; + +use issue_114682_6_extern::*; + +fn main() { + let log = 2; + //^ `log` should be identified as an ambiguous item. + let _ = log; +}