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

[Joseph Myers]

On Mon, 8 May 2017, Volker Reichelt wrote:

> 2017-05-07  Volker Reichelt  
> 
>   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.

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

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

[Volker Reichelt]

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

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  

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  

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 

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

[Joseph Myers]

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

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

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.

-- 
Joseph S. Myers
jos...@codesourcery.com

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

[Volker Reichelt]

The patch below addresses some pretty-printer issues. More precisely,
it handles 6 types of expressions that weren't handled before:

  a) EXACT_DIV_EXPR, RDIV_EXPR,
  b) LROTATE_EXPR, RROTATE_EXPR,
  c) MAX_EXPR, MIN_EXPR.

Right now we print for the testcase below:

  #'exact_div_expr' not supported by expression#'pr35441.c: In function 'foo1':
  pr35441.c:8:6: error: called object  is not a function or function pointer
  #'rdiv_expr' not supported by expression#'pr35441.c: In function 'foo2':
  pr35441.c:13:5: error: called object  is not a function or function pointer
  #'lrotate_expr' not supported by expression#'pr35441.c: In function 'foo3':
  pr35441.c:18:11: error: called object  is not a function or function pointer
  #'rrotate_expr' not supported by expression#'pr35441.c:19:11: error: called 
object  is not a function or function pointer
  #'min_expr' not supported by expression#'pr35441.c: In function 'foo4':
  pr35441.c:24:14: error: called object  is not a function or function pointer
  #'max_expr' not supported by expression#'pr35441.c:25:14: error: called 
object  is not a function or function pointer

With the patch we would print:

  pr35441.c: In function 'foo1':
  pr35441.c:8:6: error: called object '((long int)p - (long int)q) / 8' is not 
a function or function pointer
  pr35441.c: In function 'foo2':
  pr35441.c:13:5: error: called object 'x / y' is not a function or function 
pointer
  pr35441.c: In function 'foo3':
  pr35441.c:18:11: error: called object 'i << j' is not a function or function 
pointer
  pr35441.c:19:11: error: called object 'i >> j' is not a function or function 
pointer
  pr35441.c: In function 'foo4':
  pr35441.c:24:14: error: called object 'min(q,  p)' is not a function or 
function pointer
  pr35441.c:25:14: error: called object 'max(q,  p)' is not a function or 
function pointer


a) This part (with foo1 and foo2 from the testcase) is straightforward.

However, I'm not sure how to print the actual operators for the other
2 groups.

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?

c) I chose 'max(q, b)' and 'min(q, b)'.
   Would something like '__max__(q, b)' be better?
   Trying to print the expression 'p < q ? p : q' from the testcase
   might be possible. But there are several different expressions like
   'p < q ? p : q', 'p > q ? q : p', 'p >= q ? q : p' and '!(p >= q) ? p : q'
   that are all converted into MIN_EXPR, so I would rather stay with
   and explicit 'min'/'max' instead.

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.

Any comments, or is the patch OK as is for trunk?
Bootstrapped and regtested on x86_64-linux.

Regards,
Volker


2017-03-09  Volker Reichelt  

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 245962)
+++ 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,12 @@
 {
 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 == LROTATE_EXPR ? "<<" : ">>");
   pp_c_whitespace (pp);
   pp_c_additive_expression (pp, TREE_OPERAND (e, 1));
   break;
@@ -2186,6 +2199,8 @@
 case UNLT_EXPR:
 case