Re: [PATCH] Pretty-printing of some unsupported expressions (PR c/35441)

[Volker Reichelt <v.reichelt@netcologne.de>]

On  2 May, Joseph Myers wrote:
> On Fri, 10 Mar 2017, Volker Reichelt wrote:
> 
>> a) This part (with foo1 and foo2 from the testcase) is straightforward.
> 
> That part is OK.
> 
>> b) I chose the shift operators 'a << b' and 'a >> b' for the rotate
>>    expressions, which is not 100% correct. Would it be better to use
>>    something like 'lrotate(a, b)', '__lrotate__(a, b)' or 'a lrotate b'
>>    instead? Or is there something like an '__builtin_lrotate' that I misseed?
> 
> I'd be inclined to use the notation <<< and >>> for rotation, cf. 
> <https://stackoverflow.com/questions/32785998/symbol-for-bitwise-circular-shifts>.

Nice idea. It would be nice to see these operators in some future C/C++
versions. Is the nested ternary operator used in the updated patch below
OK, or would you prefer a switch?

>> c) I chose 'max(q, b)' and 'min(q, b)'.
> 
> I think that's fine.
> 
>> In addition I found some more division operators in gcc/tree.def that
>> aren't handled by the pretty-printer:
>> 
>>   CEIL_DIV_EXPR
>>   FLOOR_DIV_EXPR
>>   ROUND_DIV_EXPR
>>   CEIL_MOD_EXPR
>>   FLOOR_MOD_EXPR
>>   ROUND_MOD_EXPR
>> 
>> Alas I don't have testcases for them. Nevertheless, I could handle them
>> like the other MOD and DIV operators just to be safe.
> 
> These can probably appear from Ada code, but maybe not from C.

OK, I'll ignore them.

> Now we have caret diagnostics and location ranges I think we should be 
> moving away from printing complicated expressions from trees anyway.  So 
> for the diagnostics about calling non-functions, it would be best to make 
> a location range for the called expression available if it isn't already, 
> then do a diagnostic with a location that underlines that text rather than 
> trying to reproduce an expression text from trees.

Indeed, we can do better with caret diagnostics. But non-caret mode is
still there (and has its uses because of its usually more consise form)
and there are probably more places where this expression is printed.
Therefore, I'd like to fix this regardless of a better caret solution
for the diagnostics about calling non-functions.

Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk?

Regards,
Volker


2017-05-07  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c/35441
	* c-pretty-print.c (c_pretty_printer::expression): Handle MAX_EXPR,
	MIN_EXPR, EXACT_DIV_EXPR, RDIV_EXPR, LROTATE_EXPR, RROTATE_EXPR.
	(c_pretty_printer::postfix_expression): Handle MAX_EXPR, MIN_EXPR.
	(c_pretty_printer::multiplicative_expression): Handle EXACT_DIV_EXPR,
	RDIV_EXPR.
	(pp_c_shift_expression): Handle LROTATE_EXPR, RROTATE_EXPR.

Index: gcc/c-family/c-pretty-print.c
===================================================================
--- gcc/c-family/c-pretty-print.c	(revision 247727)
+++ gcc/c-family/c-pretty-print.c	(working copy)
@@ -1551,6 +1551,14 @@
 			   : "__builtin_islessgreater");
       goto two_args_fun;
 
+    case MAX_EXPR:
+      pp_c_ws_string (this, "max");
+      goto two_args_fun;
+
+    case MIN_EXPR:
+      pp_c_ws_string (this, "min");
+      goto two_args_fun;
+
     two_args_fun:
       pp_c_left_paren (this);
       expression (TREE_OPERAND (e, 0));
@@ -1829,6 +1837,8 @@
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case TRUNC_MOD_EXPR:
+    case EXACT_DIV_EXPR:
+    case RDIV_EXPR:
       multiplicative_expression (TREE_OPERAND (e, 0));
       pp_c_whitespace (this);
       if (code == MULT_EXPR)
@@ -1890,9 +1900,13 @@
     {
     case LSHIFT_EXPR:
     case RSHIFT_EXPR:
+    case LROTATE_EXPR:
+    case RROTATE_EXPR:
       pp_c_shift_expression (pp, TREE_OPERAND (e, 0));
       pp_c_whitespace (pp);
-      pp_string (pp, code == LSHIFT_EXPR ? "<<" : ">>");
+      pp_string (pp, code == LSHIFT_EXPR ? "<<" :
+		     code == RSHIFT_EXPR ? ">>" :
+		     code == LROTATE_EXPR ? "<<<" : ">>>");
       pp_c_whitespace (pp);
       pp_c_additive_expression (pp, TREE_OPERAND (e, 1));
       break;
@@ -2186,6 +2200,8 @@
     case UNLT_EXPR:
     case UNGE_EXPR:
     case UNGT_EXPR:
+    case MAX_EXPR:
+    case MIN_EXPR:
     case ABS_EXPR:
     case CONSTRUCTOR:
     case COMPOUND_LITERAL_EXPR:
@@ -2217,11 +2233,15 @@
     case MULT_EXPR:
     case TRUNC_MOD_EXPR:
     case TRUNC_DIV_EXPR:
+    case EXACT_DIV_EXPR:
+    case RDIV_EXPR:
       multiplicative_expression (e);
       break;
 
     case LSHIFT_EXPR:
     case RSHIFT_EXPR:
+    case LROTATE_EXPR:
+    case RROTATE_EXPR:
       pp_c_shift_expression (this, e);
       break;
 
===================================================================

2017-05-07  Volker Reichelt  <v.reichelt@netcologne.de>

	PR c/35441
	* gcc.dg/pr35441.c: New test.

Index: gcc/testsuite/gcc.dg/pr35441.c
===================================================================
--- gcc/testsuite/gcc.dg/pr35441.c	2017-03-08 18:38:39
+++ gcc/testsuite/gcc.dg/pr35441.c	2017-03-08 23:43:06
@@ -0,0 +1,26 @@
+/* PR c/35441 */
+/* { dg-do compile } */
+/* { dg-options "-fno-diagnostics-show-caret" } */
+/* { dg-bogus "not supported by" "" { target *-*-* } 0 } */
+
+void foo1(char **p, char **q)
+{
+  (p - q)();			/* { dg-error "is not a function" } */
+}
+
+void foo2(double x, double y)
+{
+  (x/y)();			/* { dg-error "is not a function" } */
+}
+
+void foo3(unsigned i, int j)
+{
+  (i << j | i >> (32 - j))();	/* { dg-error "is not a function" } */
+  (i >> j | i << (32 - j))();	/* { dg-error "is not a function" } */
+}
+
+void foo4(char *p, char *q)
+{
+  (p < q ? p : q)();		/* { dg-error "is not a function" } */
+  (p > q ? p : q)();		/* { dg-error "is not a function" } */
+}
===================================================================