diff mbox

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

Message ID tkrat.6e96c75a42c3b457@netcologne.de
State New
Headers show

Commit Message

Volker Reichelt March 10, 2017, 5:59 a.m. UTC 
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  <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.

===================================================================

Comments

Joseph Myers May 2, 2017, 10:18 p.m. UTC | #1 
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>.

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

Patch

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 UNGE_EXPR:
     case UNGT_EXPR:
+    case MAX_EXPR:
+    case MIN_EXPR:
     case ABS_EXPR:
     case CONSTRUCTOR:
     case COMPOUND_LITERAL_EXPR:
@@ -2217,11 +2232,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-03-09  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" } */
+}