Auto merge of #111776 - cuviper:beta-next, r=cuviper

[beta] backport

- debuginfo: split method declaration and definition #111167
- Encode VariantIdx so we can decode ADT variants in the right order #111494
- Simplify find_width_of_character_at_span. #111560

r? cuviper

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/mod.rs

@@ -321,7 +321,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         let tcx = self.tcx;
 
         let def_id = instance.def_id();
-        let containing_scope = get_containing_scope(self, instance);
+        let (containing_scope, is_method) = get_containing_scope(self, instance);
         let span = tcx.def_span(def_id);
         let loc = self.lookup_debug_loc(span.lo());
         let file_metadata = file_metadata(self, &loc.file);
@@ -377,8 +377,29 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
             }
         }
 
-        unsafe {
-            return llvm::LLVMRustDIBuilderCreateFunction(
+        // When we're adding a method to a type DIE, we only want a DW_AT_declaration there, because
+        // LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition.
+        // When we use this `decl` below, the subprogram definition gets created at the CU level
+        // with a DW_AT_specification pointing back to the type's declaration.
+        let decl = is_method.then(|| unsafe {
+            llvm::LLVMRustDIBuilderCreateMethod(
+                DIB(self),
+                containing_scope,
+                name.as_ptr().cast(),
+                name.len(),
+                linkage_name.as_ptr().cast(),
+                linkage_name.len(),
+                file_metadata,
+                loc.line,
+                function_type_metadata,
+                flags,
+                spflags & !DISPFlags::SPFlagDefinition,
+                template_parameters,
+            )
+        });
+
+        return unsafe {
+            llvm::LLVMRustDIBuilderCreateFunction(
                 DIB(self),
                 containing_scope,
                 name.as_ptr().cast(),
@@ -393,9 +414,9 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                 spflags,
                 maybe_definition_llfn,
                 template_parameters,
-                None,
-            );
-        }
+                decl,
+            )
+        };
 
         fn get_function_signature<'ll, 'tcx>(
             cx: &CodegenCx<'ll, 'tcx>,
@@ -494,14 +515,16 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
             names
         }
 
+        /// Returns a scope, plus `true` if that's a type scope for "class" methods,
+        /// otherwise `false` for plain namespace scopes.
         fn get_containing_scope<'ll, 'tcx>(
             cx: &CodegenCx<'ll, 'tcx>,
             instance: Instance<'tcx>,
-        ) -> &'ll DIScope {
+        ) -> (&'ll DIScope, bool) {
             // First, let's see if this is a method within an inherent impl. Because
             // if yes, we want to make the result subroutine DIE a child of the
             // subroutine's self-type.
-            let self_type = cx.tcx.impl_of_method(instance.def_id()).and_then(|impl_def_id| {
+            if let Some(impl_def_id) = cx.tcx.impl_of_method(instance.def_id()) {
                 // If the method does *not* belong to a trait, proceed
                 if cx.tcx.trait_id_of_impl(impl_def_id).is_none() {
                     let impl_self_ty = cx.tcx.subst_and_normalize_erasing_regions(
@@ -512,39 +535,34 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
 
                     // Only "class" methods are generally understood by LLVM,
                     // so avoid methods on other types (e.g., `<*mut T>::null`).
-                    match impl_self_ty.kind() {
-                        ty::Adt(def, ..) if !def.is_box() => {
-                            // Again, only create type information if full debuginfo is enabled
-                            if cx.sess().opts.debuginfo == DebugInfo::Full
-                                && !impl_self_ty.needs_subst()
-                            {
-                                Some(type_di_node(cx, impl_self_ty))
-                            } else {
-                                Some(namespace::item_namespace(cx, def.did()))
-                            }
+                    if let ty::Adt(def, ..) = impl_self_ty.kind() && !def.is_box() {
+                        // Again, only create type information if full debuginfo is enabled
+                        if cx.sess().opts.debuginfo == DebugInfo::Full
+                            && !impl_self_ty.needs_subst()
+                        {
+                            return (type_di_node(cx, impl_self_ty), true);
+                        } else {
+                            return (namespace::item_namespace(cx, def.did()), false);
                         }
-                        _ => None,
                     }
                 } else {
                     // For trait method impls we still use the "parallel namespace"
                     // strategy
-                    None
                 }
-            });
+            }
 
-            self_type.unwrap_or_else(|| {
-                namespace::item_namespace(
-                    cx,
-                    DefId {
-                        krate: instance.def_id().krate,
-                        index: cx
-                            .tcx
-                            .def_key(instance.def_id())
-                            .parent
-                            .expect("get_containing_scope: missing parent?"),
-                    },
-                )
-            })
+            let scope = namespace::item_namespace(
+                cx,
+                DefId {
+                    krate: instance.def_id().krate,
+                    index: cx
+                        .tcx
+                        .def_key(instance.def_id())
+                        .parent
+                        .expect("get_containing_scope: missing parent?"),
+                },
+            );
+            (scope, false)
         }
     }
 

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -1986,6 +1986,21 @@ extern "C" {
         Decl: Option<&'a DIDescriptor>,
     ) -> &'a DISubprogram;
 
+    pub fn LLVMRustDIBuilderCreateMethod<'a>(
+        Builder: &DIBuilder<'a>,
+        Scope: &'a DIDescriptor,
+        Name: *const c_char,
+        NameLen: size_t,
+        LinkageName: *const c_char,
+        LinkageNameLen: size_t,
+        File: &'a DIFile,
+        LineNo: c_uint,
+        Ty: &'a DIType,
+        Flags: DIFlags,
+        SPFlags: DISPFlags,
+        TParam: &'a DIArray,
+    ) -> &'a DISubprogram;
+
     pub fn LLVMRustDIBuilderCreateBasicType<'a>(
         Builder: &DIBuilder<'a>,
         Name: *const c_char,

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -831,6 +831,28 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFunction(
   return wrap(Sub);
 }
 
+extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMethod(
+    LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope,
+    const char *Name, size_t NameLen,
+    const char *LinkageName, size_t LinkageNameLen,
+    LLVMMetadataRef File, unsigned LineNo,
+    LLVMMetadataRef Ty, LLVMRustDIFlags Flags,
+    LLVMRustDISPFlags SPFlags, LLVMMetadataRef TParam) {
+  DITemplateParameterArray TParams =
+      DITemplateParameterArray(unwrap<MDTuple>(TParam));
+  DISubprogram::DISPFlags llvmSPFlags = fromRust(SPFlags);
+  DINode::DIFlags llvmFlags = fromRust(Flags);
+  DISubprogram *Sub = Builder->createMethod(
+      unwrapDI<DIScope>(Scope),
+      StringRef(Name, NameLen),
+      StringRef(LinkageName, LinkageNameLen),
+      unwrapDI<DIFile>(File), LineNo,
+      unwrapDI<DISubroutineType>(Ty),
+      0, 0, nullptr, // VTable params aren't used
+      llvmFlags, llvmSPFlags, TParams);
+  return wrap(Sub);
+}
+
 extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType(
     LLVMRustDIBuilderRef Builder, const char *Name, size_t NameLen,
     uint64_t SizeInBits, unsigned Encoding) {

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -857,7 +857,12 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         )
     }
 
