[FOLLOW UP] [BLOCKED] Fix crate name validation follow up

compiler/rustc_attr/src/builtin.rs

@@ -503,10 +503,6 @@ fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabil
     }
 }
 
-pub fn find_crate_name(attrs: &[Attribute]) -> Option<Symbol> {
-    attr::first_attr_value_str_by_name(attrs, sym::crate_name)
-}
-
 #[derive(Clone, Debug)]
 pub struct Condition {
     pub name: Symbol,

compiler/rustc_driver_impl/src/lib.rs

@@ -759,12 +759,17 @@ fn print_crate_info(
                     // no crate attributes, print out an error and exit
                     return Compilation::Continue;
                 };
-                let t_outputs = rustc_interface::util::build_output_filenames(attrs, sess);
-                let id = rustc_session::output::find_crate_name(sess, attrs);
+                let (crate_name, crate_name_origin) = passes::get_crate_name(sess, attrs);
+                let t_outputs = rustc_interface::util::build_output_filenames(
+                    sess,
+                    crate_name,
+                    crate_name_origin,
+                );
                 let crate_types = collect_crate_types(sess, attrs);
                 for &style in &crate_types {
-                    let fname =
-                        rustc_session::output::filename_for_input(sess, style, id, &t_outputs);
+                    let fname = rustc_session::output::filename_for_input(
+                        sess, style, crate_name, &t_outputs,
+                    );
                     println_info!("{}", fname.as_path().file_name().unwrap().to_string_lossy());
                 }
             }
@@ -773,8 +778,8 @@ fn print_crate_info(
                     // no crate attributes, print out an error and exit
                     return Compilation::Continue;
                 };
