From 1a6266340e53e3f663b49f71a14461dafa47de47 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 10:51:42 +1100 Subject: [PATCH 1/7] Add a test with an empty crate name. This error was untested. --- tests/ui/attributes/crate-name-empty.rs | 5 +++++ tests/ui/attributes/crate-name-empty.stderr | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 tests/ui/attributes/crate-name-empty.rs create mode 100644 tests/ui/attributes/crate-name-empty.stderr diff --git a/tests/ui/attributes/crate-name-empty.rs b/tests/ui/attributes/crate-name-empty.rs new file mode 100644 index 000000000000..dfba77a52def --- /dev/null +++ b/tests/ui/attributes/crate-name-empty.rs @@ -0,0 +1,5 @@ +// Ensure we reject `#![crate_name = ""]`. + +#![crate_name = ""] //~ ERROR crate name must not be empty + +fn main() {} diff --git a/tests/ui/attributes/crate-name-empty.stderr b/tests/ui/attributes/crate-name-empty.stderr new file mode 100644 index 000000000000..509a42d05f71 --- /dev/null +++ b/tests/ui/attributes/crate-name-empty.stderr @@ -0,0 +1,8 @@ +error: crate name must not be empty + --> $DIR/crate-name-empty.rs:3:1 + | +LL | #![crate_name = ""] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From bd61e0129f4ff393ad350f05442a9f1ac3ddbd44 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 24 Mar 2025 18:58:18 +1100 Subject: [PATCH 2/7] Use `Ident::dummy()` in `dummy_annotatable`. This is exactly the kind of case `Ident::dummy()` is for. --- compiler/rustc_builtin_macros/src/derive.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_builtin_macros/src/derive.rs b/compiler/rustc_builtin_macros/src/derive.rs index 2653a9f48b9b..e259f5b3955b 100644 --- a/compiler/rustc_builtin_macros/src/derive.rs +++ b/compiler/rustc_builtin_macros/src/derive.rs @@ -103,7 +103,7 @@ impl MultiItemModifier for Expander { fn dummy_annotatable() -> Annotatable { Annotatable::GenericParam(ast::GenericParam { id: ast::DUMMY_NODE_ID, - ident: Ident::empty(), + ident: Ident::dummy(), attrs: Default::default(), bounds: Default::default(), is_placeholder: false, From 2469ab195a558b3a09b5ef4319ea8a3f989b621f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 24 Mar 2025 18:59:44 +1100 Subject: [PATCH 3/7] Use `Option` in `panic_call`. Instead of `kw::Empty`. It makes it clearer that this is a name that is searched for and might not be found. --- compiler/rustc_lint/src/non_fmt_panic.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs index ac9f8d92dacb..16c061008085 100644 --- a/compiler/rustc_lint/src/non_fmt_panic.rs +++ b/compiler/rustc_lint/src/non_fmt_panic.rs @@ -7,7 +7,7 @@ use rustc_parse_format::{ParseMode, Parser, Piece}; use rustc_session::lint::FutureIncompatibilityReason; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::edition::Edition; -use rustc_span::{InnerSpan, Span, Symbol, hygiene, kw, sym}; +use rustc_span::{InnerSpan, Span, Symbol, hygiene, sym}; use rustc_trait_selection::infer::InferCtxtExt; use crate::lints::{NonFmtPanicBraces, NonFmtPanicUnused}; @@ -167,7 +167,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc .get_diagnostic_item(sym::Debug) .is_some_and(|t| infcx.type_implements_trait(t, [ty], param_env).may_apply()); - let suggest_panic_any = !is_str && panic == sym::std_panic_macro; + let suggest_panic_any = !is_str && panic == Some(sym::std_panic_macro); let fmt_applicability = if suggest_panic_any { // If we can use panic_any, use that as the MachineApplicable suggestion. @@ -297,10 +297,13 @@ fn find_delimiters(cx: &LateContext<'_>, span: Span) -> Option<(Span, Span, char )) } -fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol, Symbol) { +fn panic_call<'tcx>( + cx: &LateContext<'tcx>, + f: &'tcx hir::Expr<'tcx>, +) -> (Span, Option, Symbol) { let mut expn = f.span.ctxt().outer_expn_data(); - let mut panic_macro = kw::Empty; + let mut panic_macro = None; // Unwrap more levels of macro expansion, as panic_2015!() // was likely expanded from panic!() and possibly from @@ -320,7 +323,7 @@ fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, break; } expn = parent; - panic_macro = name; + panic_macro = Some(name); } let macro_symbol = From 3f32a321bc0962cd3c700a5625dca4e4967918e4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 09:09:01 +1100 Subject: [PATCH 4/7] Use `Option` in `ModuleKind::Def`. This way, `None` represents "crate root without a name" instead of `kw::Empty`. This changes makes it impossible to forget to handle the exceptional case. --- compiler/rustc_resolve/src/build_reduced_graph.rs | 10 +++++----- compiler/rustc_resolve/src/diagnostics.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 15 ++++++++------- compiler/rustc_resolve/src/macros.rs | 11 ++++++----- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 763e9207a126..08648a164ab7 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -131,7 +131,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let expn_id = self.cstore().expn_that_defined_untracked(def_id, self.tcx.sess); return Some(self.new_module( parent, - ModuleKind::Def(def_kind, def_id, self.tcx.item_name(def_id)), + ModuleKind::Def(def_kind, def_id, Some(self.tcx.item_name(def_id))), expn_id, self.def_span(def_id), // FIXME: Account for `#[no_implicit_prelude]` attributes. @@ -594,7 +594,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { // HACK(eddyb) unclear how good this is, but keeping `$crate` // in `source` breaks `tests/ui/imports/import-crate-var.rs`, // while the current crate doesn't have a valid `crate_name`. - if crate_name != kw::Empty { + if let Some(crate_name) = crate_name { // `crate_name` should not be interpreted as relative. module_path.push(Segment::from_ident_and_id( Ident { name: kw::PathRoot, span: source.ident.span }, @@ -603,7 +603,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { source.ident.name = crate_name; } if rename.is_none() { - ident.name = crate_name; + ident.name = sym::dummy; } self.r.dcx().emit_err(errors::CrateImported { span: item.span }); @@ -775,7 +775,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ItemKind::Mod(.., ref mod_kind) => { let module = self.r.new_module( Some(parent), - ModuleKind::Def(def_kind, def_id, ident.name), + ModuleKind::Def(def_kind, def_id, Some(ident.name)), expansion.to_expn_id(), item.span, parent.no_implicit_prelude @@ -811,7 +811,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ItemKind::Enum(_, _) | ItemKind::Trait(..) => { let module = self.r.new_module( Some(parent), - ModuleKind::Def(def_kind, def_id, ident.name), + ModuleKind::Def(def_kind, def_id, Some(ident.name)), expansion.to_expn_id(), item.span, parent.no_implicit_prelude, diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 5361af98f3c7..d683fd98bd21 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2439,7 +2439,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else { return None; }; - let module_name = crate_module.kind.name().unwrap(); + let module_name = crate_module.kind.name().unwrap_or(kw::Empty); let import_snippet = match import.kind { ImportKind::Single { source, target, .. } if source != target => { format!("{source} as {target}") diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index ff31af0025b5..d5fd03e5f412 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -503,18 +503,18 @@ enum ModuleKind { /// /// * A normal module – either `mod from_file;` or `mod from_block { }` – /// or the crate root (which is conceptually a top-level module). - /// Note that the crate root's [name][Self::name] will be [`kw::Empty`]. + /// The crate root will have `None` for the symbol. /// * A trait or an enum (it implicitly contains associated types, methods and variant /// constructors). - Def(DefKind, DefId, Symbol), + Def(DefKind, DefId, Option), } impl ModuleKind { /// Get name of the module. fn name(&self) -> Option { - match self { + match *self { ModuleKind::Block => None, - ModuleKind::Def(.., name) => Some(*name), + ModuleKind::Def(.., name) => name, } } } @@ -1402,7 +1402,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut module_self_bindings = FxHashMap::default(); let graph_root = arenas.new_module( None, - ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), + ModuleKind::Def(DefKind::Mod, root_def_id, None), ExpnId::root(), crate_span, attr::contains_name(attrs, sym::no_implicit_prelude), @@ -1411,7 +1411,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); let empty_module = arenas.new_module( None, - ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), + ModuleKind::Def(DefKind::Mod, root_def_id, None), ExpnId::root(), DUMMY_SP, true, @@ -2286,7 +2286,8 @@ fn module_to_string(mut module: Module<'_>) -> Option { loop { if let ModuleKind::Def(.., name) = module.kind { if let Some(parent) = module.parent { - names.push(name); + // `unwrap` is safe: the presence of a parent means it's not the crate root. + names.push(name.unwrap()); module = parent } else { break; diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index c4304a7a6df6..967cb5a0ec62 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -168,7 +168,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { hygiene::update_dollar_crate_names(|ctxt| { let ident = Ident::new(kw::DollarCrate, DUMMY_SP.with_ctxt(ctxt)); match self.resolve_crate_root(ident).kind { - ModuleKind::Def(.., name) if name != kw::Empty => name, + ModuleKind::Def(.., name) if let Some(name) = name => name, _ => kw::Crate, } }); @@ -1068,11 +1068,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); if fallback_binding.ok().and_then(|b| b.res().opt_def_id()) != Some(def_id) { let location = match parent_scope.module.kind { - ModuleKind::Def(_, _, name) if name == kw::Empty => { - "the crate root".to_string() - } ModuleKind::Def(kind, def_id, name) => { - format!("{} `{name}`", kind.descr(def_id)) + if let Some(name) = name { + format!("{} `{name}`", kind.descr(def_id)) + } else { + "the crate root".to_string() + } } ModuleKind::Block => "this scope".to_string(), }; From e576d8850de702f6c712eaca77f758f72ef13419 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 11:10:45 +1100 Subject: [PATCH 5/7] Use `Option` in `DuplicateLangItem`. For the the symbols that might not be present, instead of `kw::Empty`. --- compiler/rustc_passes/src/errors.rs | 20 +++++++++++++------- compiler/rustc_passes/src/lang_items.rs | 14 +++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index a72f40cd843a..0ee17430aab8 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1348,12 +1348,12 @@ pub(crate) struct DuplicateLangItem { pub local_span: Option, pub lang_item_name: Symbol, pub crate_name: Symbol, - pub dependency_of: Symbol, + pub dependency_of: Option, pub is_local: bool, pub path: String, pub first_defined_span: Option, - pub orig_crate_name: Symbol, - pub orig_dependency_of: Symbol, + pub orig_crate_name: Option, + pub orig_dependency_of: Option, pub orig_is_local: bool, pub orig_path: String, pub(crate) duplicate: Duplicate, @@ -1374,10 +1374,16 @@ impl Diagnostic<'_, G> for DuplicateLangItem { diag.code(E0152); diag.arg("lang_item_name", self.lang_item_name); diag.arg("crate_name", self.crate_name); - diag.arg("dependency_of", self.dependency_of); + if let Some(dependency_of) = self.dependency_of { + diag.arg("dependency_of", dependency_of); + } diag.arg("path", self.path); - diag.arg("orig_crate_name", self.orig_crate_name); - diag.arg("orig_dependency_of", self.orig_dependency_of); + if let Some(orig_crate_name) = self.orig_crate_name { + diag.arg("orig_crate_name", orig_crate_name); + } + if let Some(orig_dependency_of) = self.orig_dependency_of { + diag.arg("orig_dependency_of", orig_dependency_of); + } diag.arg("orig_path", self.orig_path); if let Some(span) = self.local_span { diag.span(span); @@ -1385,7 +1391,7 @@ impl Diagnostic<'_, G> for DuplicateLangItem { if let Some(span) = self.first_defined_span { diag.span_note(span, fluent::passes_first_defined_span); } else { - if self.orig_dependency_of.is_empty() { + if self.orig_dependency_of.is_none() { diag.note(fluent::passes_first_defined_crate); } else { diag.note(fluent::passes_first_defined_crate_depends); diff --git a/compiler/rustc_passes/src/lang_items.rs b/compiler/rustc_passes/src/lang_items.rs index fe1140d893e3..9d4b46cd3066 100644 --- a/compiler/rustc_passes/src/lang_items.rs +++ b/compiler/rustc_passes/src/lang_items.rs @@ -16,7 +16,7 @@ use rustc_hir::{LangItem, LanguageItems, MethodKind, Target}; use rustc_middle::query::Providers; use rustc_middle::ty::{ResolverAstLowering, TyCtxt}; use rustc_session::cstore::ExternCrate; -use rustc_span::{Span, kw}; +use rustc_span::Span; use crate::errors::{ DuplicateLangItem, IncorrectTarget, LangItemOnIncorrectTarget, UnknownLangItem, @@ -98,7 +98,7 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { { let lang_item_name = lang_item.name(); let crate_name = self.tcx.crate_name(item_def_id.krate); - let mut dependency_of = kw::Empty; + let mut dependency_of = None; let is_local = item_def_id.is_local(); let path = if is_local { String::new() @@ -112,8 +112,8 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { }; let first_defined_span = self.item_spans.get(&original_def_id).copied(); - let mut orig_crate_name = kw::Empty; - let mut orig_dependency_of = kw::Empty; + let mut orig_crate_name = None; + let mut orig_dependency_of = None; let orig_is_local = original_def_id.is_local(); let orig_path = if orig_is_local { String::new() @@ -127,11 +127,11 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { }; if first_defined_span.is_none() { - orig_crate_name = self.tcx.crate_name(original_def_id.krate); + orig_crate_name = Some(self.tcx.crate_name(original_def_id.krate)); if let Some(ExternCrate { dependency_of: inner_dependency_of, .. }) = self.tcx.extern_crate(original_def_id.krate) { - orig_dependency_of = self.tcx.crate_name(*inner_dependency_of); + orig_dependency_of = Some(self.tcx.crate_name(*inner_dependency_of)); } } @@ -140,7 +140,7 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { } else { match self.tcx.extern_crate(item_def_id.krate) { Some(ExternCrate { dependency_of: inner_dependency_of, .. }) => { - dependency_of = self.tcx.crate_name(*inner_dependency_of); + dependency_of = Some(self.tcx.crate_name(*inner_dependency_of)); Duplicate::CrateDepends } _ => Duplicate::Crate, From 867da30cc71017597d9ed731b03a45c79accd873 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 15:50:31 +1100 Subject: [PATCH 6/7] Avoid `kw::Empty` when dealing with `rustc_allowed_through_unstable_modules`. The existing code produces `Some(kw::Empty)` for these invalid forms: - a non-name-value, e.g. `#[rustc_allowed_through_unstable_modules]` - a non-string arg, e.g. `#[rustc_allowed_through_unstable_modules = 3]` The new code avoids the `kw::Empty` and is a little shorter. It will produce `None` in those cases, which means E0789 won't be produced if the `stable` attribute is missing for these invalid forms. This doesn't matter, because these invalid forms will trigger an "malformed `rustc_allowed_through_unstable_modules` attribute" anyway. --- compiler/rustc_attr_parsing/src/attributes/stability.rs | 8 ++------ compiler/rustc_error_codes/src/error_codes/E0789.md | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/stability.rs b/compiler/rustc_attr_parsing/src/attributes/stability.rs index 6d76456e83c8..bdad6b50186d 100644 --- a/compiler/rustc_attr_parsing/src/attributes/stability.rs +++ b/compiler/rustc_attr_parsing/src/attributes/stability.rs @@ -5,7 +5,7 @@ use rustc_attr_data_structures::{ StableSince, UnstableReason, VERSION_PLACEHOLDER, }; use rustc_errors::ErrorGuaranteed; -use rustc_span::{Span, Symbol, kw, sym}; +use rustc_span::{Span, Symbol, sym}; use super::util::parse_version; use super::{AcceptMapping, AttributeParser, SingleAttributeParser}; @@ -61,11 +61,7 @@ impl AttributeParser for StabilityParser { }), (&[sym::rustc_allowed_through_unstable_modules], |this, cx, args| { reject_outside_std!(cx); - this.allowed_through_unstable_modules = - Some(match args.name_value().and_then(|i| i.value_as_str()) { - Some(msg) => msg, - None => kw::Empty, - }); + this.allowed_through_unstable_modules = args.name_value().and_then(|i| i.value_as_str()) }), ]; diff --git a/compiler/rustc_error_codes/src/error_codes/E0789.md b/compiler/rustc_error_codes/src/error_codes/E0789.md index 2c0018cc799f..c7bc6cfde513 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0789.md +++ b/compiler/rustc_error_codes/src/error_codes/E0789.md @@ -14,7 +14,7 @@ Erroneous code example: #![unstable(feature = "foo_module", reason = "...", issue = "123")] -#[rustc_allowed_through_unstable_modules] +#[rustc_allowed_through_unstable_modules = "deprecation message"] // #[stable(feature = "foo", since = "1.0")] struct Foo; // ^^^ error: `rustc_allowed_through_unstable_modules` attribute must be From 501945a22e314bad511e08a187c0aa029df51038 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 18:00:14 +1100 Subject: [PATCH 7/7] Use `sym::dummy` for a dummy arg in `parse_fn_params`. --- compiler/rustc_parse/src/parser/item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index f4df4044dd2e..123234fedceb 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2871,7 +2871,7 @@ impl<'a> Parser<'a> { // Skip every token until next possible arg or end. p.eat_to_tokens(&[exp!(Comma), exp!(CloseParen)]); // Create a placeholder argument for proper arg count (issue #34264). - Ok(dummy_arg(Ident::new(kw::Empty, lo.to(p.prev_token.span)), guar)) + Ok(dummy_arg(Ident::new(sym::dummy, lo.to(p.prev_token.span)), guar)) }); // ...now that we've parsed the first argument, `self` is no longer allowed. first_param = false;