-    fn get_variant(self, kind: &DefKind, index: DefIndex, parent_did: DefId) -> ty::VariantDef {
+    fn get_variant(
+        self,
+        kind: DefKind,
+        index: DefIndex,
+        parent_did: DefId,
+    ) -> (VariantIdx, ty::VariantDef) {
         let adt_kind = match kind {
             DefKind::Variant => ty::AdtKind::Enum,
             DefKind::Struct => ty::AdtKind::Struct,
@@ -871,27 +876,30 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
             if adt_kind == ty::AdtKind::Enum { Some(self.local_def_id(index)) } else { None };
         let ctor = data.ctor.map(|(kind, index)| (kind, self.local_def_id(index)));
 
-        ty::VariantDef::new(
-            self.item_name(index),
-            variant_did,
-            ctor,
-            data.discr,
-            self.root
-                .tables
-                .children
-                .get(self, index)
-                .expect("fields are not encoded for a variant")
-                .decode(self)
-                .map(|index| ty::FieldDef {
-                    did: self.local_def_id(index),
-                    name: self.item_name(index),
-                    vis: self.get_visibility(index),
-                })
-                .collect(),
-            adt_kind,
-            parent_did,
-            false,
-            data.is_non_exhaustive,
+        (
+            data.idx,
+            ty::VariantDef::new(
+                self.item_name(index),
+                variant_did,
+                ctor,
+                data.discr,
+                self.root
+                    .tables
+                    .children
+                    .get(self, index)
+                    .expect("fields are not encoded for a variant")
+                    .decode(self)
+                    .map(|index| ty::FieldDef {
+                        did: self.local_def_id(index),
+                        name: self.item_name(index),
+                        vis: self.get_visibility(index),
+                    })
+                    .collect(),
+                adt_kind,
+                parent_did,
+                false,
+                data.is_non_exhaustive,
+            ),
         )
     }
 
@@ -907,7 +915,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         };
         let repr = self.root.tables.repr_options.get(self, item_id).unwrap().decode(self);
 
-        let variants = if let ty::AdtKind::Enum = adt_kind {
+        let mut variants: Vec<_> = if let ty::AdtKind::Enum = adt_kind {
             self.root
                 .tables
                 .children
@@ -918,15 +926,22 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
                     let kind = self.def_kind(index);
                     match kind {
                         DefKind::Ctor(..) => None,
-                        _ => Some(self.get_variant(&kind, index, did)),
+                        _ => Some(self.get_variant(kind, index, did)),
                     }
                 })
                 .collect()
         } else {
-            std::iter::once(self.get_variant(&kind, item_id, did)).collect()
+            std::iter::once(self.get_variant(kind, item_id, did)).collect()
         };
 
-        tcx.mk_adt_def(did, adt_kind, variants, repr)
+        variants.sort_by_key(|(idx, _)| *idx);
+
+        tcx.mk_adt_def(
+            did,
+            adt_kind,
+            variants.into_iter().map(|(_, variant)| variant).collect(),
+            repr,
+        )
     }
 
     fn get_visibility(self, id: DefIndex) -> Visibility<DefId> {

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1376,9 +1376,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             // Therefore, the loop over variants will encode its fields as the adt's children.
         }
 