-                let id = rustc_session::output::find_crate_name(sess, attrs);
-                println_info!("{id}");
+                let (crate_name, _) = passes::get_crate_name(sess, attrs);
+                println_info!("{crate_name}");
             }
             Cfg => {
                 let mut cfgs = sess

compiler/rustc_expand/src/module.rs

@@ -179,12 +179,12 @@ fn mod_file_path_from_attr(
     let first_path = attrs.iter().find(|at| at.has_name(sym::path))?;
     let Some(path_sym) = first_path.value_str() else {
         // This check is here mainly to catch attempting to use a macro,
-        // such as #[path = concat!(...)]. This isn't currently supported
+        // such as `#[path = concat!(...)]`. This isn't currently supported
         // because otherwise the InvocationCollector would need to defer
-        // loading a module until the #[path] attribute was expanded, and
-        // it doesn't support that (and would likely add a bit of
-        // complexity). Usually bad forms are checked in AstValidator (via
-        // `check_builtin_attribute`), but by the time that runs the macro
+        // loading a module until the `#[path]` attribute was expanded, and
+        // it doesn't support that (and would likely add a bit of complexity).
+        // Usually bad forms are checked during semantic analysis via
+        // `TyCtxt::check_mod_attrs`), but by the time that runs the macro
         // is expanded, and it doesn't give an error.
         validate_attr::emit_fatal_malformed_builtin_attribute(&sess.psess, first_path, sym::path);
     };

compiler/rustc_incremental/src/persist/fs.rs

@@ -116,8 +116,9 @@ use rustc_errors::ErrorGuaranteed;
 use rustc_fs_util::{link_or_copy, try_canonicalize, LinkOrCopy};
 use rustc_middle::bug;
 use rustc_session::config::CrateType;
-use rustc_session::output::{collect_crate_types, find_crate_name};
+use rustc_session::output::collect_crate_types;
 use rustc_session::{Session, StableCrateId};
+use rustc_span::Symbol;
 
 use std::fs as std_fs;
 use std::io::{self, ErrorKind};
@@ -215,7 +216,10 @@ pub fn in_incr_comp_dir(incr_comp_session_dir: &Path, file_name: &str) -> PathBu
 /// The garbage collection will take care of it.
 ///
 /// [`rustc_interface::queries::dep_graph`]: ../../rustc_interface/struct.Queries.html#structfield.dep_graph
-pub(crate) fn prepare_session_directory(sess: &Session) -> Result<(), ErrorGuaranteed> {
+pub(crate) fn prepare_session_directory(
+    sess: &Session,
+    crate_name: Symbol,
+) -> Result<(), ErrorGuaranteed> {
     if sess.opts.incremental.is_none() {
         return Ok(());
     }
@@ -225,7 +229,7 @@ pub(crate) fn prepare_session_directory(sess: &Session) -> Result<(), ErrorGuara
     debug!("prepare_session_directory");
 
     // {incr-comp-dir}/{crate-name-and-disambiguator}
-    let crate_dir = crate_path(sess);
+    let crate_dir = crate_path(sess, crate_name);
     debug!("crate-dir: {}", crate_dir.display());
     create_dir(sess, &crate_dir, "crate")?;
 
@@ -608,10 +612,9 @@ fn string_to_timestamp(s: &str) -> Result<SystemTime, &'static str> {
     Ok(UNIX_EPOCH + duration)
 }
 
-fn crate_path(sess: &Session) -> PathBuf {
+fn crate_path(sess: &Session, crate_name: Symbol) -> PathBuf {
     let incr_dir = sess.opts.incremental.as_ref().unwrap().clone();
 
-    let crate_name = find_crate_name(sess, &[]);
     let crate_types = collect_crate_types(sess, &[]);
     let stable_crate_id = StableCrateId::new(
         crate_name,

compiler/rustc_incremental/src/persist/load.rs

@@ -9,7 +9,7 @@ use rustc_serialize::opaque::MemDecoder;
 use rustc_serialize::Decodable;
 use rustc_session::config::IncrementalStateAssertion;
 use rustc_session::Session;
-use rustc_span::ErrorGuaranteed;
+use rustc_span::{ErrorGuaranteed, Symbol};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use tracing::{debug, warn};
@@ -204,9 +204,9 @@ pub fn load_query_result_cache(sess: &Session) -> Option<OnDiskCache<'_>> {
 
 /// Setups the dependency graph by loading an existing graph from disk and set up streaming of a
 /// new graph to an incremental session directory.
-pub fn setup_dep_graph(sess: &Session) -> Result<DepGraph, ErrorGuaranteed> {
+pub fn setup_dep_graph(sess: &Session, crate_name: Symbol) -> Result<DepGraph, ErrorGuaranteed> {
     // `load_dep_graph` can only be called after `prepare_session_directory`.
-    prepare_session_directory(sess)?;
+    prepare_session_directory(sess, crate_name)?;
 
     let res = sess.opts.build_dep_graph().then(|| load_dep_graph(sess));
 

compiler/rustc_interface/messages.ftl

@@ -1,6 +1,10 @@
 interface_cant_emit_mir =
     could not emit MIR: {$error}
 
+interface_crate_name_does_not_match = `--crate-name` and `#[crate_name]` are required to match, but `{$crate_name}` != `{$attr_crate_name}`
+
+interface_crate_name_invalid = crate names cannot start with a `-`, but `{$crate_name}` has a leading hyphen
+
 interface_emoji_identifier =
     identifiers cannot contain emoji: `{$ident}`
 

compiler/rustc_interface/src/errors.rs

@@ -4,6 +4,21 @@ use rustc_span::{Span, Symbol};
 use std::io;
 use std::path::Path;
 
+#[derive(Diagnostic)]
+#[diag(interface_crate_name_does_not_match)]
+pub(crate) struct CrateNameDoesNotMatch {
+    #[primary_span]
+    pub(crate) span: Span,
+    pub(crate) crate_name: Symbol,
+    pub(crate) attr_crate_name: Symbol,
+}
+
+#[derive(Diagnostic)]
+#[diag(interface_crate_name_invalid)]
+pub(crate) struct CrateNameInvalid<'a> {
+    pub(crate) crate_name: &'a str,
+}
+
 #[derive(Diagnostic)]
 #[diag(interface_ferris_identifier)]
 pub struct FerrisIdentifier {

compiler/rustc_interface/src/passes.rs

@@ -27,12 +27,12 @@ use rustc_resolve::Resolver;
 use rustc_session::code_stats::VTableSizeInfo;
 use rustc_session::config::{CrateType, Input, OutFileName, OutputFilenames, OutputType};
 use rustc_session::cstore::Untracked;
+use rustc_session::output::collect_crate_types;
 use rustc_session::output::filename_for_input;
-use rustc_session::output::{collect_crate_types, find_crate_name};
 use rustc_session::search_paths::PathKind;
 use rustc_session::{Limit, Session};
-use rustc_span::symbol::{sym, Symbol};
 use rustc_span::FileName;
+use rustc_span::{sym, Span, Symbol};
 use rustc_target::spec::PanicStrategy;
 use rustc_trait_selection::traits;
 
@@ -660,17 +660,17 @@ pub(crate) fn create_global_ctxt<'tcx>(
 
     let pre_configured_attrs = rustc_expand::config::pre_configure_attrs(sess, &krate.attrs);
 
-    // parse `#[crate_name]` even if `--crate-name` was passed, to make sure it matches.
-    let crate_name = find_crate_name(sess, &pre_configured_attrs);
+    let (crate_name, crate_name_origin) = get_crate_name(sess, &pre_configured_attrs);
     let crate_types = collect_crate_types(sess, &pre_configured_attrs);
     let stable_crate_id = StableCrateId::new(
         crate_name,
         crate_types.contains(&CrateType::Executable),
         sess.opts.cg.metadata.clone(),
         sess.cfg_version,
     );
-    let outputs = util::build_output_filenames(&pre_configured_attrs, sess);
-    let dep_graph = setup_dep_graph(sess)?;
+    let outputs = util::build_output_filenames(sess, crate_name, crate_name_origin);
+
+    let dep_graph = setup_dep_graph(sess, crate_name)?;
 
     let cstore =
         FreezeLock::new(Box::new(CStore::new(compiler.codegen_backend.metadata_loader())) as _);
@@ -1044,23 +1044,91 @@ pub(crate) fn start_codegen<'tcx>(
     Ok(codegen)
 }
 
-fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit {
-    if let Some(attr) = krate_attrs
-        .iter()
-        .find(|attr| attr.has_name(sym::recursion_limit) && attr.value_str().is_none())
+/// Compute and validate the crate name.
+pub fn get_crate_name(sess: &Session, krate_attrs: &[ast::Attribute]) -> (Symbol, CrateNameOrigin) {
+    // We unconditionally validate all `#![crate_name]`s even if a crate name was
+    // set on the command line via `--crate-name` which we prioritize over the
+    // crate attributes. We perform the validation here instead of later to ensure
+    // it gets run in all code paths requiring the crate name very early on.
+    // Namely, print requests (`--print`).
+    let attr_crate_name =
+        validate_and_find_value_str_builtin_attr(sym::crate_name, sess, krate_attrs);
+
+    let validate = |name, span, origin| {
+        rustc_session::output::validate_crate_name(sess, name, span);
+        (name, origin)
+    };
+
+    if let Some(crate_name) = &sess.opts.crate_name {
+        let crate_name = Symbol::intern(crate_name);
+        if let Some((attr_crate_name, span)) = attr_crate_name
+            && attr_crate_name != crate_name
+        {
+            sess.dcx().emit_err(errors::CrateNameDoesNotMatch {
+                span,
+                crate_name,
+                attr_crate_name,
+            });
+        }
+        return validate(crate_name, None, CrateNameOrigin::CliOpt);
+    }
+
+    if let Some((crate_name, span)) = attr_crate_name {
+        return validate(crate_name, Some(span), CrateNameOrigin::CrateAttr);
+    }
+
+    if let Input::File(ref path) = sess.io.input
+        && let Some(file_stem) = path.file_stem().and_then(|s| s.to_str())
     {
-        // This is here mainly to check for using a macro, such as
-        // #![recursion_limit = foo!()]. That is not supported since that
-        // would require expanding this while in the middle of expansion,
-        // which needs to know the limit before expanding. Otherwise,
-        // validation would normally be caught in AstValidator (via
-        // `check_builtin_attribute`), but by the time that runs the macro
-        // is expanded, and it doesn't give an error.
-        validate_attr::emit_fatal_malformed_builtin_attribute(
-            &sess.psess,
-            attr,
-            sym::recursion_limit,
-        );
+        if file_stem.starts_with('-') {
+            sess.dcx().emit_err(errors::CrateNameInvalid { crate_name: file_stem });
+        } else {
+            let crate_name = file_stem.replace('-', "_");
+            return validate(Symbol::intern(&crate_name), None, CrateNameOrigin::InputFile);
+        }
     }
+
+    (Symbol::intern("rust_out"), CrateNameOrigin::Fallback)
+}
+
+pub enum CrateNameOrigin {
+    CliOpt,
+    CrateAttr,
+    InputFile,
+    Fallback,
+}
+
+fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit {
+    // We don't permit macro calls inside of the attribute (e.g., #![recursion_limit = `expand!()`])
+    // because that would require expanding this while in the middle of expansion, which needs to
+    // know the limit before expanding.
+    let _ = validate_and_find_value_str_builtin_attr(sym::recursion_limit, sess, krate_attrs);
     rustc_middle::middle::limits::get_recursion_limit(krate_attrs, sess)
 }
+
+/// Validate *all* occurrences of the given "[value-str]" built-in attribute and return the first find.
+///
+/// This validator is intended for built-in attributes whose value needs to be known very early
+/// during compilation (namely, before macro expansion) and it mainly exists to reject macro calls
+/// inside of the attributes, such as in `#![name = expand!()]`. Normal attribute validation happens
+/// during semantic analysis via [`TyCtxt::check_mod_attrs`] which happens *after* macro expansion
+/// when such macro calls (here: `expand`) have already been expanded and we can no longer check for
+/// their presence.
+///
+/// [value-str]: ast::Attribute::value_str
+fn validate_and_find_value_str_builtin_attr(
+    name: Symbol,
+    sess: &Session,
+    krate_attrs: &[ast::Attribute],
+) -> Option<(Symbol, Span)> {
+    let mut result = None;
+    // Validate *all* relevant attributes, not just the first occurrence.
+    for attr in ast::attr::filter_by_name(krate_attrs, name) {
+        let Some(value) = attr.value_str() else {
+            validate_attr::emit_fatal_malformed_builtin_attribute(&sess.psess, attr, name)
+        };
+        // Choose the first occurrence as our result.
+        result.get_or_insert((value, attr.span));
+    }
+    result
+}