On Tue, 18 Jul 2017, Jakub Jelinek wrote: > On Tue, Jul 18, 2017 at 10:47:37AM -0600, Martin Sebor wrote: > > On 07/18/2017 09:43 AM, Jakub Jelinek wrote: > > > On Tue, Jul 18, 2017 at 09:31:11AM -0600, Martin Sebor wrote: > > > > > --- gcc/match.pd.jj 2017-07-17 16:25:20.000000000 +0200 > > > > > +++ gcc/match.pd 2017-07-18 12:32:52.896924558 +0200 > > > > > @@ -1125,6 +1125,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > > > > && wi::neg_p (@1, TYPE_SIGN (TREE_TYPE (@1)))) > > > > > (cmp @2 @0)))))) > > > > > > > > > > +/* (X - 1U) <= INT_MAX-1U into (int) X > 0. */ > > > > > > > > Since the transformation applies to other types besides int I > > > > suggest to make it clear in the comment. E.g., something like: > > > > > > > > /* (X - 1U) <= TYPE_MAX - 1U into (TYPE) X > 0 for any integer > > > > TYPE. */ > > > > > > > > (with spaces around all the operators as per GCC coding style). > > > > > > I think many of the match.pd comments are also not fully generic > > > to describe what it does, just to give an idea what it does. > > ... > > > Examples of other comments that "suffer" from similar lack of sufficient > > > genericity, but perhaps are good enough to let somebody understand it > > > quickly: > > > > Sure, but that doesn't make them a good example to follow. As > > someone pointed out to me in code reviews, existing deviations > > from the preferred style, whether documented or not, or lack of > > clarity, aren't a license to add more. Please take my suggestion > > here in the same constructive spirit. > > The point I'm trying to make is that in order to make the > comments generic enough they will be too large and too hard to parse. > IMHO sometimes it is better to just give an example of what it > does, and those who want to read all the details on what exactly it does, > there is the simplify below it with all the details. > Consider another randomly chosen comment: > /* hypot(x,x) -> fabs(x)*sqrt(2). */ > This also isn't describing generically what it does, because > it handles not just hypot ->fabs*sqrt, but also hypotl ->fabsl*sqrtl, > hypotf ->fabsf*sqrtf, maybe others. > > In the end, it is Richard's call on what he wants to have in match.pd > comments.
Generally one example that gives an idea what the pattern does is fine, enumarating all variants is usually too noisy. When we can make them more generic without making them harder to understand I'm all for it but I have no strong opinion here (but I would ask to not simply repeat the pattern description for all cases covered). Richard.