-        for variant in adt_def.variants().iter() {
+        for (idx, variant) in adt_def.variants().iter_enumerated() {
             let data = VariantData {
                 discr: variant.discr,
+                idx,
                 ctor: variant.ctor.map(|(kind, def_id)| (kind, def_id.index)),
                 is_non_exhaustive: variant.is_field_list_non_exhaustive(),
             };

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -31,6 +31,7 @@ use rustc_span::edition::Edition;
 use rustc_span::hygiene::{ExpnIndex, MacroKind};
 use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::{self, ExpnData, ExpnHash, ExpnId, Span};
+use rustc_target::abi::VariantIdx;
 use rustc_target::spec::{PanicStrategy, TargetTriple};
 
 use std::marker::PhantomData;
@@ -423,6 +424,7 @@ define_tables! {
 
 #[derive(TyEncodable, TyDecodable)]
 struct VariantData {
+    idx: VariantIdx,
     discr: ty::VariantDiscr,
     /// If this is unit or tuple-variant/struct, then this is the index of the ctor id.
     ctor: Option<(CtorKind, DefIndex)>,

compiler/rustc_span/src/lib.rs

@@ -20,6 +20,7 @@
 #![feature(min_specialization)]
 #![feature(rustc_attrs)]
 #![feature(let_chains)]
+#![feature(round_char_boundary)]
 #![deny(rustc::untranslatable_diagnostic)]
 #![deny(rustc::diagnostic_outside_of_impl)]
 

compiler/rustc_span/src/source_map.rs

@@ -1019,36 +1019,19 @@ impl SourceMap {
 
         let src = local_begin.sf.external_src.borrow();
 
-        // We need to extend the snippet to the end of the src rather than to end_index so when
-        // searching forwards for boundaries we've got somewhere to search.
-        let snippet = if let Some(ref src) = local_begin.sf.src {
-            &src[start_index..]
+        let snippet = if let Some(src) = &local_begin.sf.src {
+            src
         } else if let Some(src) = src.get_source() {
-            &src[start_index..]
+            src
         } else {
             return 1;
         };
-        debug!("snippet=`{:?}`", snippet);
 
-        let mut target = if forwards { end_index + 1 } else { end_index - 1 };
-        debug!("initial target=`{:?}`", target);
-
-        while !snippet.is_char_boundary(target - start_index) && target < source_len {
-            target = if forwards {
-                target + 1
-            } else {
-                match target.checked_sub(1) {
-                    Some(target) => target,
-                    None => {
-                        break;
-                    }
-                }
-            };
-            debug!("target=`{:?}`", target);
+        if forwards {
+            (snippet.ceil_char_boundary(end_index + 1) - end_index) as u32
+        } else {
+            (end_index - snippet.floor_char_boundary(end_index - 1)) as u32
         }
-        debug!("final target=`{:?}`", target);
-
-        if forwards { (target - end_index) as u32 } else { (end_index - target) as u32 }
     }
 
     pub fn get_source_file(&self, filename: &FileName) -> Option<Lrc<SourceFile>> {

tests/run-make/issue-109934-lto-debuginfo/Makefile

@@ -0,0 +1,12 @@
+# ignore-cross-compile
+include ../tools.mk
+
+# With the upgrade to LLVM 16, this was getting:
+#
+#   error: Cannot represent a difference across sections
+#
+# The error stemmed from DI function definitions under type scopes, fixed by
+# only declaring in type scope and defining the subprogram elsewhere.
+
+all:
+	$(RUSTC) lib.rs --test -C lto=fat -C debuginfo=2 -C incremental=$(TMPDIR)/inc-fat

tests/run-make/issue-109934-lto-debuginfo/lib.rs

@@ -0,0 +1,9 @@
+extern crate alloc;
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn something_alloc() {
+        assert_eq!(Vec::<u32>::new(), Vec::<u32>::new());
+    }
+}