Auto merge of #156041 - JonathanBrouwer:rollup-yc0vOVk, r=JonathanBrouwer

Rollup of 9 pull requests

Successful merges:

 - #149637 (Do not run jump-threading for GPUs)
 - #154971 (Verify that penultimate segment of enum variant path refers to enum if it has args)
 - #155186 (Avoid loop_match self-assignment in MIR lowering)
 - #155948 (Fix order-dependent visibility diagnostics)
 - #156001 (ssa-range-prop: fix ICE when encountering self-domiating bb)
 - #155600 (Adds a couple UI tests for polonius)
 - #155995 (-Zembed-source: also embed external source)
 - #156019 (Feed cleanups)
 - #156031 (Return a single diagnostic from `lex_token_trees`.)

Author: bors

compiler/rustc_codegen_cranelift/src/debuginfo/line_info.rs

@@ -47,7 +47,8 @@ fn osstr_as_utf8_bytes(path: &OsStr) -> &[u8] {
 
 fn make_file_info(source_file: &SourceFile, embed_source: bool) -> Option<FileInfo> {
     let has_md5 = source_file.src_hash.kind == SourceFileHashAlgorithm::Md5;
-    let has_source = embed_source && source_file.src.is_some();
+    let has_source = embed_source
+        && (source_file.src.is_some() || source_file.external_src.read().get_source().is_some());
 
     if !has_md5 && !has_source {
         return None;
@@ -62,6 +63,8 @@ fn make_file_info(source_file: &SourceFile, embed_source: bool) -> Option<FileIn
     if embed_source {
         if let Some(src) = &source_file.src {
             info.source = Some(LineString::String(src.as_bytes().to_vec()));
+        } else if let Some(src) = source_file.external_src.read().get_source() {
+            info.source = Some(LineString::String(src.as_bytes().to_vec()));
         }
     }
 
@@ -79,19 +82,22 @@ impl DebugContext {
         let span = hygiene::walk_chain_collapsed(span, function_span);
         match tcx.sess.source_map().lookup_line(span.lo()) {
             Ok(SourceFileAndLine { sf: file, line }) => {
-                let file_id = self.add_source_file(&file);
+                let file_id = self.add_source_file(tcx, &file);
                 let line_pos = file.lines()[line];
                 let col = file.relative_position(span.lo()) - line_pos;
 
                 (file_id, u64::try_from(line).unwrap() + 1, u64::from(col.to_u32()) + 1)
             }
-            Err(file) => (self.add_source_file(&file), 0, 0),
+            Err(file) => (self.add_source_file(tcx, &file), 0, 0),
         }
     }
 
-    pub(crate) fn add_source_file(&mut self, source_file: &SourceFile) -> FileId {
+    pub(crate) fn add_source_file(&mut self, tcx: TyCtxt<'_>, source_file: &SourceFile) -> FileId {
         let cache_key = (source_file.stable_id, source_file.src_hash);
         *self.created_files.entry(cache_key).or_insert_with(|| {
+            if self.embed_source && source_file.src.is_none() {
+                tcx.sess.source_map().ensure_source_file_source_present(source_file);
+            }
             let line_program: &mut LineProgram = &mut self.dwarf.unit.line_program;
             let line_strings: &mut LineStringTable = &mut self.dwarf.line_strings;
 

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -2,7 +2,6 @@ use std::borrow::Cow;
 use std::fmt::{self, Write};
 use std::hash::{Hash, Hasher};
 use std::path::PathBuf;
-use std::sync::Arc;
 use std::{assert_matches, iter, ptr};
 
 use libc::{c_longlong, c_uint};
@@ -607,8 +606,16 @@ pub(crate) fn file_metadata<'ll>(cx: &CodegenCx<'ll, '_>, source_file: &SourceFi
         };
         let hash_value = hex_encode(source_file.src_hash.hash_bytes());
 
-        let source =
-            cx.sess().opts.unstable_opts.embed_source.then_some(()).and(source_file.src.as_ref());
+        let mut source = None;
+        let external_src;
+        if cx.sess().opts.unstable_opts.embed_source {
+            source = source_file.src.as_deref().map(String::as_str);
+            if source.is_none() {
+                cx.tcx.sess.source_map().ensure_source_file_source_present(source_file);
+                external_src = source_file.external_src.read();
+                source = external_src.get_source();
+            }
+        }
 
         create_file(DIB(cx), &file_name, &directory, &hash_value, hash_kind, source)
     }
@@ -626,7 +633,7 @@ fn create_file<'ll>(
     directory: &str,
     hash_value: &str,
     hash_kind: llvm::ChecksumKind,
-    source: Option<&Arc<String>>,
+    source: Option<&str>,
 ) -> &'ll DIFile {
     unsafe {
         llvm::LLVMRustDIBuilderCreateFile(

compiler/rustc_data_structures/src/graph/dominators/mod.rs

@@ -386,6 +386,24 @@ impl<Node: Idx> Dominators<Node> {
             }
         }
     }
+
+    /// Returns true if `a` **strictly** dominates `b`
+    ///
+    /// # Panics
+    ///
+    /// Panics if `b` is unreachable
+    #[inline]
+    pub fn strictly_dominates(&self, a: Node, b: Node) -> bool {
+        match &self.kind {
+            Kind::Path => a.index() < b.index(),
+            Kind::General(g) => {
+                let a = g.time[a];
+                let b = g.time[b];
+                assert!(b.start != 0, "node {b:?} is not reachable");
+                a.start < b.start && b.finish < a.finish
+            }
+        }
+    }
 }
 
 /// Describes the number of vertices discovered at the time when processing of a particular vertex

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -2174,28 +2174,51 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             // Case 2. Reference to a variant constructor.
             DefKind::Ctor(CtorOf::Variant, ..) | DefKind::Variant => {
                 let (generics_def_id, index) = if let Some(self_ty) = self_ty {
+                    // We have something like `<module::Enum>::Variant`.
+
                     let adt_def = self.probe_adt(span, self_ty).unwrap();
                     debug_assert!(adt_def.is_enum());
+
+                    // FIXME: Stating that the last segment (here: `Variant`) is allowed to have
+                    // generic args is a lie! We should set the index to `None` instead as it's
+                    // the *self type* that's allowed to have args.
+                    // HIR typeck's `instantiate_value_path` actually contains a special case to
+                    // reject args on `DefKind::Ctor` segments (see `is_alias_variant_ctor`).
+                    // Using `None` here for this should allow us to get rid of that workaround.
+                    //
+                    // (For additional context, `DefKind::Variant` segments never actually reach
+                    // this branch as they're interpreted as `TypeRelative` paths whose lowering
+                    // routines manually reject args on them).
+
                     (adt_def.did(), last)
-                } else if last >= 1 && segments[last - 1].args.is_some() {
-                    // Everything but the penultimate segment should have no
-                    // parameters at all.
-                    let mut def_id = def_id;
+                } else if let [.., second_to_last, _] = segments
+                    && second_to_last.args.is_some()
+                    && let Res::Def(DefKind::Enum, _) = second_to_last.res
+                {
+                    // We have something like `module::Enum::<…>::Variant`.
+                    // No segment other than the penultimate one is allowed to have generic args.
+
+                    // We had to check that the second to last segment actually referred to an enum
+                    // since at this stage it could very well refer to a module in which case we
+                    // certainly don't want to allow generic args on it!
 
                     // `DefKind::Ctor` -> `DefKind::Variant`
-                    if let DefKind::Ctor(..) = kind {
-                        def_id = tcx.parent(def_id);
-                    }
+                    let def_id = match kind {
+                        DefKind::Ctor(..) => tcx.parent(def_id),
+                        _ => def_id,
+                    };
 
                     // `DefKind::Variant` -> `DefKind::Enum`
                     let enum_def_id = tcx.parent(def_id);
+
                     (enum_def_id, last - 1)
                 } else {
+                    // We have something like `module::Enum::Variant` or `module::Variant`.
+                    // No segment other than the final one is allowed to have generic args.
+
                     // FIXME: lint here recommending `Enum::<...>::Variant` form
                     // instead of `Enum::Variant::<...>` form.
 
-                    // Everything but the final segment should have no
-                    // parameters at all.
                     let generics = tcx.generics_of(def_id);
                     // Variant and struct constructors use the
                     // generics of their parent type definition.

compiler/rustc_middle/src/mir/mod.rs

@@ -1609,6 +1609,11 @@ impl Location {
             dominators.dominates(self.block, other.block)
         }
     }
+
+    #[inline]
+    pub fn strictly_dominates(&self, other: Location, dominators: &Dominators<BasicBlock>) -> bool {
+        self.block != other.block && dominators.strictly_dominates(self.block, other.block)
+    }
 }
 
 /// `DefLocation` represents the location of a definition - either an argument or an assignment

compiler/rustc_middle/src/ty/context.rs

@@ -10,7 +10,7 @@ use std::cmp::Ordering;
 use std::env::VarError;
 use std::ffi::OsStr;
 use std::hash::{Hash, Hasher};
-use std::marker::{PhantomData, PointeeSized};
+use std::marker::PointeeSized;
 use std::ops::{Bound, Deref};
 use std::sync::{Arc, OnceLock};
 use std::{fmt, iter, mem};
@@ -539,36 +539,14 @@ pub struct FreeRegionInfo {
 
 /// This struct should only be created by `create_def`.
 #[derive(Copy, Clone)]
-pub struct TyCtxtFeed<'tcx, KEY: Copy> {
+pub struct TyCtxtFeed<'tcx, K: Copy> {
     pub tcx: TyCtxt<'tcx>,
     // Do not allow direct access, as downstream code must not mutate this field.
-    key: KEY,
+    key: K,
 }
 
-/// Never return a `Feed` from a query. Only queries that create a `DefId` are
-/// allowed to feed queries for that `DefId`.
-impl<KEY: Copy> !HashStable for TyCtxtFeed<'_, KEY> {}
-
-/// The same as `TyCtxtFeed`, but does not contain a `TyCtxt`.
-/// Use this to pass around when you have a `TyCtxt` elsewhere.
-/// Just an optimization to save space and not store hundreds of
-/// `TyCtxtFeed` in the resolver.
-#[derive(Copy, Clone)]
-pub struct Feed<'tcx, KEY: Copy> {
-    _tcx: PhantomData<TyCtxt<'tcx>>,
-    // Do not allow direct access, as downstream code must not mutate this field.
-    key: KEY,
-}
-
-/// Never return a `Feed` from a query. Only queries that create a `DefId` are
-/// allowed to feed queries for that `DefId`.
-impl<KEY: Copy> !HashStable for Feed<'_, KEY> {}
-
-impl<T: fmt::Debug + Copy> fmt::Debug for Feed<'_, T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.key.fmt(f)
-    }
-}
+/// Only queries that create a `DefId` are allowed to feed queries for that `DefId`.
+impl<K: Copy> !HashStable for TyCtxtFeed<'_, K> {}
 
 /// Some workarounds to use cases that cannot use `create_def`.
 /// Do not add new ways to create `TyCtxtFeed` without consulting
