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.
Jakub
