Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

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

Open
wants to merge 2 commits into
base: rustc-backend
Choose a base branch
from

Conversation

scrabsha
Copy link
Contributor

@scrabsha scrabsha commented Oct 25, 2021

Previous type equality was performed with ptr::eq. It was nice because rustc is built such that "if two types are equal, then they are the same pointer". This is really good for internal compiler development, this not work well here, because the two types may come from the two versions of the crate we're analyzing (ie: previous::path::to::Type and next::path::to::Type), whichtriggers incorrect modifications in situations such as:

// Previous:
pub struct S;
pub fn f(_: S) {}

// Next
pub struct S;
pub fn f(_: S) {}

This PR fixes this behaviour by adding a custom equality implementation. The idea is simple: if ptr::eq returns false, then we check if the two types are ADTs or not. If this is the case, we compare their paths. This solution is maybe not future-proof, but it works well for now. These functions will make easier the handling of other languages items such as struct/enum.

Fix #38.

@scrabsha scrabsha linked an issue Oct 25, 2021 that may be closed by this pull request
Copy link
Member

@o0Ignition0o o0Ignition0o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll be able to slim it down to an impl Eq for our Custom type later, but this is good enough for now

Comment on lines +38 to +45
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's revisit this later


use rustc_middle::ty::{self, List, TyCtxt, TyKind};

pub(crate) trait Compare {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks a lot like impl Eq

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properly handle user-defined type
2 participants