From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by sourceware.org (Postfix) with ESMTPS id A71CB385B834 for ; Mon, 30 Mar 2020 14:13:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A71CB385B834 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-130.subs.proxad.net [91.166.131.130]) (Authenticated sender: dodji@seketeli.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id ADDEB20001F; Mon, 30 Mar 2020 14:13:31 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id DF051581890; Mon, 30 Mar 2020 16:13:30 +0200 (CEST) From: Dodji Seketeli To: Giuliano Procida Cc: libabigail@sourceware.org, kernel-team@android.com, maennich@google.com Subject: Re: [PATCH 4/7] abidiff: Remove member function diff blank lines. Organization: Me, myself and I References: <20200327140017.1917-1-gprocida@google.com> <20200327140017.1917-5-gprocida@google.com> X-Operating-System: Fedora 33 X-URL: http://www.seketeli.net/~dodji Date: Mon, 30 Mar 2020 16:13:30 +0200 In-Reply-To: <20200327140017.1917-5-gprocida@google.com> (Giuliano Procida's message of "Fri, 27 Mar 2020 14:00:14 +0000") Message-ID: <87bloeq6b9.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-19.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, 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: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Mar 2020 14:13:40 -0000 Giuliano Procida a =C3=A9crit: > Currently, lists of member function deletions, additions and changes > are each followed by a blank line in abidiff output. > > While this formatting works reasonably well in leaf-changes-only mode > for top-level changes to types, it is inconsistent with everthing > else. In particular, when reporting member function diffs in default > mode, or more deeply nested in leaf mode, the blank lines are > jarring. > > There was also no test coverage for leaf-changes-only mode. > > This change removes these blank lines and associated state variables > and logic. It adds a test case for leaf-changes-only mode. > > Note that the patch also modifies default mode reporting of member > types, template member functions and template member classes > analogously, but I wasn't able to exercise those code paths. The template handling code can be ignored for now. I initially started to work on it but it needed huge adaptations on the compiler side to have it emit debug info for non-instantiated templates. I eventually gave up on doing that because of more pressing matters. So until we have a compiler that emits debug information for non instantiated templates, I don't think that code will be exercised. [...] > diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc b/t= ests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc > new file mode 100644 > index 00000000..0437584e > --- /dev/null > +++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc > @@ -0,0 +1,14 @@ > +struct ops { > + // TODO: type, name and size are differnent from deleted_var, but this= is > + // still reported as a change rather than a deletion and an addition. > + long added_var; > + virtual long added_fn() { return 0; } This is by design. If a member variable S::foo_member at a given offset/ is replaced by another variable S::bar_member (foo_member and bar_member have the same offset), we chose to detect that foo_member was renamed into bar_member. So I think we can remove that TODO above. Would you agree? Cheers, --=20 Dodji