From owner-svn-src-all@FreeBSD.ORG Fri Dec 13 18:46:08 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6B771ACA for ; Fri, 13 Dec 2013 18:46:08 +0000 (UTC) Received: from nm32-vm0.bullet.mail.bf1.yahoo.com (nm32-vm0.bullet.mail.bf1.yahoo.com [72.30.239.136]) by mx1.freebsd.org (Postfix) with SMTP id F3A3C119B for ; Fri, 13 Dec 2013 18:46:07 +0000 (UTC) Received: from [98.139.212.150] by nm32.bullet.mail.bf1.yahoo.com with NNFMP; 13 Dec 2013 18:40:45 -0000 Received: from [98.139.211.203] by tm7.bullet.mail.bf1.yahoo.com with NNFMP; 13 Dec 2013 18:40:44 -0000 Received: from [127.0.0.1] by smtp212.mail.bf1.yahoo.com with NNFMP; 13 Dec 2013 18:40:44 -0000 X-Yahoo-Newman-Id: 975621.43504.bm@smtp212.mail.bf1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: dlvXta0VM1mki.5HzK8wPr0yOGktjPOy1boRaBPcidBeEFF HFAZWVz4LcQKmrTLwst6iShr7goiReWOnu3WPUlXjeeFJ72zUPtRu0BfRH4w Yob4zaUASW5I.mmj5UyBSw4lDE87s9oS2IK_mZ3xoorFAfG9gwvmuZ5nf67U VnDo2fMRhx.Q5An1NDiMvT1iJp7dc4xqRDzrOU3C5oIe_0jQ_nnZBBAKUoUD CRABBIHfV3stGS8JmBJgCWcLWiRPziQYA7CAytplSaeE2c2OGxMQHFrSvUQb .7vvEXU1NR40FzIGc2M42ZqIg.e3sPj79teTRMyC8jwXdsFDkYWMvsLqm1_G w7HgeMK4LgLk9Rq8ScRnN9SCFiX3smxR285iXk1Z0YeiGx5iVesp_6CdrZF5 WckBw0RJXxcpPvvLJ16cJe28Vk3nLuwGcG5diNmy3HL06kQL0_gfSkJqv2xi 1P35W44FaNm2d6s7x7FwH9W9HL2cu2SI1mWkiF3WlCKwSing1P3NetNvPfC5 339k.h80uEmYB2GaYgiYGlLaZ6NejjpSX23I2NMkElSFi7VKDJCEg3MdAH4y LHwI_KRckURzrQr5_ozj.S6054yiKil0oULJSVWXBeQqKzlxtzqS9iN0aNEd AIhAl8RFtJ6AXcC7ZSVeLlJHQzPnEc.UCYISzkY0E6E1SMYM4Wk_P90_WrHI 00wQ4dk9LYvcNI2Mnpis_ X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf X-Rocket-Received: from [192.168.0.102] (pfg@190.157.126.109 with plain [98.138.105.21]) by smtp212.mail.bf1.yahoo.com with SMTP; 13 Dec 2013 10:40:44 -0800 PST Message-ID: <52AB54A9.6040306@FreeBSD.org> Date: Fri, 13 Dec 2013 13:40:41 -0500 From: Pedro Giffuni User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Bruce Evans , Sean Bruno Subject: Re: svn commit: r258139 - head/contrib/gperf/src References: <201311141841.rAEIfwFU040620@svn.freebsd.org> <20131115145452.G954@besplex.bde.org> In-Reply-To: <20131115145452.G954@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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.17 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: Fri, 13 Dec 2013 18:46:08 -0000 Hello; Sorry for the delay in looking into this issue. On 15.11.2013 00:06, Bruce Evans wrote: > On Thu, 14 Nov 2013, Sean Bruno wrote: > >> Log: >> Repair build after svn r258115 >> >> options.get_size_type() appears to return a const char *, so assume >> that >> its a string as oppose to *nothing*. I have no idea what apple's >> code is >> trying to do here: >> >> http://opensource.apple.com/source/gperf/gperf-9/patches/size_type.patch >> > > It is trying to add an arg to a printf, while retaining the old behaviour > of printing nothing (now including the new arg) in some cases. The > compiler doesn't like this, since the new arg is unconditional while > the format is conditional. I think it is a compiler bug to warn in > this case. > > This commit replaces the warning (which is really about the compiler bug) > by a bug in gperf. But I think the bug is harmless because it is in > unreachable code. But then why have the unreachable code? (It is for > a default case which I think is unreacheable because all possible cases > that occur are handled individually.) > I decided not to merge that patch from Apple, after all it has limited (no) use at this time. For the code in current, I agree the warning is bogus and I think it would be better to revert Sean's workaround and set the WARNS level to zero. This code is unlikely to evolve further before it is removed from the tree and we won't really need extra warnings for it. Does that sound acceptable? The alternative would be, of course, just reverting the Apple patch. Regards, Pedro. >> Modified: head/contrib/gperf/src/output.cc >> ============================================================================== >> >> --- head/contrib/gperf/src/output.cc Thu Nov 14 16:10:21 2013 >> (r258138) >> +++ head/contrib/gperf/src/output.cc Thu Nov 14 18:41:58 2013 >> (r258139) >> @@ -779,7 +779,7 @@ Output::output_hash_function () const >> " register %s len;\n" : >> option[ANSIC] | option[CPLUSPLUS] ? >> "(register const char *str, register %s len)\n" : >> - "", option.get_size_type()); >> + "%s", option.get_size_type()); >> >> /* Note that when the hash function is called, it has already been >> verified >> that min_key_len <= len <= max_key_len. */ >> ... > > The old code hard-coded the type of the integer arg as "unsigned > int". This > is now spelled "%s" with a new arg. When there is no declaration at all, > then there is no integer arg and the declaration was spelled "". This is > still the correct spelling. Changing it to "%s" gives a syntax error > like > a bare "unsigned int" in the output. However, this case seems to be > unreachable. > > The printf() is rather complicated and fancily formatted, with the fancy > format partly destroyed by the svn mail bug of not quoting patches and > mail > programs then sometimes removing leading whitespace. It uses a nice > conditional ladder of the following form: > > printf( > case1 ? "decl1 %s len" : > case2 ? "decl2 %s len" : > case3 ? "decl3 %s len" : > /* default (unreachable?) */ "", typestr); > > where the formatting is much fancier than this (but is perfectly > consistent > except for the default case and now for the option at the end (the option > should be on a line by itself and not grouped with the complicated > conditional ladder, especially not with its unreachable part). > > The cases are: > > case1: K&R C > case2: plain C > case3: ANSI (sic) C or C++ > default: unreachable? > > and the generated code is now: > > K&R C: "register len'\n" > plain C: "register len;\n" > ANSI (sic) C or C++: "register len;\n" > unreachable?: "" /* syntax error */ > > Before this commit, the generated code was: > > K&R C: "register len'\n" > plain C: "register len;\n" > ANSI (sic) C or C++: "register len;\n" > unreachable?: "" /* no syntax error */ > > and a few days ago it was: > > K&R C: "register unsigned len'\n" > plain C: "register unsigned len;\n" > ANSI (sic) C or C++: "register unsigned len;\n" > unreachable?: "" /* no syntax error */ > > This particular declaration doesn't even depend on the language. The > printf() just groups it with another one for a pointer because this used > to be convenient. > > The supported languages are sort of documented in the man page. ANSI C > doesn't exist, and there have been several versions of Standard C since > it existed, but there is no special version-dependent support. It is > even less clear what plain C is. It seems to be for a 1980's C that > is not quite as old as K&R C -- according to the generated code, plain C > has const but not signed char, while K&R C has neither. I get the lack > of signed char from smallest_integral_type(). smallest_integral_type() > is fundamentally wrong since it uses the host SCHAR_MIN for building > K&R and plain C targets that don't have SCHAR_MIN and might even have > a different character size when running on the same hardware as gperf. > > Bruce