From owner-svn-src-head@FreeBSD.ORG Fri Nov 15 05:07:01 2013 Return-Path: Delivered-To: svn-src-head@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 9FACB95B; Fri, 15 Nov 2013 05:07:01 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 4899D29C4; Fri, 15 Nov 2013 05:07:00 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 33F37780E7C; Fri, 15 Nov 2013 16:06:52 +1100 (EST) Date: Fri, 15 Nov 2013 16:06:42 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Sean Bruno Subject: Re: svn commit: r258139 - head/contrib/gperf/src In-Reply-To: <201311141841.rAEIfwFU040620@svn.freebsd.org> Message-ID: <20131115145452.G954@besplex.bde.org> References: <201311141841.rAEIfwFU040620@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=YYGEuWhf c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=yIfHJzyiBScA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=XB2iG6U_zloA:10 a=t-IPkPogAAAA:8 a=wX8kGpdw02fNGXW0OdoA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Nov 2013 05:07:01 -0000 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.) > 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