From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by sourceware.org (Postfix) with ESMTPS id F15B23858D35 for ; Mon, 27 Jul 2020 07:33:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F15B23858D35 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 X-Originating-IP: 91.166.131.130 Received: from localhost (91-166-131-130.subs.proxad.net [91.166.131.130]) (Authenticated sender: dodji@seketeli.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D2B21FF808; Mon, 27 Jul 2020 07:33:00 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 66EC218008D0; Mon, 27 Jul 2020 09:32:59 +0200 (CEST) From: Dodji Seketeli To: Giuliano Procida Cc: libabigail@sourceware.org, kernel-team@android.com, maennich@google.com Subject: Re: [PATCH 0/3] Type equality refactor and instrumentation Organization: Me, myself and I References: <20200708095315.948634-1-gprocida@google.com> X-Operating-System: Red Hat Enterprise Linux Workstation 7.8 Beta X-URL: http://www.seketeli.net/~dodji Date: Mon, 27 Jul 2020 09:32:59 +0200 In-Reply-To: <20200708095315.948634-1-gprocida@google.com> (Giuliano Procida's message of "Wed, 8 Jul 2020 10:53:12 +0100") Message-ID: <878sf5jtuc.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=-3.6 required=5.0 tests=BAYES_00, 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: Mon, 27 Jul 2020 07:33:05 -0000 Hello Giuliano, Giuliano Procida a =C3=A9crit: > Hi Dodji. > > This series refactors various operator=3D=3D methods making their common > functionality evident. > > The first patch is just a prelude to make the second smaller. > > The second patch does the refactoring. I'm not attached to the name > 'equality_helper'. Thank you for working on this. I have looked into these first two patches and they look good to me in gene= ral. I'll post their individual review as usual, shortly. > The third patch is not intended for direct inclusion in libabigail but > builds on the refactoring to investigate how equality and canonical > types work in practice. It identifies some potential discrepancies, > but they may be entirely expected. In it's current form, I'd rather hold the inclusion of this one for now. It's super intrusive, clutters the code quite a bit and I am not really sure about its practical use at the moment. > In general, it can be risky to define operator=3D=3D in a way where > reflexivity, symmetry and transitivity do not obviously hold or can be > sensitive to small code changes or in way where equality, say for > class_decl_sptr, can be affected by something like canonicalisation. > More instrumentation could be added to check behaviour. The comparison code is tricky. The number one reason for this is that it has to be fast. So yes, it's risky. One category of tests that are "simple" to perform and in practise are quite powerful to detect and debug potential comparison issues are "identity tests". That is, comparing a binary against itself and requiring that the result be the empty set. This is why the option "--abidiff" was added to abidw. So I tend to favor schemes that keep the code the less cluttered and the most "debuggable" as possible, as opposed to adding a lot of instrumentation. I guess some balance has to found there. Cheers, --=20 Dodji