@@ -622,28 +600,11 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 }
 
-impl<'tcx, KEY: Copy> TyCtxtFeed<'tcx, KEY> {
+impl<'tcx, K: Copy> TyCtxtFeed<'tcx, K> {
     #[inline(always)]
-    pub fn key(&self) -> KEY {
+    pub fn key(&self) -> K {
         self.key
     }
-
-    #[inline(always)]
-    pub fn downgrade(self) -> Feed<'tcx, KEY> {
-        Feed { _tcx: PhantomData, key: self.key }
-    }
-}
-
-impl<'tcx, KEY: Copy> Feed<'tcx, KEY> {
-    #[inline(always)]
-    pub fn key(&self) -> KEY {
-        self.key
-    }
-
-    #[inline(always)]
-    pub fn upgrade(self, tcx: TyCtxt<'tcx>) -> TyCtxtFeed<'tcx, KEY> {
-        TyCtxtFeed { tcx, key: self.key }
-    }
 }
 
 impl<'tcx> TyCtxtFeed<'tcx, LocalDefId> {

compiler/rustc_middle/src/ty/mod.rs

@@ -83,7 +83,7 @@ pub use self::consts::{
     const_lit_matches_ty,
 };
 pub use self::context::{
-    CtxtInterners, CurrentGcx, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, TyCtxtFeed, tls,
+    CtxtInterners, CurrentGcx, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, TyCtxtFeed, tls,
 };
 pub use self::fold::*;
 pub use self::instance::{Instance, InstanceKind, ReifyReason};

compiler/rustc_mir_build/src/builder/expr/into.rs

@@ -326,36 +326,61 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     );
 
                     let state_place = scrutinee_place_builder.to_place(this);
+                    let state_result = this.temp(state_ty, expr_span);
+                    let state_result_place = Place::from(state_result);
 
                     // This is logic for the labeled block: a block is a drop scope, hence
                     // `in_scope`, and a labeled block can be broken out of with a `break 'label`,
                     // hence the `in_breakable_scope`.
                     //
+                    // The state update is still modeled like `state = 'blk: { ... }`: normal
+                    // match arm results and ordinary breaks to the block are first written to
+                    // `state_result_place`, then written back to `state_place`. This avoids
+                    // building an overlapping assignment like `state = state`.
+                    //
                     // Then `in_const_continuable_scope` stores information for the lowering of
-                    // `#[const_continue]`, and finally the match is lowered in the standard way.
+                    // `#[const_continue]`, which still updates the actual `state_place` directly
+                    // so it can jump to the statically known next match branch.
                     unpack!(
                         body_block = this.in_scope(
                             (region_scope, source_info),
                             LintLevel::Inherited,
                             move |this| {
-                                this.in_breakable_scope(None, state_place, expr_span, |this| {
-                                    Some(this.in_const_continuable_scope(
-                                        Box::from(arms),
-                                        built_tree.clone(),
-                                        state_place,
-                                        expr_span,
-                                        |this| {
-                                            this.lower_match_arms(
-                                                state_place,
-                                                scrutinee_place_builder,
-                                                scrutinee_span,
-                                                arms,
-                                                built_tree,
-                                                this.source_info(match_span),
+                                this.in_const_continuable_scope(
+                                    Box::from(arms),
+                                    built_tree.clone(),
+                                    state_place,
+                                    expr_span,
+                                    |this| {
+                                        let block = this
+                                            .in_breakable_scope(
+                                                None,
+                                                state_result_place,
+                                                expr_span,
+                                                |this| {
+                                                    Some(this.lower_match_arms(
+                                                        state_result_place,
+                                                        scrutinee_place_builder,
+                                                        scrutinee_span,
+                                                        arms,
+                                                        built_tree,
+                                                        this.source_info(match_span),
+                                                    ))
+                                                },
                                             )
-                                        },
-                                    ))
-                                })
+                                            .into_block();
+
+                                        this.cfg.push_assign(
+                                            block,
+                                            source_info,
+                                            state_place,
+                                            Rvalue::Use(
+                                                this.consume_by_copy_or_move(state_result_place),
+                                            ),
+                                        );
+                                        block.unit()
+                                    },
+                                )
                             }
                         )
                     );

compiler/rustc_mir_transform/src/jump_threading.rs

@@ -76,6 +76,14 @@ const MAX_COST: u8 = 100;
 
 impl<'tcx> crate::MirPass<'tcx> for JumpThreading {
     fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
+        if sess.target.is_like_gpu {
+            // Jump threading can duplicate calls in control-flow.
+            // This leads to incorrect code when done for so called "convergent" operations on GPU
+            // targets, similar to how inline assembly cannot be duplicated on all targets.
+            // Conservatively prevent this by disabling the pass.
+            // See also issue #137086.
+            return false;
+        }
         sess.mir_opt_level() >= 2
     }
 

compiler/rustc_mir_transform/src/lint.rs

@@ -66,6 +66,12 @@ impl<'a, 'tcx> Lint<'a, 'tcx> {
     }
 }
 
+/// Checks whether reading `src` while assigning to `dest` would violate MIR's
+/// non-overlap requirement for assignments.
+fn places_conflict_for_assignment<'tcx>(dest: Place<'tcx>, src: Place<'tcx>) -> bool {
+    dest == src || (dest.local == src.local && !dest.is_indirect() && !src.is_indirect())
+}
+
 impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> {
     fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) {
         if context.is_use() {
@@ -96,11 +102,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> {
                 // The sides of an assignment must not alias.
                 if forbid_aliasing {
                     VisitPlacesWith(|src: Place<'tcx>, _| {
-                        if *dest == src
-                            || (dest.local == src.local
-                                && !dest.is_indirect()
-                                && !src.is_indirect())
-                        {
+                        if places_conflict_for_assignment(*dest, src) {
                             self.fail(
                                 location,
                                 format!(