Don't flag functions with user-defined datatypes as changes

src/comparator.rs

@@ -12,8 +12,8 @@ use crate::{invocation_settings::GlueCompilerInvocationSettings, public_api::Api
 
 use semver::{BuildMetadata, Prerelease, Version};
 
-use rustc_middle::middle::cstore::ExternCrateSource;
 use rustc_middle::ty::TyCtxt;
+use rustc_session::cstore::ExternCrateSource;
 use rustc_span::def_id::CrateNum;
 
 use crate::{diagnosis::DiagnosisItem, public_api::PublicApi};

src/compiler/instrumented_compiler.rs

@@ -7,6 +7,7 @@ use crate::{
 use anyhow::{anyhow, Context, Result as AnyResult};
 use rustc_driver::{Callbacks, Compilation, RunCompiler};
 use rustc_interface::Config;
+use rustc_middle::ty::TyCtxt;
 use rustc_session::config::Input;
 use rustc_span::FileName;
 use std::{
@@ -208,7 +209,7 @@ impl Callbacks for InstrumentedCompiler {
         // get prev & next
         let changeset = queries.global_ctxt().unwrap().take().enter(|tcx| {
             let comparator = ApiComparator::from_tcx_and_settings(&tcx, settings)?;
-            get_changeset(&comparator)
+            get_changeset(&tcx, &comparator)
         });
 
         *self = InstrumentedCompiler::Finished(changeset);
@@ -226,10 +227,10 @@ pub struct ChangeSet {
 }
 
 impl ChangeSet {
-    pub(crate) fn from_diffs(d: Vec<Diff>) -> Self {
+    pub(crate) fn from_diffs(tcx: &TyCtxt, d: Vec<Diff>) -> Self {
         let mut changes = d
             .into_iter()
-            .filter_map(Change::from_diff)
+            .filter_map(|diff| Change::from_diff(tcx, diff))
             .map(ExposedChange::from_change)
             .collect::<Vec<_>>();
 
@@ -407,7 +408,7 @@ pub(crate) enum Change<'tcx> {
 }
 
 impl<'tcx> Change<'tcx> {
-    pub(crate) fn from_diff(d: Diff<'tcx>) -> Option<Change<'tcx>> {
+    pub(crate) fn from_diff(tcx: &TyCtxt, d: Diff<'tcx>) -> Option<Change<'tcx>> {
         // TODO [sasha]: there's some polymorphism here to perform
         // to figure out if a change is breaking
 
@@ -421,7 +422,7 @@ impl<'tcx> Change<'tcx> {
                 Some(Change::Breaking(d))
             }
 
-            Diff::Edition(prev, next) => ApiItem::changes_between(prev, next),
+            Diff::Edition(prev, next) => ApiItem::changes_between(tcx, prev, next),
 
             // Removing anything that is publicly exposed is always breaking.
             Diff::Deletion(_) => Some(Change::Breaking(d)),
@@ -437,10 +438,11 @@ impl<'tcx> Change<'tcx> {
 }
 
 fn get_changeset<'tcx, 'rustc>(
+    tcx: &TyCtxt,
     comparator: &'rustc impl Comparator<'tcx, 'rustc>,
 ) -> AnyResult<ChangeSet> {
     // get additions / editions / deletions
     let diffs: Vec<Diff> = comparator.get_diffs();
 
-    Ok(ChangeSet::from_diffs(diffs))
+    Ok(ChangeSet::from_diffs(tcx, diffs))
 }

src/public_api.rs

@@ -1,5 +1,6 @@
 mod functions;
 mod modules;
+mod utils;
 
 use std::{cmp::Ordering, collections::HashMap};
 
@@ -108,11 +109,12 @@ impl<'tcx> ApiItem<'tcx> {
     }
 
     pub(crate) fn changes_between(
+        tcx: &TyCtxt,
         prev: ApiItem<'tcx>,
         next: ApiItem<'tcx>,
     ) -> Option<Change<'tcx>> {
         match (prev, next) {
-            (ApiItem::Fn(prev), ApiItem::Fn(next)) => FnMetadata::changes_between(prev, next),
+            (ApiItem::Fn(prev), ApiItem::Fn(next)) => FnMetadata::changes_between(tcx, prev, next),
             (ApiItem::Mod(prev), ApiItem::Mod(next)) => ModMetadata::changes_between(prev, next),
 
             _ => unreachable!("Attempt to generate changes for two different-kinded types"),

src/public_api/functions.rs

@@ -4,7 +4,7 @@ use rustc_middle::ty::{FnSig, TyCtxt};
 
 use crate::{comparator::Diff, compiler::Change};
 
-use super::ApiItem;
+use super::{utils::Compare, ApiItem};
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(crate) struct FnMetadata<'tcx> {
@@ -30,15 +30,29 @@ impl<'tcx> FnMetadata<'tcx> {
     }
 
     pub(crate) fn changes_between(
+        tcx: &TyCtxt,
         prev: FnMetadata<'tcx>,
         next: FnMetadata<'tcx>,
     ) -> Option<Change<'tcx>> {
         if prev.sig != next.sig {
-            // Adding or removing an argument is *always* a breaking change.
-            Some(Change::Breaking(Diff::Edition(
-                prev.to_api_item(),
-                next.to_api_item(),
-            )))
+            let from_args =
+                tcx.ty_list_eq(&prev.sig.inputs_and_output, &next.sig.inputs_and_output);
+
+            let is_unchanged = tcx
+                .ty_list_eq(&prev.sig.inputs_and_output, &next.sig.inputs_and_output)
+                && prev.sig.c_variadic == next.sig.c_variadic
+                && prev.sig.unsafety == next.sig.unsafety
+                && prev.sig.abi == next.sig.abi;
+
+            if !is_unchanged {
+                // Adding or removing an argument is *always* a breaking change.
+                Some(Change::Breaking(Diff::Edition(
+                    prev.to_api_item(),
+                    next.to_api_item(),
+                )))
+            } else {
+                None
+            }
         } else {
             None
         }

src/public_api/utils.rs

@@ -0,0 +1,47 @@
+use std::ptr;
+
+use rustc_middle::ty::{self, List, TyCtxt, TyKind};
+
+pub(crate) trait Compare {
+    fn ty_eq(&self, left: ty::Ty, right: ty::Ty) -> bool {
+        ptr::eq(left, right) || self.adt_ty_eq(left, right).unwrap_or_default()
+    }
+
+    fn ty_list_eq(&self, left: &List<ty::Ty>, right: &List<ty::Ty>) -> bool {
+        left == right
+            || (left.len() == right.len()
+                && left
+                    .iter()
+                    .zip(right.iter())
+                    .all(|(left, right)| self.ty_eq(left, right)))
+    }
+
+    /// Returns Some(true) if both left and right are ADTs and refer to the same
+    /// type.
+    ///
+    /// Returns Some(false) if both left and right are ADTs but do not refer to
+    /// the same type.
+    ///
+    /// Returns None if at least one of left or right is not an ADT.
+    fn adt_ty_eq(&self, left: ty::Ty, right: ty::Ty) -> Option<bool>;
+}
+
+impl<'tcx> Compare for TyCtxt<'tcx> {
+    fn adt_ty_eq(&self, left: ty::Ty, right: ty::Ty) -> Option<bool> {
+        let left_path = adt_path_of_ty(self, left)?;
+        let right_path = adt_path_of_ty(self, right)?;
+
+        Some(left_path == right_path)
+    }
+}
+
+fn adt_path_of_ty(tcx: &TyCtxt<'_>, ty: ty::Ty) -> Option<String> {
+    let adt_def_id = match ty.kind() {
+        TyKind::Adt(adt, _) => adt.did,
+        _ => return None,
+    };
+
+    let path = tcx.def_path(adt_def_id).to_string_no_crate_verbose();
+
+    Some(path)
+}

tests/function.rs

@@ -99,6 +99,22 @@ fn fn_arg_last_character_not_removed() {
     assert_eq!(diff.to_string(), "≠ a\n");
 }
 
+#[test]
+fn user_defined_type_change_is_not_reported() {
+    let diff = compatibility_diagnosis! {
+        {
+            pub struct S;
+            pub fn f(_: S) {}
+        },
+        {
+            pub struct S;
+            pub fn f(_: S) {}
+        },
+    };
+
+    assert!(diff.is_empty());
+}
+
 #[test]
 fn empty_struct_kind_change_is_modification() {
     let files = ["pub struct A;", "pub struct A();", "pub struct A {}"];