Auto merge of #154877 - JonathanBrouwer:rollup-rbarkwZ, r=JonathanBrouwer

Rollup of 3 pull requests

Successful merges:

 - rust-lang/rust#154554 (emit error on `#[track_caller]` with `extern fn`)
 - rust-lang/rust#154858 (Change api of formatting diagnostic attribute strings.)
 - rust-lang/rust#154876 (attr parsing: make sure we pass the right target when errors could be emitted)

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -1683,19 +1683,28 @@ fn visit_fn(&mut self, fk: FnKind<'a>, attrs: &AttrVec, span: Span, id: NodeId)
             self.check_item_safety(span, safety);
         }
 
-        if let FnKind::Fn(ctxt, _, fun) = fk
-            && let Extern::Explicit(str_lit, extern_abi_span) = fun.sig.header.ext
-            && let Ok(abi) = ExternAbi::from_str(str_lit.symbol.as_str())
-        {
-            self.check_extern_fn_signature(abi, ctxt, &fun.ident, &fun.sig);
+        if let FnKind::Fn(ctxt, _, fun) = fk {
+            let ext = match fun.sig.header.ext {
+                Extern::None => None,
+                Extern::Implicit(span) => Some((ExternAbi::FALLBACK, span)),
+                Extern::Explicit(str_lit, span) => {
+                    ExternAbi::from_str(str_lit.symbol.as_str()).ok().map(|abi| (abi, span))
+                }
+            };
 
-            if let Some(attr) = attr::find_by_name(attrs, sym::track_caller)
-                && abi != ExternAbi::Rust
-            {
-                self.dcx().emit_err(errors::RequiresRustAbi {
-                    track_caller_span: attr.span,
-                    extern_abi_span,
-                });
+            if let Some((extern_abi, extern_abi_span)) = ext {
+                // Some ABIs impose special restrictions on the signature.
+                self.check_extern_fn_signature(extern_abi, ctxt, &fun.ident, &fun.sig);
+
+                // #[track_caller] can only be used with the rust ABI.
+                if let Some(attr) = attr::find_by_name(attrs, sym::track_caller)
+                    && extern_abi != ExternAbi::Rust
+                {
+                    self.dcx().emit_err(errors::RequiresRustAbi {
+                        track_caller_span: attr.span,
+                        extern_abi_span,
+                    });
+                }
             }
         }
 

compiler/rustc_attr_parsing/src/interface.rs

@@ -64,6 +64,7 @@ pub fn parse_limited(
             sym,
             target_span,
             target_node_id,
+            Target::Crate, // Does not matter, we're not going to emit errors anyways
             features,
             ShouldEmit::Nothing,
         )
@@ -79,6 +80,7 @@ pub fn parse_limited_should_emit(
         sym: Symbol,
         target_span: Span,
         target_node_id: NodeId,
+        target: Target,
         features: Option<&'sess Features>,
         should_emit: ShouldEmit,
     ) -> Option<Attribute> {
@@ -86,7 +88,7 @@ pub fn parse_limited_should_emit(
             sess,
             attrs,
             Some(sym),
-            Target::Crate, // Does not matter, we're not going to emit errors anyways
+            target,
             target_span,
             target_node_id,
             features,

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -6,11 +6,10 @@
 use either::Either;
 use hir::{ClosureKind, Path};
 use rustc_data_structures::fx::FxIndexSet;
-use rustc_data_structures::thin_vec::ThinVec;
 use rustc_errors::codes::*;
 use rustc_errors::{Applicability, Diag, MultiSpan, struct_span_code_err};
 use rustc_hir as hir;
-use rustc_hir::attrs::diagnostic::FormatArgs;
+use rustc_hir::attrs::diagnostic::{CustomDiagnostic, FormatArgs};
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::intravisit::{Visitor, walk_block, walk_expr};
 use rustc_hir::{
@@ -146,7 +145,7 @@ pub(crate) fn report_use_of_moved_or_uninitialized(
                 self.body.local_decls[moved_place.local].ty.kind()
                 && let Some(Some(directive)) = find_attr!(self.infcx.tcx, item_def.did(), OnMove { directive, .. }  => directive)
             {
-                let item_name = self.infcx.tcx.item_name(item_def.did()).to_string();
+                let this = self.infcx.tcx.item_name(item_def.did()).to_string();
                 let mut generic_args: Vec<_> = self
                     .infcx
                     .tcx
@@ -155,21 +154,22 @@ pub(crate) fn report_use_of_moved_or_uninitialized(
                     .iter()
                     .filter_map(|param| Some((param.name, args[param.index as usize].to_string())))
                     .collect();
-                generic_args.push((kw::SelfUpper, item_name));
+                generic_args.push((kw::SelfUpper, this.clone()));
 
                 let args = FormatArgs {
-                    this: String::new(),
-                    trait_sugared: String::new(),
+                    this,
+                    // Unused
+                    this_sugared: String::new(),
+                    // Unused
                     item_context: "",
                     generic_args,
                 };
-                (
-                    directive.message.as_ref().map(|e| e.1.format(&args)),
-                    directive.label.as_ref().map(|e| e.1.format(&args)),
-                    directive.notes.iter().map(|e| e.format(&args)).collect(),
-                )
+                let CustomDiagnostic { message, label, notes, parent_label: _ } =
+                    directive.eval(None, &args);
+
+                (message, label, notes)
             } else {
-                (None, None, ThinVec::new())
+                (None, None, Vec::new())
             };
 
             let mut err = self.cannot_act_on_moved_value(

compiler/rustc_hir/src/attrs/diagnostic.rs

@@ -52,30 +52,40 @@ pub fn visit_params(&self, visit: &mut impl FnMut(Symbol, Span)) {
         }
     }
 
-    pub fn evaluate_directive(
+    pub fn eval(
         &self,
-        trait_name: impl Debug,
-        condition_options: &ConditionOptions,
+        condition_options: Option<&ConditionOptions>,
         args: &FormatArgs,
-    ) -> OnUnimplementedNote {
+    ) -> CustomDiagnostic {
+        let this = &args.this;
+        info!("eval({self:?}, this={this}, options={condition_options:?}, args ={args:?})");
+
+        let Some(condition_options) = condition_options else {
+            debug_assert!(
+                !self.is_rustc_attr,
+                "Directive::eval called for `rustc_on_unimplemented` without `condition_options`"
+            );
+            return CustomDiagnostic {
+                label: self.label.as_ref().map(|l| l.1.format(args)),
+                message: self.message.as_ref().map(|m| m.1.format(args)),
+                notes: self.notes.iter().map(|n| n.format(args)).collect(),
+                parent_label: None,
+            };
+        };
         let mut message = None;
         let mut label = None;
         let mut notes = Vec::new();
         let mut parent_label = None;
-        info!(
-            "evaluate_directive({:?}, trait_ref={:?}, options={:?}, args ={:?})",
-            self, trait_name, condition_options, args
-        );
 
         for command in self.subcommands.iter().chain(Some(self)).rev() {
             debug!(?command);
             if let Some(ref condition) = command.condition
                 && !condition.matches_predicate(condition_options)
             {
-                debug!("evaluate_directive: skipping {:?} due to condition", command);
+                debug!("eval: skipping {command:?} due to condition");
                 continue;
             }
-            debug!("evaluate_directive: {:?} succeeded", command);
+            debug!("eval: {command:?} succeeded");
             if let Some(ref message_) = command.message {
                 message = Some(message_.clone());
             }
@@ -91,7 +101,7 @@ pub fn evaluate_directive(
             }
         }
 
-        OnUnimplementedNote {
+        CustomDiagnostic {
             label: label.map(|l| l.1.format(args)),
             message: message.map(|m| m.1.format(args)),
             notes: notes.into_iter().map(|n| n.format(args)).collect(),
@@ -100,8 +110,9 @@ pub fn evaluate_directive(
     }
 }
 
+/// A custom diagnostic, created from a diagnostic attribute.
 #[derive(Default, Debug)]
-pub struct OnUnimplementedNote {
+pub struct CustomDiagnostic {
     pub message: Option<String>,
     pub label: Option<String>,
     pub notes: Vec<String>,
@@ -116,8 +127,13 @@ pub struct FormatString {
     pub span: Span,
     pub pieces: ThinVec<Piece>,
 }
+
 impl FormatString {
-    pub fn format(&self, args: &FormatArgs) -> String {
+    /// Formats the format string.
+    ///
+    /// This is a private method, use `Directive::eval` instead. A diagnostic attribute being used
+    /// should issue a `tracing` event, which `Directive::eval` does.
+    fn format(&self, args: &FormatArgs) -> String {
         let mut ret = String::new();
         for piece in &self.pieces {
             match piece {
@@ -147,7 +163,7 @@ pub fn format(&self, args: &FormatArgs) -> String {
                 // It's only `rustc_onunimplemented` from here
                 Piece::Arg(FormatArg::This) => ret.push_str(&args.this),
                 Piece::Arg(FormatArg::Trait) => {
-                    let _ = fmt::write(&mut ret, format_args!("{}", &args.trait_sugared));
+                    let _ = fmt::write(&mut ret, format_args!("{}", &args.this_sugared));
                 }
                 Piece::Arg(FormatArg::ItemContext) => ret.push_str(args.item_context),
             }
@@ -193,7 +209,7 @@ fn visit_params(&self, visit: &mut impl FnMut(Symbol, Span)) {
 /// ```rust,ignore (just an example)
 /// FormatArgs {
 ///     this: "FromResidual",
-///     trait_sugared: "FromResidual<Option<Infallible>>",
+///     this_sugared: "FromResidual<Option<Infallible>>",
 ///     item_context: "an async function",
 ///     generic_args: [("Self", "u32"), ("R", "Option<Infallible>")],
 /// }
@@ -201,7 +217,7 @@ fn visit_params(&self, visit: &mut impl FnMut(Symbol, Span)) {
 #[derive(Debug)]
 pub struct FormatArgs {
     pub this: String,
-    pub trait_sugared: String,
+    pub this_sugared: String,
     pub item_context: &'static str,
     pub generic_args: Vec<(Symbol, String)>,
 }

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -16,7 +16,7 @@
 use rustc_errors::{
     Applicability, Diag, MultiSpan, StashKey, StringPart, listify, pluralize, struct_span_code_err,
 };
-use rustc_hir::attrs::diagnostic::OnUnimplementedNote;
+use rustc_hir::attrs::diagnostic::CustomDiagnostic;
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{self, Visitor};
@@ -2067,7 +2067,7 @@ fn handle_unsatisfied_predicates(
                             // Avoid crashing.
                             return (None, None, Vec::new());
                         }
-                        let OnUnimplementedNote { message, label, notes, .. } = self
+                        let CustomDiagnostic { message, label, notes, .. } = self
                             .err_ctxt()
                             .on_unimplemented_note(trait_ref, &obligation, err.long_ty_path());
                         (message, label, notes)

compiler/rustc_interface/src/passes.rs

@@ -22,7 +22,7 @@
 use rustc_hir::definitions::Definitions;
 use rustc_hir::limit::Limit;
 use rustc_hir::lints::DelayedLint;
-use rustc_hir::{Attribute, MaybeOwner, find_attr};
+use rustc_hir::{Attribute, MaybeOwner, Target, find_attr};
 use rustc_incremental::setup_dep_graph;
 use rustc_lint::{
     BufferedEarlyLint, DecorateAttrLint, EarlyCheckNode, LintStore, unerased_lint_store,
@@ -1372,6 +1372,7 @@ pub(crate) fn parse_crate_name(
             sym::crate_name,
             DUMMY_SP,
             rustc_ast::node_id::CRATE_NODE_ID,
+            Target::Crate,
             None,
             emit_errors,
         )?
@@ -1421,6 +1422,7 @@ pub fn collect_crate_types(
                 sym::crate_type,
                 crate_span,
                 CRATE_NODE_ID,
+                Target::Crate,
                 None,
                 ShouldEmit::EarlyFatal { also_emit_lints: false },
             )
@@ -1477,6 +1479,7 @@ fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit
         sym::recursion_limit,
         DUMMY_SP,
         rustc_ast::node_id::CRATE_NODE_ID,
+        Target::Crate,
         None,
         // errors are fatal here, but lints aren't.
         // If things aren't fatal we continue, and will parse this again.

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

@@ -14,7 +14,7 @@
     Applicability, Diag, ErrorGuaranteed, Level, MultiSpan, StashKey, StringPart, Suggestions, msg,
     pluralize, struct_span_code_err,
 };
-use rustc_hir::attrs::diagnostic::OnUnimplementedNote;
+use rustc_hir::attrs::diagnostic::CustomDiagnostic;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::{self as hir, LangItem, Node, find_attr};
@@ -188,7 +188,7 @@ pub fn report_selection_error(
                             })
                             .unwrap_or_default();
 
-                        let OnUnimplementedNote {
+                        let CustomDiagnostic {
                             message,
                             label,
                             notes,
@@ -907,12 +907,11 @@ fn report_host_effect_error(
                     );
 
                     if let Some(command) = find_attr!(self.tcx, impl_did, OnConst {directive, ..} => directive.as_deref()).flatten(){
-                        let note = command.evaluate_directive(
-                             predicate.skip_binder().trait_ref,
-                            &condition_options,
+                        let note = command.eval(
+                            Some(&condition_options),
                             &format_args,
                         );
-                        let OnUnimplementedNote {
+                        let CustomDiagnostic {
                             message,
                             label,
                             notes,

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs

@@ -1,7 +1,7 @@
 use std::path::PathBuf;
 
 use rustc_hir as hir;
-use rustc_hir::attrs::diagnostic::{ConditionOptions, FormatArgs, OnUnimplementedNote};
+use rustc_hir::attrs::diagnostic::{ConditionOptions, CustomDiagnostic, FormatArgs};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::find_attr;
 pub use rustc_hir::lints::FormatWarning;
@@ -37,20 +37,19 @@ pub fn on_unimplemented_note(
         trait_pred: ty::PolyTraitPredicate<'tcx>,
         obligation: &PredicateObligation<'tcx>,
         long_ty_path: &mut Option<PathBuf>,
-    ) -> OnUnimplementedNote {
+    ) -> CustomDiagnostic {
         if trait_pred.polarity() != ty::PredicatePolarity::Positive {
-            return OnUnimplementedNote::default();
+            return CustomDiagnostic::default();
         }
         let (condition_options, format_args) =
             self.on_unimplemented_components(trait_pred, obligation, long_ty_path);
         if let Some(command) = find_attr!(self.tcx, trait_pred.def_id(), OnUnimplemented {directive, ..} => directive.as_deref()).flatten() {
-            command.evaluate_directive(
-                trait_pred.skip_binder().trait_ref,
-                &condition_options,
+            command.eval(
+                Some(&condition_options),
                 &format_args,
             )
         } else {
-            OnUnimplementedNote::default()
+            CustomDiagnostic::default()
         }
     }
 
@@ -211,7 +210,7 @@ pub(crate) fn on_unimplemented_components(
         }));
 
         let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id);
-        let trait_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();
+        let this_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();
 
         let condition_options = ConditionOptions {
             self_types,
@@ -249,7 +248,7 @@ pub(crate) fn on_unimplemented_components(
             })
             .collect();
 
-        let format_args = FormatArgs { this, trait_sugared, generic_args, item_context };
+        let format_args = FormatArgs { this, this_sugared, generic_args, item_context };
         (condition_options, format_args)
     }
 }

tests/ui/rfcs/rfc-2091-track-caller/error-with-invalid-abi.rs

@@ -27,4 +27,10 @@ extern "C" fn c_method() {}
     extern "Rust" fn rust_method() {}
 }
 
+#[rustfmt::skip] // rustfmt will insert the implicit "C"
+#[track_caller]
+//~^ ERROR `#[track_caller]` can only be used with the Rust ABI
+pub extern fn extern_missing_abi() {}
+//~^ WARN `extern` declarations without an explicit ABI are deprecated
+
 fn main() {}

tests/ui/rfcs/rfc-2091-track-caller/error-with-invalid-abi.stderr

@@ -24,6 +24,23 @@ LL |
 LL |     extern "C" fn c_method() {}
    |     ---------- not using the Rust ABI because of this
 
-error: aborting due to 3 previous errors
+error[E0737]: `#[track_caller]` can only be used with the Rust ABI
+  --> $DIR/error-with-invalid-abi.rs:31:1
+   |
+LL | #[track_caller]
+   | ^^^^^^^^^^^^^^^ using `#[track_caller]` here
+LL |
+LL | pub extern fn extern_missing_abi() {}
+   |     ------ not using the Rust ABI because of this
+
+warning: `extern` declarations without an explicit ABI are deprecated
+  --> $DIR/error-with-invalid-abi.rs:33:5
+   |
+LL | pub extern fn extern_missing_abi() {}
+   |     ^^^^^^ help: explicitly specify the "C" ABI: `extern "C"`
+   |
+   = note: `#[warn(missing_abi)]` on by default
+
+error: aborting due to 4 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0737`.