From owner-svn-src-all@FreeBSD.ORG Wed Dec 17 19:12:45 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id EDF5E6A7; Wed, 17 Dec 2014 19:12:44 +0000 (UTC) Received: from troutmask.apl.washington.edu (troutmask.apl.washington.edu [128.95.76.21]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "troutmask", Issuer "troutmask" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id C4276DB3; Wed, 17 Dec 2014 19:12:44 +0000 (UTC) Received: from troutmask.apl.washington.edu (localhost [127.0.0.1]) by troutmask.apl.washington.edu (8.14.9/8.14.9) with ESMTP id sBHJCZ4A093691 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 17 Dec 2014 11:12:35 -0800 (PST) (envelope-from sgk@troutmask.apl.washington.edu) Received: (from sgk@localhost) by troutmask.apl.washington.edu (8.14.9/8.14.9/Submit) id sBHJCZI6093690; Wed, 17 Dec 2014 11:12:35 -0800 (PST) (envelope-from sgk) Date: Wed, 17 Dec 2014 11:12:35 -0800 From: Steve Kargl To: Ed Schouten Subject: Re: svn commit: r275819 - in head/lib/msun: ld128 ld80 src Message-ID: <20141217191235.GA89501@troutmask.apl.washington.edu> References: <201412160921.sBG9LvFY064961@svn.freebsd.org> <20141216162055.GA64273@troutmask.apl.washington.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Dec 2014 19:12:45 -0000 On Wed, Dec 17, 2014 at 04:30:32PM +0100, Ed Schouten wrote: > Steve, > > 2014-12-16 17:20 GMT+01:00 Steve Kargl : > > This seems like a lot of code churn for very little benefit. > > > > In particular, I know that the one person working on fixing > > problems with FreeBSD's libm has a private repo and the openlibm > > and android developers base their libm off of FreeBSD's libm > > and now they'll need to resync their codebases and resolve > > conflicts. > > I'm always afraid of statements like these, as they can be brought to > the table to prevent any changes from being made. The fact that > someone else (be it Android or openlibm) uses our code should not > limit us as a project to make changes. Hopefully this change will > merge into their direction as well? I stand corrected. Foisting unnecessary code churn on others is now an acceptable practice. > The fact that we often do not dare to refactor our code is exactly > what puts us in the spot that a lot of our code is often not directly > reusable by others, needs to be forked and adjusted. Examples include > u_intX_t, bcopy(), etc. Your refactoring is nothing more than a gratuitous code change. The reasons you give in your commit log are bogus justification. But the damage is done. Asking you to revert the patch would simply be more code churn. > > This comment isn't true! These functions pre-date C11 by years. > > See r151865. These functions were designed to deal with gcc's > > poorly implemented I. See the paragraph above your comment. > > Keep in mind that the phrasing is intended to say that CMPLX*() and > friends are part of C11. Those do not pre-date C11. The phrasing is wrong. cpack[fl] came at least 6 years before C11 and were designed to work around defects in C99. CMPLX[FL] were introduced into C11 to address those defects. Changing cpack[fl] to CMPLX[FL] and claiming that the functions are modeled after the C11 macros is wrong (unless the meaning of "before" and "after" have changed). > > Upon further inspection with md5, this change affects only a single > > file. This last paragraph appears to be an excuse for a drive-by > > commit. > > But also acts as proof that the change is harmless. I am not entirely > sure what you're trying to imply with this. Are changes that do not > affect checksums of object files are bad? Gratuitous changes are well gratuitous. You are causing extra work for people actively working on libm and other projects that use FreeBSD's libm. The fact that I ran md5 over the *.o files suggests that you made the change without even checking on its effect on the resulting library. To be blunt, your patch has/had ZERO BENEFIT at the expense of causing additional work for others. There are ample opportunities to improve libm. powl, tgammal, j0l, j1l, jnl, y0l, y1l, ynl, and number of the long double complex functions are not implemented. -- Steve