diff mbox

[C++,diagnostic] Partially fix c++/58363 and more

Message ID 522CDD90.9030708@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Sept. 8, 2013, 8:26 p.m. UTC
Hi all, Gaby,

I was having a look to c++/58363 and besides the main issue, that is we 
probably want to help the user and tell him/her something about 
destructors, etc, I noticed that we aren't able to pretty print the 
pseudo destructor expression at issue:

cannot convert ‘f.#‘var_decl’ not supported by dump_type#<type 
error>::~’ (type ‘void’) to type ‘int’

Weird. Thus I went to cp-tree.def and found the very clear comment:

/* A pseudo-destructor, of the form "OBJECT.~DESTRUCTOR" or
"OBJECT.SCOPE::~DESTRUCTOR. The first operand is the OBJECT. The
second operand (if non-NULL) is the SCOPE. The third operand is
the TYPE node corresponding to the DESTRUCTOR.

which in fact is inconsistent with the code in error.c:dump_expr. As 
regards cxx-pretty-print.c, the code in postfix_expression seems largely 
Ok (that confirmed my analysis), only I don't think the case of NULL 
second operand is handled correctly. What do you think about the below? 
Certainly passes the testsuite and the pretty printing for 58363 is Ok.

Thanks!
Paolo.

///////////////////////
2013-09-08  Paolo Carlini  <paolo.carlini@oracle.com>

	* error.c (dump_expr, [PSEUDO_DTOR_EXPR]): Fix.
	* cxx-pretty-print.c (cxx_pretty_printer::postfix_expression):
	Tweak, TREE_OPERAND (t, 1) may be null.

Comments

Gabriel Dos Reis Sept. 9, 2013, 1:02 p.m. UTC | #1
On Sun, Sep 8, 2013 at 3:26 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi all, Gaby,
>
> I was having a look to c++/58363 and besides the main issue, that is we
> probably want to help the user and tell him/her something about destructors,
> etc, I noticed that we aren't able to pretty print the pseudo destructor
> expression at issue:
>
> cannot convert ‘f.#‘var_decl’ not supported by dump_type#<type error>::~’
> (type ‘void’) to type ‘int’
>
> Weird. Thus I went to cp-tree.def and found the very clear comment:
>
> /* A pseudo-destructor, of the form "OBJECT.~DESTRUCTOR" or
> "OBJECT.SCOPE::~DESTRUCTOR. The first operand is the OBJECT. The
> second operand (if non-NULL) is the SCOPE. The third operand is
> the TYPE node corresponding to the DESTRUCTOR.
>
> which in fact is inconsistent with the code in error.c:dump_expr. As regards
> cxx-pretty-print.c, the code in postfix_expression seems largely Ok (that
> confirmed my analysis), only I don't think the case of NULL second operand
> is handled correctly. What do you think about the below? Certainly passes
> the testsuite and the pretty printing for 58363 is Ok.

Looks good.  OK to commit.

I have been hoping that we would have ditched dump_expr and consorts, in
favor of the C++ specific pretty printers, but now…

-- Gaby
diff mbox

Patch

Index: error.c
===================================================================
--- error.c	(revision 202365)
+++ error.c	(working copy)
@@ -2472,12 +2472,15 @@  dump_expr (cxx_pretty_printer *pp, tree t, int fla
       break;
 
     case PSEUDO_DTOR_EXPR:
-      dump_expr (pp, TREE_OPERAND (t, 2), flags);
+      dump_expr (pp, TREE_OPERAND (t, 0), flags);
       pp_cxx_dot (pp);
-      dump_type (pp, TREE_OPERAND (t, 0), flags);
-      pp_cxx_colon_colon (pp);
+      if (TREE_OPERAND (t, 1))
+	{
+	  dump_type (pp, TREE_OPERAND (t, 1), flags);
+	  pp_cxx_colon_colon (pp);
+	}
       pp_cxx_complement (pp);
-      dump_type (pp, TREE_OPERAND (t, 1), flags);
+      dump_type (pp, TREE_OPERAND (t, 2), flags);
       break;
 
     case TEMPLATE_ID_EXPR:
Index: cxx-pretty-print.c
===================================================================
--- cxx-pretty-print.c	(revision 202365)
+++ cxx-pretty-print.c	(working copy)
@@ -618,8 +618,11 @@  cxx_pretty_printer::postfix_expression (tree t)
     case PSEUDO_DTOR_EXPR:
       postfix_expression (TREE_OPERAND (t, 0));
       pp_cxx_dot (this);
-      pp_cxx_qualified_id (this, TREE_OPERAND (t, 1));
-      pp_cxx_colon_colon (this);
+      if (TREE_OPERAND (t, 1))
+	{
+	  pp_cxx_qualified_id (this, TREE_OPERAND (t, 1));
+	  pp_cxx_colon_colon (this);
+	}
       pp_complement (this);
       pp_cxx_unqualified_id (this, TREE_OPERAND (t, 2));
       break;