From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by sourceware.org (Postfix) with ESMTPS id BF1D63857C71 for ; Fri, 4 Dec 2020 11:02:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BF1D63857C71 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org Received: from localhost (91-166-131-65.subs.proxad.net [91.166.131.65]) (Authenticated sender: dodj@seketeli.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id F2834100011; Fri, 4 Dec 2020 11:02:56 +0000 (UTC) Received: by localhost (Postfix, from userid 1001) id A48C51A262F; Fri, 4 Dec 2020 12:02:55 +0100 (CET) From: Dodji Seketeli To: Giuliano Procida Cc: libabigail@sourceware.org, kernel-team@android.com, maennich@google.com Subject: Re: [PATCH] abidiff: improve treatment of array types in leaf changes mode Organization: Me, myself and I References: <20201203150916.3540551-1-gprocida@google.com> X-Operating-System: Red Hat Enterprise Linux Server 7.8 X-URL: http://www.seketeli.net/~dodji Date: Fri, 04 Dec 2020 12:02:55 +0100 In-Reply-To: <20201203150916.3540551-1-gprocida@google.com> (Giuliano Procida's message of "Thu, 3 Dec 2020 15:09:16 +0000") Message-ID: <865z5hq1eo.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Dec 2020 11:03:00 -0000 Hello Giuliano, Giuliano Procida a =C3=A9crit: [...] > diff --git a/src/abg-ir.cc b/src/abg-ir.cc > index c6f7c13e..b0db9c39 100644 > --- a/src/abg-ir.cc > +++ b/src/abg-ir.cc > @@ -23606,7 +23606,7 @@ types_have_similar_structure(const type_base* fir= st, > || ty1->get_dimension_count() !=3D ty2->get_dimension_count() > || !types_have_similar_structure(ty1->get_element_type(), > ty2->get_element_type(), > - indirect_type)) > + true)) > return false; >=20=20 > return true; I think this change is correct, thanks for spotting that. However ... [...] > If an array's element type doesn't change name but has some other > (local) change, then the change should not also be considered local to > the array type. I find this comment confusing, even if I think I see what you mean. A change being local or not, is a concept that is not at the same logical level as the concept of "type similarity" defined in the comment of the function types_have_similar_structure. I would say that the type similarity concept is at a lower logical level. So seeing this comment that applies to a higher level concept for a change made to something But I agree that the comments of types_have_similar_structure are confusing as well. So I am proposing two patches following this message, for this issue. One patch amends the the comments for types_have_similar_structure, and the second one is your patch, with amended comments. And we can discuss from there as you like. [...] Cheers, --=20 Dodji