Auto merge of rust-lang#108421 - Dylan-DPC:rollup-mpeovxd, r=Dylan-DPC

Rollup of 10 pull requests

Successful merges:

 - rust-lang#106541 (implement const iterator using `rustc_do_not_const_check`)
 - rust-lang#106918 (Rebuild BinaryHeap on unwind from retain)
 - rust-lang#106923 (Restore behavior when primary bundle is missing)
 - rust-lang#108169 (Make query keys `Copy`)
 - rust-lang#108287 (Add test for bad cast with deferred projection equality)
 - rust-lang#108370 (std: time: Avoid to use "was created" in elapsed() description)
 - rust-lang#108377 (Fix ICE in 'duplicate diagnostic item' diagnostic)
 - rust-lang#108388 (parser: provide better suggestions and errors on closures with braces missing)
 - rust-lang#108391 (Fix `is_terminal`'s handling of long paths on Windows.)
 - rust-lang#108401 (diagnostics: remove inconsistent English article "this" from E0107)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_error_messages/src/lib.rs

@@ -135,7 +135,10 @@ pub fn fluent_bundle(
 
     let fallback_locale = langid!("en-US");
     let requested_fallback_locale = requested_locale.as_ref() == Some(&fallback_locale);
-
+    trace!(?requested_fallback_locale);
+    if requested_fallback_locale && additional_ftl_path.is_none() {
+        return Ok(None);
+    }
     // If there is only `-Z additional-ftl-path`, assume locale is "en-US", otherwise use user
     // provided locale.
     let locale = requested_locale.clone().unwrap_or(fallback_locale);
@@ -153,7 +156,7 @@ pub fn fluent_bundle(
     bundle.set_use_isolating(with_directionality_markers);
 
     // If the user requests the default locale then don't try to load anything.
-    if !requested_fallback_locale && let Some(requested_locale) = requested_locale {
+    if let Some(requested_locale) = requested_locale {
         let mut found_resources = false;
         for sysroot in user_provided_sysroot.iter_mut().chain(sysroot_candidates.iter_mut()) {
             sysroot.push("share");

compiler/rustc_errors/src/translation.rs

@@ -1,4 +1,4 @@
-use crate::error::TranslateError;
+use crate::error::{TranslateError, TranslateErrorKind};
 use crate::snippet::Style;
 use crate::{DiagnosticArg, DiagnosticMessage, FluentBundle};
 use rustc_data_structures::sync::Lrc;
@@ -95,6 +95,16 @@ pub trait Translate {
                 // The primary bundle was present and translation succeeded
                 Some(Ok(t)) => t,
 
+                // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely
+                // just that the primary bundle doesn't contain the message being translated, so
+                // proceed to the fallback bundle.
+                Some(Err(
+                    primary @ TranslateError::One {
+                        kind: TranslateErrorKind::MessageMissing, ..
+                    },
+                )) => translate_with_bundle(self.fallback_fluent_bundle())
+                    .map_err(|fallback| primary.and(fallback))?,
+
                 // Always yeet out for errors on debug (unless
                 // `RUSTC_TRANSLATION_NO_DEBUG_ASSERT` is set in the environment - this allows
                 // local runs of the test suites, of builds with debug assertions, to test the
@@ -106,9 +116,8 @@ pub trait Translate {
                     do yeet primary
                 }
 
-                // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely
-                // just that the primary bundle doesn't contain the message being translated or
-                // something else went wrong) so proceed to the fallback bundle.
+                // ..otherwise, for end users, an error about this wouldn't be useful or actionable, so
+                // just hide it and try with the fallback bundle.
                 Some(Err(primary)) => translate_with_bundle(self.fallback_fluent_bundle())
                     .map_err(|fallback| primary.and(fallback))?,
 

compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs

@@ -462,7 +462,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
 
         if self.gen_args.span_ext().is_some() {
             format!(
-                "this {} takes {}{} {} argument{} but {} {} supplied",
+                "{} takes {}{} {} argument{} but {} {} supplied",
                 def_kind,
                 quantifier,
                 bound,

compiler/rustc_hir_typeck/src/lib.rs

@@ -71,7 +71,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_session::config;
 use rustc_session::Session;
 use rustc_span::def_id::{DefId, LocalDefId};
-use rustc_span::Span;
+use rustc_span::{sym, Span};
 
 fluent_messages! { "../locales/en-US.ftl" }
 
@@ -207,6 +207,11 @@ fn typeck_with_fallback<'tcx>(
 
     let typeck_results = Inherited::build(tcx, def_id).enter(|inh| {
         let param_env = tcx.param_env(def_id);
+        let param_env = if tcx.has_attr(def_id.to_def_id(), sym::rustc_do_not_const_check) {
+            param_env.without_const()
+        } else {
+            param_env
+        };
         let mut fcx = FnCtxt::new(&inh, param_env, def_id);
 
         if let Some(hir::FnSig { header, decl, .. }) = fn_sig {

compiler/rustc_parse/src/parser/mod.rs

@@ -982,7 +982,11 @@ impl<'a> Parser<'a> {
         let initial_semicolon = self.token.span;
 
         while self.eat(&TokenKind::Semi) {
-            let _ = self.parse_stmt(ForceCollect::Yes)?;
+            let _ =
+                self.parse_stmt_without_recovery(false, ForceCollect::Yes).unwrap_or_else(|e| {
+                    e.cancel();
+                    None
+                });
         }
 
         expect_err.set_primary_message(

compiler/rustc_passes/locales/en-US.ftl

@@ -407,10 +407,10 @@ passes_duplicate_diagnostic_item =
 
 passes_duplicate_diagnostic_item_in_crate =
     duplicate diagnostic item in crate `{$crate_name}`: `{$name}`.
+    .note = the diagnostic item is first defined in crate `{$orig_crate_name}`.
 
 passes_diagnostic_item_first_defined =
     the diagnostic item is first defined here
-    .note = the diagnostic item is first defined in crate `{$orig_crate_name}`.
 
 passes_abi =
     abi: {$abi}

compiler/rustc_query_system/src/query/caches.rs

@@ -21,7 +21,7 @@ pub trait QueryStorage {
 }
 
 pub trait QueryCache: QueryStorage + Sized {
-    type Key: Hash + Eq + Clone + Debug;
+    type Key: Hash + Eq + Copy + Debug;
 
     /// Checks if the query is already computed and in the cache.
     /// It returns the shard index and a lock guard to the shard,
@@ -61,7 +61,7 @@ impl<K: Eq + Hash, V: Copy + Debug> QueryStorage for DefaultCache<K, V> {
 
 impl<K, V> QueryCache for DefaultCache<K, V>
 where
-    K: Eq + Hash + Clone + Debug,
+    K: Eq + Hash + Copy + Debug,
     V: Copy + Debug,
 {
     type Key = K;
@@ -179,7 +179,7 @@ impl<K: Eq + Idx, V: Copy + Debug> QueryStorage for VecCache<K, V> {
 
 impl<K, V> QueryCache for VecCache<K, V>
 where
-    K: Eq + Idx + Clone + Debug,
+    K: Eq + Idx + Copy + Debug,
     V: Copy + Debug,
 {
     type Key = K;

compiler/rustc_query_system/src/query/config.rs

@@ -19,7 +19,9 @@ pub type TryLoadFromDisk<Qcx, Q> =
 pub trait QueryConfig<Qcx: QueryContext> {
     const NAME: &'static str;
 
-    type Key: DepNodeParams<Qcx::DepContext> + Eq + Hash + Clone + Debug;
+    // `Key` and `Value` are `Copy` instead of `Clone` to ensure copying them stays cheap,
+    // but it isn't necessary.
+    type Key: DepNodeParams<Qcx::DepContext> + Eq + Hash + Copy + Debug;
     type Value: Debug + Copy;
 
     type Cache: QueryCache<Key = Self::Key, Value = Self::Value>;

compiler/rustc_query_system/src/query/plumbing.rs

@@ -48,7 +48,7 @@ enum QueryResult<D: DepKind> {
 
 impl<K, D> QueryState<K, D>
 where
-    K: Eq + Hash + Clone + Debug,
+    K: Eq + Hash + Copy + Debug,
     D: DepKind,
 {
     pub fn all_inactive(&self) -> bool {
@@ -77,7 +77,7 @@ where
             for shard in shards.iter() {
                 for (k, v) in shard.iter() {
                     if let QueryResult::Started(ref job) = *v {
-                        let query = make_query(qcx, k.clone());
+                        let query = make_query(qcx, *k);
                         jobs.insert(job.id, QueryJobInfo { query, job: job.clone() });
                     }
                 }
@@ -91,7 +91,7 @@ where
             // really hurt much.)
             for (k, v) in self.active.try_lock()?.iter() {
                 if let QueryResult::Started(ref job) = *v {
-                    let query = make_query(qcx, k.clone());
+                    let query = make_query(qcx, *k);
                     jobs.insert(job.id, QueryJobInfo { query, job: job.clone() });
                 }
             }
@@ -111,7 +111,7 @@ impl<K, D: DepKind> Default for QueryState<K, D> {
 /// This will poison the relevant query if dropped.
 struct JobOwner<'tcx, K, D: DepKind>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + Hash + Copy,
 {
     state: &'tcx QueryState<K, D>,
     key: K,
@@ -163,7 +163,7 @@ where
 
 impl<'tcx, K, D: DepKind> JobOwner<'tcx, K, D>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + Hash + Copy,
 {
     /// Either gets a `JobOwner` corresponding the query, allowing us to
     /// start executing the query, or returns with the result of the query.
@@ -195,7 +195,7 @@ where
                 let job = qcx.current_query_job();
                 let job = QueryJob::new(id, span, job);
 
-                let key = entry.key().clone();
+                let key = *entry.key();
                 entry.insert(QueryResult::Started(job));
 
                 let owner = JobOwner { state, id, key };
@@ -274,7 +274,7 @@ where
 
 impl<'tcx, K, D> Drop for JobOwner<'tcx, K, D>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + Hash + Copy,
     D: DepKind,
 {
     #[inline(never)]
@@ -291,7 +291,7 @@ where
                 QueryResult::Started(job) => job,
                 QueryResult::Poisoned => panic!(),
             };
-            shard.insert(self.key.clone(), QueryResult::Poisoned);
+            shard.insert(self.key, QueryResult::Poisoned);
             job
         };
         // Also signal the completion of the job, so waiters
@@ -310,7 +310,7 @@ pub(crate) struct CycleError<D: DepKind> {
 /// The result of `try_start`.
 enum TryGetJob<'tcx, K, D>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + Hash + Copy,
     D: DepKind,
 {
     /// The query is not yet started. Contains a guard to the cache eventually used to start it.
@@ -358,10 +358,9 @@ where
     Q: QueryConfig<Qcx>,
     Qcx: QueryContext,
 {
-    match JobOwner::<'_, Q::Key, Qcx::DepKind>::try_start(&qcx, state, span, key.clone()) {
+    match JobOwner::<'_, Q::Key, Qcx::DepKind>::try_start(&qcx, state, span, key) {
         TryGetJob::NotYetStarted(job) => {
-            let (result, dep_node_index) =
-                execute_job::<Q, Qcx>(qcx, key.clone(), dep_node, job.id);
+            let (result, dep_node_index) = execute_job::<Q, Qcx>(qcx, key, dep_node, job.id);
             if Q::FEEDABLE {
                 // We should not compute queries that also got a value via feeding.
                 // This can't happen, as query feeding adds the very dependencies to the fed query
@@ -551,7 +550,7 @@ where
     let prof_timer = qcx.dep_context().profiler().query_provider();
 
     // The dep-graph for this computation is already in-place.
-    let result = dep_graph.with_ignore(|| Q::compute(qcx, key.clone()));
+    let result = dep_graph.with_ignore(|| Q::compute(qcx, *key));
 
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 

library/alloc/src/collections/binary_heap/mod.rs

@@ -851,18 +851,30 @@ impl<T: Ord> BinaryHeap<T> {
     where
         F: FnMut(&T) -> bool,
     {
-        let mut first_removed = self.len();
+        struct RebuildOnDrop<'a, T: Ord> {
+            heap: &'a mut BinaryHeap<T>,
+            first_removed: usize,
+        }
+
+        let mut guard = RebuildOnDrop { first_removed: self.len(), heap: self };
+
         let mut i = 0;
-        self.data.retain(|e| {
+        guard.heap.data.retain(|e| {
             let keep = f(e);
-            if !keep && i < first_removed {
-                first_removed = i;
+            if !keep && i < guard.first_removed {
+                guard.first_removed = i;
             }
             i += 1;
             keep
         });
-        // data[0..first_removed] is untouched, so we only need to rebuild the tail:
-        self.rebuild_tail(first_removed);
+
+        impl<'a, T: Ord> Drop for RebuildOnDrop<'a, T> {
+            fn drop(&mut self) {
+                // data[..first_removed] is untouched, so we only need to
+                // rebuild the tail:
+                self.heap.rebuild_tail(self.first_removed);
+            }
+        }
     }
 }
 

library/alloc/src/collections/binary_heap/tests.rs

@@ -474,6 +474,25 @@ fn test_retain() {
     assert!(a.is_empty());
 }
 
+#[test]
+fn test_retain_catch_unwind() {
+    let mut heap = BinaryHeap::from(vec![3, 1, 2]);
+
+    // Removes the 3, then unwinds out of retain.
+    let _ = catch_unwind(AssertUnwindSafe(|| {
+        heap.retain(|e| {
+            if *e == 1 {
+                panic!();
+            }
+            false
+        });
+    }));
+
+    // Naively this would be [1, 2] (an invalid heap) if BinaryHeap delegates to
+    // Vec's retain impl and then does not rebuild the heap after that unwinds.
+    assert_eq!(heap.into_vec(), [2, 1]);
+}
+
 // old binaryheap failed this test
 //
 // Integrity means that all elements are present after a comparison panics,