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