public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ diagnostic Patch] Partially fix c++/58363 and more
@ 2013-09-08 23:09 Paolo Carlini
  2013-09-09 13:11 ` Gabriel Dos Reis
  0 siblings, 1 reply; 2+ messages in thread
From: Paolo Carlini @ 2013-09-08 23:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Gabriel Dos Reis

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

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.

///////////////////////

[-- Attachment #2: CL --]
[-- Type: text/plain, Size: 209 bytes --]

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.

[-- Attachment #3: p --]
[-- Type: text/plain, Size: 1422 bytes --]

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;

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [C++ diagnostic Patch] Partially fix c++/58363 and more
  2013-09-08 23:09 [C++ diagnostic Patch] Partially fix c++/58363 and more Paolo Carlini
@ 2013-09-09 13:11 ` Gabriel Dos Reis
  0 siblings, 0 replies; 2+ messages in thread
From: Gabriel Dos Reis @ 2013-09-09 13:11 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-09-09 13:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-08 23:09 [C++ diagnostic Patch] Partially fix c++/58363 and more Paolo Carlini
2013-09-09 13:11 ` Gabriel Dos Reis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).