[wwwdocs] Update c++-concepts and var-template branches info
This patch updates the maintainership of the c++-concepts and var-template branches. -- Gaby Index: svn.html === RCS file: /cvs/gcc/wwwdocs/htdocs/svn.html,v retrieving revision 1.188 diff -p -r1.188 svn.html *** svn.html18 Aug 2013 20:59:25 - 1.188 --- svn.html14 Sep 2013 00:41:54 - *** the command codesvn log --stop-on-copy *** 542,554 dtc++-concepts/dt ddThis is the sandbox for renewed work on emconcepts for C++/em. ! The branch is maintained by ! a href=mailto:g...@gcc.gnu.org;Gabriel Dos Reis/a./dd dtvar-template/dt ddThis branch is for implementation work on ! emvariable template for C++/em. It is maintained by ! a href=mailto:g...@gcc.gnu.org;Gabriel Dos Reis/a./dd dtcilkplus/dt ddThis branch is for the development of --- 542,556 dtc++-concepts/dt ddThis is the sandbox for renewed work on emconcepts for C++/em. ! It was originally created by Gabriel Dos Reis, with implementation ! contributed by Andrew Sutton. It is currently maintained by ! a href=mailto:ja...@gcc.gnu.org;Jason Merrill/a./dd dtvar-template/dt ddThis branch is for implementation work on ! emvariable template for C++/em. It was originally !created by Gabriel Dos Reis. It is maintained by ! a href=mailto:ja...@gcc.gnu.org;Jason Merrill/a./dd dtcilkplus/dt ddThis branch is for the development of
Remove myself from MAINTAINERS
Applied to trunk. -- Gaby Index: MAINTAINERS === --- MAINTAINERS (revision 202586) +++ MAINTAINERS (working copy) @@ -195,7 +195,6 @@ dwarf debugging code Jason Merrill ja...@redhat.com dwarf debugging code Cary Coutantccout...@google.com c++ runtime libs Paolo Carlini paolo.carl...@oracle.com -c++ runtime libs Gabriel Dos Reisg...@integrable-solutions.net c++ runtime libs Ulrich Drepper drep...@gmail.com c++ runtime libs Benjamin De Kosnik b...@gnu.org c++ runtime libs Loren J. Rittle ljrit...@acm.org @@ -214,7 +213,6 @@ basic block reordering Jason Eckhardt j...@rice.edu i18n Philipp Thomas p...@suse.de i18n Joseph Myersjos...@codesourcery.com -diagnostic messagesGabriel Dos Reisg...@integrable-solutions.net build machinery (*.in) Paolo Bonzini bonz...@gnu.org build machinery (*.in) DJ Delorie d...@redhat.com build machinery (*.in) Nathanael Nerodenero...@gcc.gnu.org
Re: [c++-concepts] template parameter constraints
Andrew Sutton andrew.n.sut...@gmail.com writes: | I added a new macro to replace the use of TREE_TYPE to get | constraints. It's called TEMPLATE_PARMS_CONSTRAINTS. Patch attached: | | 2013-09-10 Andrew Sutton andrew.n.sut...@gmail.com | * gcc/cp/cp-tree.h (TEMPLATE_PARMS_CONSTRAINTS): New. | * gcc/cp/parser.c (cp_parser_template_declaration_after_export), | (cp_parser_type_parameter): Use TEMPLATE_PARMS_CONSTRAINTS. | * gcc/cp/semantics.c (fixup_template_scope): Use | TEMPLATE_PARMS_CONSTRAINTS. | +// Access template constraints associated with the template | +// parameter lists. Template parameter constraints are stored in | +// the TREE_TYPE of list. Hmm, we can have several levels of template parameter list. What about: // Logical constraints on the template parameters introduced at a // given template parameter list level indicated by NODE. Patch OK with that change. Thanks, -- Gaby
Re: [c++-concepts] Class template constraints
Andrew Sutton andrew.n.sut...@gmail.com writes: | Ok to commit? Attached is the doc fix patch. I'll send the TREE_TYPE | patch shortly. Yes, please -- that is what Jason earlier. Let me know when youve committed so that I can synchronize with trunk. -- Gaby
Re: operator new returns nonzero
On Mon, Sep 9, 2013 at 4:06 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Sat, 7 Sep 2013, Mike Stump wrote:
On Sep 7, 2013, at 12:27 PM, Marc Glisse marc.gli...@inria.fr wrote:
Now flag_check_new should probably disable this optimization…
Yes, this why my point.
Ok, here it is (again, no proper testing until bootstrap is fixed)
I wonder what happens on targets where 0 is a valid address of an object
(stated by !flag_delete_null_pointer_checks)?
We should distinguish between front-end notion of null pointer,
and backend notion of address zero. The language, and therefore
the front-end, does not allow an object to have 'this' value to be
a null pointer, nor does is allow 'operator new' to return null pointer.
Consequently, if we have a target (which ones?) where zero is
a valid address for an object, that target should take precaution
to satisfy the requirement of the language.
-- Gaby
Richard.
2013-09-07 Marc Glisse marc.gli...@inria.fr
PR c++/19476
gcc/
* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
* tree-vrp.c (gimple_stmt_nonzero_warnv_p,
stmt_interesting_for_vrp):
Likewise.
(vrp_visit_stmt): Remove duplicated code.
gcc/testsuite/
* g++.dg/tree-ssa/pr19476-1.C: New file.
* g++.dg/tree-ssa/pr19476-2.C: Likewise.
* g++.dg/tree-ssa/pr19476-3.C: Likewise.
--
Marc Glisse
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-1.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C (revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options -O -fdump-tree-ccp1 } */
+
+#include new
+
+int f(){
+ return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+ return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump return 42 ccp1 } } */
+/* { dg-final { scan-tree-dump-not return 33 ccp1 } } */
+/* { dg-final { cleanup-tree-dump ccp1 } } */
Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___
Added: svn:keywords
+ Author Date Id Revision URL
Added: svn:eol-style
+ native
Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-2.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-tree-optimized } */
+
+#include new
+
+int f(){
+ int *p = new(std::nothrow) int;
+ return 33 + (0 == p);
+}
+int g(){
+ int *p = new int[50];
+ return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump return 42 optimized } } */
+/* { dg-final { scan-tree-dump-not return 33 optimized } } */
+/* { dg-final { cleanup-tree-dump optimized } } */
Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___
Added: svn:keywords
+ Author Date Id Revision URL
Added: svn:eol-style
+ native
Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===
--- testsuite/g++.dg/tree-ssa/pr19476-3.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C (revision 0)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options -O3 -fcheck-new -fdump-tree-optimized } */
+
+#include new
+
+int g(){
+ return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not return 42 optimized } } */
+/* { dg-final { cleanup-tree-dump optimized } } */
Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___
Added: svn:keywords
+ Author Date Id Revision URL
Added: svn:eol-style
+ native
Index: fold-const.c
===
--- fold-const.c(revision 202351)
+++ fold-const.c(working copy)
@@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
case MODIFY_EXPR:
case BIND_EXPR:
return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
strict_overflow_p);
case SAVE_EXPR:
return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
strict_overflow_p);
case CALL_EXPR:
- return alloca_call_p (t);
+ {
+ tree fn = CALL_EXPR_FN (t);
+ if (TREE_CODE (fn) != ADDR_EXPR) return false;
+ tree fndecl = TREE_OPERAND (fn, 0);
+ if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+ if (!flag_check_new
+DECL_IS_OPERATOR_NEW (fndecl)
+!TREE_NOTHROW (fndecl))
+ return true;
+ return alloca_call_p (t);
Re: operator new returns nonzero
On Mon, Sep 9, 2013 at 4:19 AM, Marc Glisse marc.gli...@inria.fr wrote: On Mon, 9 Sep 2013, Richard Biener wrote: On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse marc.gli...@inria.fr wrote: On Sat, 7 Sep 2013, Mike Stump wrote: On Sep 7, 2013, at 12:27 PM, Marc Glisse marc.gli...@inria.fr wrote: Now flag_check_new should probably disable this optimization… Yes, this why my point. Ok, here it is (again, no proper testing until bootstrap is fixed) I wonder what happens on targets where 0 is a valid address of an object (stated by !flag_delete_null_pointer_checks)? I am not at all familiar with those targets (I thought you had to write asm to access 0 so the compiler doesn't mess with your code), but it makes sense to me to test (flag_delete_null_pointer_checks !flag_check_new) instead of just !flag_check_new. Consider the patch changed this way. (we have so many options, I wouldn't be surprised if there is yet another one to check…) If we have such a target (do we?) where 0 is a valid address of an object, I would not be surprised if it breaks in so many other ways, since the language explicitly states that can never happen (programmers write code depending on that). -- Marc Glisse
Re: [Patch to gcc/function] PR 58362
On Mon, Sep 9, 2013 at 4:46 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, Sep 09, 2013 at 11:45:08AM +0200, Richard Biener wrote: Well, in this case the patch should IMHO be a no-op. - warning (OPT_Wunused_parameter, unused parameter %q+D, decl); + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wunused_parameter, + unused parameter %qD, decl); no? Unless I misunderstand what %q+D should do. The question is how exactly is %q+D defined, if it is warning_at (location_of (decl), OPT_Wunused_parameter, unused parameter %qD, decl); in this case, or DECL_SOURCE_LOCATION (decl) instead. The semantics of '%+D' was defined long before I got involved. The way it was supposed to work is that we pick the location of the decl being specified, instead of taking the current location. When we figured that was insufficient, we introduced %H to say: pick this location. For that reason, one can only have on +D in a diagnostic message (I don't think we -- Gaby
Re: [Patch to gcc/function] PR 58362
On Mon, Sep 9, 2013 at 5:04 AM, Richard Biener rguent...@suse.de wrote: On Mon, 9 Sep 2013, Jakub Jelinek wrote: On Mon, Sep 09, 2013 at 11:45:08AM +0200, Richard Biener wrote: Well, in this case the patch should IMHO be a no-op. - warning (OPT_Wunused_parameter, unused parameter %q+D, decl); + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wunused_parameter, + unused parameter %qD, decl); no? Unless I misunderstand what %q+D should do. The question is how exactly is %q+D defined, if it is warning_at (location_of (decl), OPT_Wunused_parameter, unused parameter %qD, decl); in this case, or DECL_SOURCE_LOCATION (decl) instead. It can't be 'location_of' because that's a C++ FE speciality but warning_at and %q+D are diagnostic machinery level. %+D was, and has for long, been a C++ FE-specific marker. I don't recall when we decided to make that available to all front-ends. Unless of course the meaning of %q+D depends on the frontend which would make its use from the middle-end ill-defined. We introduced xxx_at so that a different locus (different from the decl and different from current locus, if ever defined) can be specified. -- Gaby
Re: [Patch to gcc/function] PR 58362
On Mon, Sep 9, 2013 at 5:13 AM, Richard Biener rguent...@suse.de wrote: On Mon, 9 Sep 2013, Paolo Carlini wrote: On 09/09/2013 12:04 PM, Richard Biener wrote: On Mon, 9 Sep 2013, Jakub Jelinek wrote: On Mon, Sep 09, 2013 at 11:45:08AM +0200, Richard Biener wrote: Well, in this case the patch should IMHO be a no-op. - warning (OPT_Wunused_parameter, unused parameter %q+D, decl); + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wunused_parameter, + unused parameter %qD, decl); no? Unless I misunderstand what %q+D should do. The question is how exactly is %q+D defined, if it is warning_at (location_of (decl), OPT_Wunused_parameter, unused parameter %qD, decl); in this case, or DECL_SOURCE_LOCATION (decl) instead. It can't be 'location_of' because that's a C++ FE speciality but warning_at and %q+D are diagnostic machinery level. Everything happens via call backs. Thus from the generic diagnostic machinery, you go to cp_printer for C++, thus location_of for C++. In C is different, but again there is, evidently, a mechanism which uses DECL_CONTEXT for PARM_DECLs which leads to an inaccurate location when we *really* want the location of the parameter (exactly as I explained for C++). I understand that. But I question it. Why would that ever be useful? Can't the places that want that simply use warning/error_at with the proper location? I agree with Richard that if you want a locus that is not the current locus, and is not the locus of the decl, you should use the _at version. -- Gaby
Re: [Patch to gcc/function] PR 58362
On Mon, Sep 9, 2013 at 5:38 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 09/09/2013 11:37 AM, Richard Biener wrote: That said, grepping for %q+D reveals quite some uses and it looks like all of them expect the location being used to be that of the decl passed to the diagnostic call, not some random other location. If the decl is *not* a PARM_DECL, I expect %q+D to be often accurate. In fact, even when *is* a PARM_DECL what we have now is pretty decent, because normally the location of the corresponding FUNCTION_DECL isn't that far. The point is whether we want to be *more* accurate and point to the specific unused parameter, for C and C++, as clang and icc do. I think the logic is simpler if we use the xxx_at form in these cases. -- Gaby
Re: [Patch to gcc/function] PR 58362
On Mon, Sep 9, 2013 at 5:41 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, Sep 09, 2013 at 12:38:46PM +0200, Paolo Carlini wrote: On 09/09/2013 11:37 AM, Richard Biener wrote: That said, grepping for %q+D reveals quite some uses and it looks like all of them expect the location being used to be that of the decl passed to the diagnostic call, not some random other location. If the decl is *not* a PARM_DECL, I expect %q+D to be often accurate. In fact, even when *is* a PARM_DECL what we have now is pretty decent, because normally the location of the corresponding FUNCTION_DECL isn't that far. The point is whether we want to be *more* accurate and point to the specific unused parameter, for C and C++, as clang and icc do. I guess the primary question is why location_of special cases the PARM_DECL and in which case it is useful to do so, and whether the number of cases (if any) when it is useful to do so is bigger than the number of place when it is undesirable. Most likely historical reason, the exact sequence of which is lost to history. -- Gaby
Re: [C++, diagnostic Patch] PR 58362
On Mon, Sep 9, 2013 at 7:49 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi all, hi Gaby, thus a more sensible try at fixing this issue: function.c:do_warn_unused_parameter uses %q+D for the error call. That means that cp/error.c:cp_printer does: if (set_locus t != NULL) *text-locus = location_of (t); and it's important that location_of (t) is correct. However, for PARM_DECLs, location_of (still) does: if (TREE_CODE (t) == PARM_DECL DECL_CONTEXT (t)) t = DECL_CONTEXT (t); else if ... ... which means that for those we end up with the location of a FUNCTION_DECL or anyway not exactly the location of the PARM_DECL itself. I went through the C++ front-end and currently there are very few cases if any where location_of may be fed a PARM_DECL, and the very few seem Ok to me. The testsuite passes on x86_64-linux of course, thus I'm proposing the below. IMHO now that we are in Stage 1 the risk of breaking locations somewhere else is low enough. By the way, as discussed today elsewhere, the C front-end is already Ok. Patch OK; thanks! -- Gaby
Re: [C++ diagnostic Patch] Partially fix c++/58363 and more
On Sun, Sep 8, 2013 at 3:26 PM, Paolo Carlini paolo.carl...@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
[c++-concepts] merge from trunk
at revision 202396. No conflict. -- Gaby
Re: operator new returns nonzero
On Mon, Sep 9, 2013 at 11:16 AM, Mike Stump mikest...@comcast.net wrote: You've failed to understand g++. Please seek to understand the compiler, before you weigh in. Gee Mike, see a medical assistance for your and our benefits. -- Gaby
Re: [c++-concepts] pretty print fix
Andrew Sutton andrew.n.sut...@gmail.com writes: | The last merge didn't compile for me. Apparently some pretty printing | functions have disappeared that were being called in the concept | stuff. This patch just replaces the broken calls to pp_cxx_type_id | with pp-type_id. | | Ok to commit? yes, please. Sorry for that. -- Gaby
Re: operator new returns nonzero
On Mon, Sep 9, 2013 at 12:54 PM, Mike Stump mikest...@comcast.net wrote: On Sep 9, 2013, at 10:25 AM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Mon, Sep 9, 2013 at 11:16 AM, Mike Stump mikest...@comcast.net wrote: You've failed to understand g++. Please seek to understand the compiler, before you weigh in. Gee Mike, see a medical assistance for your and our benefits. Seems overly hostile to me. Funny you would say that. -- Gaby
Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...
On Sun, Sep 8, 2013 at 4:36 AM, Paolo Carlini paolo.carl...@oracle.com wrote: .. it's r202296 aka bootstrap/58340, Jeff is already on it. Thanks for the detective work, Paolo. -- Gaby
Re: [diagnostic patch] PR 54941
On Sun, Sep 8, 2013 at 8:00 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi all, Gaby, in this bug Manuel noticed that the zeros in the diagnostic lines of the form: built-in:0:0: don't provide useful information. Thus the below just avoids printing the zeros basing directly on the file name: admittedly, it may seem a bit gross, but in practice the strcmp would often fail early, and should be rather efficient anyway because the second argument is known at compile-time. For builtins, we probably do not want to print the line number and the column number (both of which are zero). Tested x86_64-linux. Thanks, Paolo.
Re: Add varpool node removal/insertion hooks
On Sun, Sep 8, 2013 at 9:23 AM, Jan Hubicka hubi...@ucw.cz wrote:
Hi,
this patch adds API for varpool node removal/insertion hooks that are fully
symmetric
to cgraph code.
Bootstrapped/regtested x86_64-linux after working around PR58340, will commit
it shortly.
Hi Honza,
I can't bootstrap trunk at the moment on x86_64-linux. How did you
get past that?
-- Gaby
Honza
* cgraph.h (varpool_node_hook, varpool_node_hook_list,
varpool_add_node_removal_hook, varpool_add_variable_insertion_hook,
varpool_remove_variable_insertion_hook): Declare.
* varpool.c (varpool_node_hook_list): New structure.
(first_varpool_node_removal_hook,
first_varpool_variable_insertion_hook): New variables.
(varpool_add_node_removal_hook, varpool_remove_node_removal_hook,
varpool_call_node_removal_hooks, varpool_add_variable_insertion_hook,
varpool_remove_variable_insertion_hook,
varpool_call_variable_insertion_hooks): New functions.
(varpool_remove_node): Use it.
Index: cgraph.h
===
--- cgraph.h(revision 202364)
+++ cgraph.h(working copy)
@@ -701,12 +701,14 @@ void cgraph_mark_address_taken_node (str
typedef void (*cgraph_edge_hook)(struct cgraph_edge *, void *);
typedef void (*cgraph_node_hook)(struct cgraph_node *, void *);
+typedef void (*varpool_node_hook)(struct varpool_node *, void *);
typedef void (*cgraph_2edge_hook)(struct cgraph_edge *, struct cgraph_edge *,
void *);
typedef void (*cgraph_2node_hook)(struct cgraph_node *, struct cgraph_node *,
void *);
struct cgraph_edge_hook_list;
struct cgraph_node_hook_list;
+struct varpool_node_hook_list;
struct cgraph_2edge_hook_list;
struct cgraph_2node_hook_list;
struct cgraph_edge_hook_list *cgraph_add_edge_removal_hook
(cgraph_edge_hook, void *);
@@ -714,9 +716,15 @@ void cgraph_remove_edge_removal_hook (st
struct cgraph_node_hook_list *cgraph_add_node_removal_hook (cgraph_node_hook,
void *);
void cgraph_remove_node_removal_hook (struct cgraph_node_hook_list *);
+struct varpool_node_hook_list *varpool_add_node_removal_hook
(varpool_node_hook,
+ void *);
+void varpool_remove_node_removal_hook (struct varpool_node_hook_list *);
struct cgraph_node_hook_list *cgraph_add_function_insertion_hook
(cgraph_node_hook,
void *);
void cgraph_remove_function_insertion_hook (struct cgraph_node_hook_list *);
+struct varpool_node_hook_list *varpool_add_variable_insertion_hook
(varpool_node_hook,
+ void *);
+void varpool_remove_variable_insertion_hook (struct varpool_node_hook_list
*);
void cgraph_call_function_insertion_hooks (struct cgraph_node *node);
struct cgraph_2edge_hook_list *cgraph_add_edge_duplication_hook
(cgraph_2edge_hook, void *);
void cgraph_remove_edge_duplication_hook (struct cgraph_2edge_hook_list *);
Index: varpool.c
===
--- varpool.c (revision 202364)
+++ varpool.c (working copy)
@@ -36,6 +36,100 @@ along with GCC; see the file COPYING3.
#include tree-flow.h
#include flags.h
+/* List of hooks triggered on varpool_node events. */
+struct varpool_node_hook_list {
+ varpool_node_hook hook;
+ void *data;
+ struct varpool_node_hook_list *next;
+};
+
+/* List of hooks triggered when a node is removed. */
+struct varpool_node_hook_list *first_varpool_node_removal_hook;
+/* List of hooks triggered when an variable is inserted. */
+struct varpool_node_hook_list *first_varpool_variable_insertion_hook;
+
+/* Register HOOK to be called with DATA on each removed node. */
+struct varpool_node_hook_list *
+varpool_add_node_removal_hook (varpool_node_hook hook, void *data)
+{
+ struct varpool_node_hook_list *entry;
+ struct varpool_node_hook_list **ptr = first_varpool_node_removal_hook;
+
+ entry = (struct varpool_node_hook_list *) xmalloc (sizeof (*entry));
+ entry-hook = hook;
+ entry-data = data;
+ entry-next = NULL;
+ while (*ptr)
+ptr = (*ptr)-next;
+ *ptr = entry;
+ return entry;
+}
+
+/* Remove ENTRY from the list of hooks called on removing nodes. */
+void
+varpool_remove_node_removal_hook (struct varpool_node_hook_list *entry)
+{
+ struct varpool_node_hook_list **ptr = first_varpool_node_removal_hook;
+
+ while (*ptr != entry)
+ptr = (*ptr)-next;
+ *ptr = entry-next;
+ free (entry);
+}
+
+/* Call all node removal hooks. */
+static void
+varpool_call_node_removal_hooks (struct varpool_node *node)
+{
+ struct varpool_node_hook_list *entry = first_varpool_node_removal_hook;
+
Re: Add varpool node removal/insertion hooks
On Sun, Sep 8, 2013 at 9:58 AM, Jan Hubicka hubi...@ucw.cz wrote: On Sun, Sep 8, 2013 at 9:23 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, this patch adds API for varpool node removal/insertion hooks that are fully symmetric to cgraph code. Bootstrapped/regtested x86_64-linux after working around PR58340, will commit it shortly. Hi Honza, I can't bootstrap trunk at the moment on x86_64-linux. How did you get past that? I attached my workaround to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58340 Honza Thank you. -- Gaby
Re: operator new returns nonzero
On Sat, Sep 7, 2013 at 6:44 AM, Basile Starynkevitch bas...@starynkevitch.net wrote: On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote: Hello, this patch teaches the compiler that operator new, when it can throw, isn't allowed to return a null pointer. Note that it doesn't work for placement new (that one may have to be handled in the front-end or the inliner), and it will not work on user-defined operator new if they are inlined. I guess it would be possible to teach the inliner to insert an assert_expr or something when inlining such a function, but that's not for now. The code I removed from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and thus useless. Interesting patch. However, I feel that we should give advanced users the ability to say that their new operator is never returning null... In particular, if I define my own new operator which never returns nil, I find strange that it would be less optimized than the system's operator new. Perhaps we need an attribute `nonnullresult' which whould means that. (we already the nonnull attribute for function arguments) 'operator new' is a replaceable function, so when you define it, you have to abide by the standard semantics. -- Gaby
Re: operator new returns nonzero
On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse marc.gli...@inria.fr wrote:
On Sat, 7 Sep 2013, Marc Glisse wrote:
this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer. Note that it doesn't work for
placement new (that one may have to be handled in the front-end or the
inliner),
Placement new is a completely different thing. For one, it is nothrow (so
the test doesn't apply). But mostly, it is a condition on the argument more
than the result. Using the nonnull attribute would make sense, but the
compiler doesn't seem clever enough to use that information. The easiest
seems to be in the library:
--- o/new 2013-09-07 11:11:17.388756320 +0200
+++ i/new 2013-09-07 18:00:32.204797291 +0200
@@ -144,9 +144,9 @@
// Default placement versions of operator new.
inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }
inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }
// Default placement versions of operator delete.
inline void operator delete (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
Though I am not sure what the policy is for this kind of thing. And that's
assuming it is indeed undefined to pass a null pointer to it, I don't have a
good reference for that.
and it will not work on user-defined operator new if they are inlined.
But then if that operator new is inlined, it may already contain a line like
if(p==0)throw(); which lets the compiler know all it needs.
I am not sure I like this version of the patch.
I think the appropriate patch should be in the compiler, not
here. C++ has several places where certain parameters cannot
have non-null values. For example, the implicit parameter 'this'
in non-static member functions. This is another instance.
Furthermore, I do think that the compiler should have special nodes
for both standard placement new and the global operator new functions,
as I explained in previous messages.
-- Gaby
I guess it would be possible to teach the inliner to insert an assert_expr
or something when inlining such a function, but that's not for now. The code
I removed from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and
thus useless.
I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper
testing when trunk bootstraps again.
2013-09-07 Marc Glisse marc.gli...@inria.fr
PR c++/19476
gcc/
* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
* tree-vrp.c (gimple_stmt_nonzero_warnv_p,
stmt_interesting_for_vrp):
Likewise.
(vrp_visit_stmt): Remove duplicated code.
gcc/testsuite/
* g++.dg/tree-ssa/pr19476-1.C: New file.
* g++.dg/tree-ssa/pr19476-2.C: Likewise.
--
Marc Glisse
Re: operator new returns nonzero
On Sat, Sep 7, 2013 at 12:59 PM, Marc Glisse marc.gli...@inria.fr wrote: Furthermore, I do think that the compiler should have special nodes for both standard placement new and the global operator new functions, That's one way to do it. Since this is the first time I look at those, I don't really see the advantage compared to the current status, but I trust you. What would you do with this special-node placement new? placement new really is about calling a contractor, and marking the beginning of the lifetime of a new object, hence aiding lifetime-based alias analysis. Keep it as is until after vrp so we can use the !=0 property and then expand it to its first argument? Or expand it early to the equivalent of the library code I wrote above? as I explained in previous messages. I couldn't find them, sorry if they contained information that makes this post irrelevant. http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01276.html http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01291.html -- Gaby
Re: operator new returns nonzero
On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Sat, 7 Sep 2013, Mike Stump wrote:
On Sep 7, 2013, at 3:33 AM, Marc Glisse marc.gli...@inria.fr wrote:
this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer.
You sure:
@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated. This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option. In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}. See also
@samp{new (nothrow)}.
?
Thanks, I didn't know that option. But it doesn't do the same.
Indeed.
-- Gaby
Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...
On Sat, Sep 7, 2013 at 3:41 PM, Paolo Carlini paolo.carl...@oracle.com wrote: -- Caroline, something seems wrong with the patch, I can't build anymore. Something in libvtv/testsuite: make[4]: Entering directory `/home/paolo/Gcc/svn-dirs/trunk-build/x86_64-unknown-linux-gnu/libvtv/testsuite' Makefile:369: *** missing separator. Stop. Paolo. Indeed. I saw this last night. With --disable-libvtv, I was stopped by: /home/gdr/build/gcc/./prev-gcc/xg++ -B/home/gdr/build/gcc/./prev-gcc/ -B/home/gdr/gnu/x86_64-unknown-linux-gnu/bin/ -nostdinc++ -B/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include -I/home/gdr/src/gcc.svn/libstdc++-v3/libsupc++ -L/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/gdr/build/gcc/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -c -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Icp -I/home/gdr/src/gcc.svn/gcc -I/home/gdr/src/gcc.svn/gcc/cp -I/home/gdr/src/gcc.svn/gcc/../include -I/home/gdr/src/gcc.svn/gcc/../libcpp/include -I/home/gdr/src/gcc.svn/gcc/../libdecnumber -I/home/gdr/src/gcc.svn/gcc/../libdecnumber/bid -I../libdecnumber -I/home/gdr/src/gcc.svn/gcc/../libbacktrace /home/gdr/src/gcc.svn/gcc/cp/typeck.c -o cp/typeck.o /home/gdr/src/gcc.svn/gcc/cp/pt.c: In function 'tree_node* maybe_get_template_decl_from_type_decl(tree)': /home/gdr/src/gcc.svn/gcc/cp/pt.c:7064:1: internal compiler error: in propagate_threaded_block_debug_into, at tree-ssa-threadedge.c:623 maybe_get_template_decl_from_type_decl (tree decl) ^ 0x528a20 ??? ../sysdeps/x86_64/start.S:123 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See http://gcc.gnu.org/bugs.html for instructions. make[3]: *** [cp/pt.o] Error 1 -- Gaby
Re: operator new returns nonzero
On Sat, Sep 7, 2013 at 2:46 PM, Mike Stump mikest...@comcast.net wrote:
On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Sat, 7 Sep 2013, Mike Stump wrote:
On Sep 7, 2013, at 3:33 AM, Marc Glisse marc.gli...@inria.fr wrote:
this patch teaches the compiler that operator new, when it can throw,
isn't allowed to return a null pointer.
You sure:
@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated. This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option. In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}. See also
@samp{new (nothrow)}.
?
Thanks, I didn't know that option. But it doesn't do the same.
Indeed.
Can this throw:
void *operator new (long unsigned int s) {
return 0;
}
? Is this allowed to return 0?
If that is supposed to be a definition for the global function 'operator new',
then it fails the requirement of the C++ standards since 1998.
-- Gaby
Clean up pretty printers [21/n]
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-09-04 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::simple_type_specifier): Now
a virtual member function.
(pp_simple_type_specifier): Remove.
(pp_c_type_specifier): Likewise.
* c-pretty-print.c (c_pretty_printer::simple_type_specifier):
Rename from pp_c_type_specifier. Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
simple_type_specifier.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::simple_type_specifier):
Declare as overrider.
* cxx-pretty-print.c (cxx_pretty_printer::simple_type_specifier):
Rename from pp_cxx_simple_type_specifier.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
simple_type_specifier.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 202240)
+++ c-family/c-pretty-print.c (working copy)
@@ -305,7 +305,10 @@
}
}
-/* type-specifier:
+/* simple-type-specifier:
+ type-specifier
+
+ type-specifier:
void
char
short
@@ -328,17 +331,17 @@
__vector__ */
void
-pp_c_type_specifier (c_pretty_printer *pp, tree t)
+c_pretty_printer::simple_type_specifier (tree t)
{
const enum tree_code code = TREE_CODE (t);
switch (code)
{
case ERROR_MARK:
- pp-translate_string (type-error);
+ translate_string (type-error);
break;
case IDENTIFIER_NODE:
- pp_c_identifier (pp, IDENTIFIER_POINTER (t));
+ pp_c_identifier (this, IDENTIFIER_POINTER (t));
break;
case VOID_TYPE:
@@ -349,7 +352,7 @@
if (TYPE_NAME (t))
{
t = TYPE_NAME (t);
- pp_c_type_specifier (pp, t);
+ simple_type_specifier (t);
}
else
{
@@ -360,11 +363,11 @@
t = c_common_type_for_mode (TYPE_MODE (t), TYPE_UNSIGNED (t));
if (TYPE_NAME (t))
{
- pp_c_type_specifier (pp, t);
+ simple_type_specifier (t);
if (TYPE_PRECISION (t) != prec)
{
- pp_colon (pp);
- pp_decimal_int (pp, prec);
+ pp_colon (this);
+ pp_decimal_int (this, prec);
}
}
else
@@ -372,52 +375,52 @@
switch (code)
{
case INTEGER_TYPE:
- pp-translate_string (TYPE_UNSIGNED (t)
-? unnamed-unsigned:
-: unnamed-signed:);
+ translate_string (TYPE_UNSIGNED (t)
+? unnamed-unsigned:
+: unnamed-signed:);
break;
case REAL_TYPE:
- pp-translate_string (unnamed-float:);
+ translate_string (unnamed-float:);
break;
case FIXED_POINT_TYPE:
- pp-translate_string (unnamed-fixed:);
+ translate_string (unnamed-fixed:);
break;
default:
gcc_unreachable ();
}
- pp_decimal_int (pp, prec);
- pp_greater (pp);
+ pp_decimal_int (this, prec);
+ pp_greater (this);
}
}
break;
case TYPE_DECL:
if (DECL_NAME (t))
- pp-id_expression (t);
+ id_expression (t);
else
- pp-translate_string (typedef-error);
+ translate_string (typedef-error);
break;
case UNION_TYPE:
case RECORD_TYPE:
case ENUMERAL_TYPE:
if (code == UNION_TYPE)
- pp_c_ws_string (pp, union);
+ pp_c_ws_string (this, union);
else if (code == RECORD_TYPE)
- pp_c_ws_string (pp, struct);
+ pp_c_ws_string (this, struct);
else if (code == ENUMERAL_TYPE)
- pp_c_ws_string (pp, enum);
+ pp_c_ws_string (this, enum);
else
- pp-translate_string (tag-error);
+ translate_string (tag-error);
if (TYPE_NAME (t))
- pp-id_expression (TYPE_NAME (t));
+ id_expression (TYPE_NAME (t));
else
- pp-translate_string (anonymous);
+ translate_string (anonymous);
break;
default:
- pp_unsupported_tree (pp, t);
+ pp_unsupported_tree (this, t);
break;
}
}
@@ -483,7 +486,7 @@
break;
default:
- pp_simple_type_specifier (pp, t);
+ pp-simple_type_specifier (t);
break;
}
if ((pp-flags pp_c_flag_gnu_v3) code != POINTER_TYPE)
@@ -2328,7 +2331,6 @@
type_specifier_seq= pp_c_specifier_qualifier_list;
ptr_operator = pp_c_pointer;
parameter_list= pp_c_parameter_type_list;
- simple_type_specifier = pp_c_type_specifier
Clean up pretty printers [20/n]
Same topic as 19/n.
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-09-03 Gabriel Dos Reis g...@integrable-solutions.net
c/
* c-objc-common.c (c_tree_printer): Tidy.
c-family/
* c-pretty-print.h (c_pretty_printer::type_id): Now a virtual
member function.
(c_pretty_printer::storage_class_specifier): Likewise.
(c_pretty_printer::initializer): Likewise.
(pp_declaration): Remove.
(pp_declaration_specifiers): Likewise.
(pp_abstract_declarator): Likewise.
(pp_declarator): Likewise.
(pp_type_id): Likewise.
(pp_statement): Likewise.
(pp_constant): Likewise.
(pp_id_expression): Likewise.
(pp_primary_expression): Likewise.
(pp_unary_expression): Likewise.
(pp_multiplicative_expression): Likewise.
(pp_conditional_expression): Likewise.
(pp_assignment_expression): Likewise.
(pp_expression): Likewise.
(pp_c_type_id): Likewise.
(pp_c_storage_class_specifier): Likewise.
* c-pretty-print.c (pp_c_type_cast): Tidy.
(pp_c_pointer): Likewise.
(pp_c_type_specifier): Likewise.
(pp_c_parameter_type_list): Likewise.
(pp_c_function_definition): Likewise.
(pp_c_init_declarator): Likewise.
(pp_c_initializer_list): Likewise.
(pp_c_constructor_elts): Likewise.
(c_pretty_printer::direct_abstract_declarator): Likewise.
(c_pretty_printer::declaration_specifiers): Likewise.
(c_pretty_printer::primary_expression): Likewise.
(c_pretty_printer::postfix_expression): Likewise.
(c_pretty_printer::type_id): Rename from pp_c_type_id.
(c_pretty_printer::storage_class_specifier): Rename from
pp_c_storage_class_specifier.
(c_pretty_printer::initializer): Rename from pp_c_initializer.
(c_pretty_printer::c_pretty_printer): Do not assign to type_id,
storage_class_specifier, initializer, offset_list, flags.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::type_id): Declare as
overrider.
* cxx-pretty-print.c (pp_cxx_storage_class_specifier): Remove.
(pp_cxx_userdef_literal): Tidy.
(pp_cxx_template_argument_list): Likewise.
(pp_cxx_typeid_expression): Likewise.
(pp_cxx_offsetof_expression_1): Likewise.
(cxx_pretty_printer::postfix_expression): Likewise.
(cxx_pretty_printer::unary_expression): Likewise.
(cxx_pretty_printer::statement): Likewise.
(cxx_pretty_printer::type_id): Rename from pp_cxx_type_id.
(c_pretty_printer::cxx_pretty_printer): Do not assign to type_id.
* error.c (dump_decl): Tidy.
(dump_expr): Likewise.
Index: c/c-objc-common.c
===
--- c/c-objc-common.c (revision 202201)
+++ c/c-objc-common.c (working copy)
@@ -120,7 +120,7 @@
t = DECL_DEBUG_EXPR (t);
if (!DECL_P (t))
{
- pp_expression (cpp, t);
+ cpp-expression (t);
return true;
}
}
@@ -143,12 +143,12 @@
if (DECL_NAME (name))
pp_identifier (cpp, lang_hooks.decl_printable_name (name, 2));
else
- pp_type_id (cpp, t);
+ cpp-type_id (t);
return true;
}
else
{
- pp_type_id (cpp, t);
+ cpp-type_id (t);
return true;
}
break;
@@ -157,7 +157,7 @@
if (TREE_CODE (t) == IDENTIFIER_NODE)
pp_identifier (cpp, IDENTIFIER_POINTER (t));
else
- pp_expression (cpp, t);
+ cpp-expression (t);
return true;
case 'V':
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 202201)
+++ c-family/c-pretty-print.c (working copy)
@@ -208,7 +208,7 @@
pp_c_type_cast (c_pretty_printer *pp, tree t)
{
pp_c_left_paren (pp);
- pp_type_id (pp, t);
+ pp-type_id (t);
pp_c_right_paren (pp);
}
@@ -296,7 +296,7 @@
/* ??? This node is now in GENERIC and so shouldn't be here. But
we'll fix that later. */
case DECL_EXPR:
- pp_declaration (pp, DECL_EXPR_DECL (t));
+ pp-declaration (DECL_EXPR_DECL (t));
pp_needs_newline (pp) = true;
break;
@@ -393,7 +393,7 @@
case TYPE_DECL:
if (DECL_NAME (t))
- pp_id_expression (pp, t);
+ pp-id_expression (t);
else
pp-translate_string (typedef-error);
break;
@@ -411,7 +411,7 @@
pp-translate_string (tag-error);
if (TYPE_NAME (t))
- pp_id_expression (pp, TYPE_NAME (t));
+ pp-id_expression (TYPE_NAME (t));
else
pp-translate_string (anonymous);
break;
@@ -431,7 +431,7 @@
function declarations, this routine prints not just the
specifier-qualifier-list of such entities or types
Re: Ada PATCH: Fix ada/58239 by linking with xg++, not xgcc
Eric Botcazou ebotca...@adacore.com writes: | It seems the patch needs a couple of minor amendments to work with Darwin - | and I've added an updated version to the PR which passes bootstrap and make | check-ada on x86_64-darwin12. Iain | | Thanks, here is the final patch I just installed. Thanks! -- Gaby
Re: Ada PATCH: Fix ada/58239 by linking with xg++, not xgcc
Eric Botcazou ebotca...@adacore.com writes: | This patch fixes that by introducing GXX_LINK which is GCC_LINK except | that CXX (e.g. xg++) instead of CC (e.g. xgcc) is invoked. | | Eric, are there other executables that need to be linked with GXX_LINK | too but aren't triggered yet? | | Yes, all not covered executables linking with TOOLS_LIBS since it contains | libcommon.a which now drags the C++ library. So the simplest solution is to | change GCC_LINK (there is one potential problematic case, vxaddr2line, but it | probably didn't link before so let's forget it for now). | | I'll attach a patch to the PR so that the Darwin folks can test it. Thank you; that is very much appreciated. -- Gaby
Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz m...@suse.de wrote: And the manual GTY markers are so not maintainable in the long run, gengtype or something else really needs to be taught to create them automatically. I thought the principle that was acquired was that gengtype shouldn't be improved to support more than what it does now…. -- Gaby
Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek ja...@redhat.com wrote: On Fri, Aug 30, 2013 at 08:58:43AM -0500, Gabriel Dos Reis wrote: On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz m...@suse.de wrote: And the manual GTY markers are so not maintainable in the long run, gengtype or something else really needs to be taught to create them automatically. I thought the principle that was acquired was that gengtype shouldn't be improved to support more than what it does now…. If it means that we'll need to write and maintain tons of hand written code that could otherwise be generated and maintained by a tool for us, that principle doesn't look very good. Jakub Back in March 2013, I asked about gengtype support for inheritance. http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html This http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html was the definitive answer that appeared to be the consensus. -- Gaby
Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
On Fri, Aug 30, 2013 at 10:21 AM, Michael Matz m...@suse.de wrote: Hi, On Fri, 30 Aug 2013, Gabriel Dos Reis wrote: On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek ja...@redhat.com wrote: I thought the principle that was acquired was that gengtype shouldn't be improved to support more than what it does now…. If it means that we'll need to write and maintain tons of hand written code that could otherwise be generated and maintained by a tool for us, that principle doesn't look very good. Exactly. Back in March 2013, I asked about gengtype support for inheritance. http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html This http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html was the definitive answer that appeared to be the consensus. Well, it was a wrong decision then. For some smaller types writing manual marker might be a sensible thing, or for some extra complicated constructs. But here we're talking about the most simple struct hierarchy imaginable. Having to write manual markers for that one is absurd IMO. We can reserve the emotional strong words for later. For now, the focus should be for us to ensure we are being consistent and making design decisions that carry consensus, hence my original note. -- Gaby
Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
On Fri, Aug 30, 2013 at 10:37 AM, Jakub Jelinek ja...@redhat.com wrote:
On Fri, Aug 30, 2013 at 11:28:34AM -0400, Diego Novillo wrote:
Well, it was a wrong decision then. For some smaller types writing manual
marker might be a sensible thing, or for some extra complicated
constructs. But here we're talking about the most simple struct hierarchy
imaginable. Having to write manual markers for that one is absurd IMO.
I want to discourage extending gengtype more than necessary. Long
term, I would like to see memory pools replacing GC. However, that is
likely a long road so we should find an interim solution.
I have doubts that, still somewhat remember the obstack era and memory pools
would again bring all the issues back, but let's put that aside.
We didn't have the automation of RAII with obstacle, back then.
We do have evidence of widely use industrial
C and C++ compilers that are built around the idea of memory pools.
I vaguely remember thinking about what would be needed to have
gengtype deal with inheritance. It needed some pretty ugly
annotations. This made gengtype even more magic. That's very bad for
maintenance.
Teaching the gengtype parser about
{struct,class} name : public {struct,class} someothername { … }
I would suggest NOT to allow the {struct, class} part in the base-class clause.
as opposed to current
{struct,class} name { ... }
shouldn't be that hard. And, if the complaint is that we'd need to write
whole C++ parser for it, then the response could be that we already have
one C++ parser (and, even have plugin support and/or emit dwarf etc.).
We should not need one. At most the base classes would have the form
optional-typename optional-qualifying-scope identifier
optional-template-argument-list
that should cover most of what we want.
So, gengtype could very well use a g++ plugin to emit the stuff it needs,
or parse DWARF, etc. And, we even could not require everybody actually
emitting those themselves, we could check some text form of that
(gengtype.state?) into the tree, so only people actually changing the
compiler would need to run the plugin.
One thing I liked is a suggestion that went something along the lines
of creating some base templates that could be used to facilitate
writing the manual markers.
Even if you have some stuff that helps you writing those, still it will be a
big source of bugs (very hard to debug) and a maintainance nightmare.
Jakub
Clean up pretty printers [19/n]
Most of declaration pretty printing functions.
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-08-30 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::declaration): Now a virtual
member function.
(c_pretty_printer::declaration_specifiers): Likewise.
(c_pretty_printer::declarator): Likewise.
(c_pretty_printer::abstract_declarator): Likewise.
(c_pretty_printer::direct_abstract_declarator): Likewise.
(c_pretty_printer::direct_declarator): Likewise.
(c_pretty_printer::function_specifier): Likewise.
(pp_declaration): Adjust.
(pp_declaration_specifiers): Likewise.
(pp_abstract_declarator): Likewise.
(pp_direct_declarator): Likewise.
(pp_function_specifier): Likewise.
(pp_direct_abstract_declarator): Remove as unused.
(pp_c_declaration): Remove.
(pp_c_declaration_specifiers): Likewise.
(pp_c_declarator): Likewise.
(pp_c_direct_declarator): Likewise.
(pp_c_function_specifier): Likewise.
(pp_c_direct_abstract_declarator): Likewise.
* c-pretty-print.c (c_pretty_printer::abstract_declarator): Rename
from pp_c_abstract_declarator. Adjust.
(c_pretty_printer::direct_abstract_declarator): Rename from
pp_c_direct_abstract_declarator. Adjust.
(c_pretty_printer::function_specifier): Rename from
pp_c_function_specifier. Adjust.
(c_pretty_printer::declaration_specifiers): Rename from
pp_c_declaration_specifiers. Adjust.
(c_pretty_printer::direct_declarator): Rename from
pp_c_direct_declarator. Adjust.
(c_pretty_printer::declarator): Rename from pp_c_declarator. Adjust.
(c_pretty_printer::declaration): Rename from pp_c_declaration. Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
declaration, declaration_specifiers, declarator,
direct_declarator, direct_abstract_declarator, function_specifier.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::declaration): Declare as
overrider.
(cxx_pretty_printer::declaration_specifiers): Likewise.
(cxx_pretty_printer::function_specifier): Likewise.
(cxx_pretty_printer::declarator): Likewise.
(cxx_pretty_printer::direct_declarator): Likewise.
(cxx_pretty_printer::abstract_declarator): Likewise.
(cxx_pretty_printer::direct_abstract_declarator): Likewise.
(pp_cxx_declaration): Remove.
* cxx-pretty-print.c (cxx_pretty_printer::function_specifier):
Rename from pp_cxx_function_specifier. Adjust.
(cxx_pretty_printer::declaration_specifiers): Rename from
pp_cxx_decl_specifier_seq. Adjust.
(cxx_pretty_printer::direct_declarator): Rename from
pp_cxx_direct_declarator. Adjust.
(cxx_pretty_printer::declarator): Rename from pp_cxx_declarator.
Adjust.
(cxx_pretty_printer::abstract_declarator): Rename from
pp_cxx_abstract_declarator. Adjust.
(cxx_pretty_printer::direct_abstract_declarator): Rename from
pp_cxx_direct_abstract_declarator. Adjust.
(cxx_pretty_printer::declaration): Rename from
pp_cxx_declaration. Adjust.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
declaration, declaration_specifiers, function_specifier,
declarator, direct_declarator, abstract_declarator,
direct_abstract_declarator.
* error.c (dump_decl): Adjust.
Index: gcc/c-family/c-pretty-print.c
===
--- gcc/c-family/c-pretty-print.c (revision 202108)
+++ gcc/c-family/c-pretty-print.c (working copy)
@@ -431,7 +431,7 @@
function declarations, this routine prints not just the
specifier-qualifier-list of such entities or types of such entities,
but also the 'pointer' production part of their declarators. The
- remaining part is done by pp_declarator or pp_c_abstract_declarator. */
+ remaining part is done by pp_declarator or pp_abstract_declarator. */
void
pp_c_specifier_qualifier_list (c_pretty_printer *pp, tree t)
@@ -533,18 +533,18 @@
pointer
pointer(opt) direct-abstract-declarator */
-static void
-pp_c_abstract_declarator (c_pretty_printer *pp, tree t)
+void
+c_pretty_printer::abstract_declarator (tree t)
{
if (TREE_CODE (t) == POINTER_TYPE)
{
if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE
|| TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
- pp_c_right_paren (pp);
+ pp_c_right_paren (this);
t = TREE_TYPE (t);
}
- pp_direct_abstract_declarator (pp, t);
+ direct_abstract_declarator (t);
}
/* direct-abstract-declarator:
@@ -554,34 +554,34 @@
direct-abstract-declarator(opt) ( parameter-type-list(opt) ) */
void
-pp_c_direct_abstract_declarator (c_pretty_printer *pp, tree t
Re: [PATCH] Support dumping type bindings and 'mutable' qualifier in lambda diagnostics.
On Thu, Aug 29, 2013 at 9:20 AM, Adam Butcher a...@jessamine.co.uk wrote:
* error.c (dump_lambda_function): New function, dependent on ...
(maybe_dump_template_bindings): ... this new function, factored out of
...
(dump_function_decl): ... here. Updated to early-out with call to
dump_lambda_function after determining template bindings.
---
Reworked as requested. Okay to commit?
Document the new functions.
Use pp-translate_string (with) instead of
pp_cxx_ws_string (pp, M_(with)).
OK to commit with those changes.
Thanks,
-- Gaby
+static void
+maybe_dump_template_bindings (cxx_pretty_printer *pp,
+ tree t, tree template_parms, tree template_args,
+ int flags)
+{
+ if (template_parms != NULL_TREE template_args != NULL_TREE
+ !(flags TFF_NO_TEMPLATE_BINDINGS))
+{
+ vectree, va_gc *typenames = find_typenames (t);
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp_cxx_ws_string (pp, M_(with));
+ pp_cxx_whitespace (pp);
+ dump_template_bindings (pp, template_parms, template_args, typenames);
+ pp_cxx_right_bracket (pp);
+}
+}
+
+static void
+dump_lambda_function (cxx_pretty_printer *pp,
+ tree fn, tree template_parms, tree template_args,
+ int flags)
+{
+ /* A lambda's signature is essentially its type. */
+ dump_type (pp, DECL_CONTEXT (fn), flags);
+ if (!(TYPE_QUALS (class_of_this_parm (TREE_TYPE (fn))) TYPE_QUAL_CONST))
+{
+ pp-padding = pp_before;
+ pp_c_ws_string (pp, mutable);
+}
+ maybe_dump_template_bindings (pp, fn, template_parms, template_args,
flags);
+}
+
/* Pretty print a function decl. There are several ways we want to print a
function declaration. The TFF_ bits in FLAGS tells us how to behave.
As error can only apply the '#' flag once to give 0 and 1 for V, there
@@ -1379,15 +1412,6 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
int show_return = flags TFF_RETURN_TYPE || flags TFF_DECL_SPECIFIERS;
int do_outer_scope = ! (flags TFF_UNQUALIFIED_NAME);
tree exceptions;
- vectree, va_gc *typenames = NULL;
-
- if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
-{
- /* A lambda's signature is essentially its type, so defer. */
- gcc_assert (LAMBDA_TYPE_P (DECL_CONTEXT (t)));
- dump_type (pp, DECL_CONTEXT (t), flags);
- return;
-}
flags = ~(TFF_UNQUALIFIED_NAME | TFF_TEMPLATE_NAME);
if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -1409,10 +1433,13 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
{
template_parms = DECL_TEMPLATE_PARMS (tmpl);
t = tmpl;
- typenames = find_typenames (t);
}
}
+ if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
+return dump_lambda_function (pp, t, template_parms, template_args,
+flags);
+
fntype = TREE_TYPE (t);
parmtypes = FUNCTION_FIRST_USER_PARMTYPE (t);
@@ -1476,17 +1503,8 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
if (show_return)
dump_type_suffix (pp, TREE_TYPE (fntype), flags);
- /* If T is a template instantiation, dump the parameter binding. */
- if (template_parms != NULL_TREE template_args != NULL_TREE
- !(flags TFF_NO_TEMPLATE_BINDINGS))
- {
- pp_cxx_whitespace (pp);
- pp_cxx_left_bracket (pp);
- pp_cxx_ws_string (pp, M_(with));
- pp_cxx_whitespace (pp);
- dump_template_bindings (pp, template_parms, template_args,
typenames);
- pp_cxx_right_bracket (pp);
- }
+ maybe_dump_template_bindings (pp, t, template_parms, template_args,
+ flags);
}
else if (template_args)
{
--
1.8.4
Re: [PATCH] Support dumping type bindings and 'mutable' qualifier in lambda diagnostics.
On Thu, Aug 29, 2013 at 1:51 PM, Adam Butcher a...@jessamine.co.uk wrote:
* error.c (dump_lambda_function): New function, dependent on ...
(maybe_dump_with_clause): ... this new function, factored out of ...
(subst_to_string): ... here and ...
(dump_function_decl): ... here. Updated to early-out with call to
dump_lambda_function after determining template bindings.
---
Hi Gaby,
On 29.08.2013 18:04, Adam Butcher wrote:
On 29.08.2013 16:25, Gabriel Dos Reis wrote:
Document the new functions.
Use pp-translate_string (with) instead of
pp_cxx_ws_string (pp, M_(with)).
Done. In documenting 'maybe_dump_template_bindings' and reviewing
it again I'm not sure it's got the right name. It wraps
'dump_template_bindings' in [with and ]. So it does more than
just filter a call to 'dump_template_bindings'.
Any suggestions? What do you think of 'maybe_dump_with_clause' or
something similar?
Here's a diff with the name change (though I'm not all that happy with
it) and the docs.
call it dump_substitution. We don't need the 'maybe_' prefix since
an empty substitution is the identity transformation.
I've also reimplemented subst_to_string in terms of the new function
as it was duplicating much of the code from dump_function_decl (the
only downside of this is some unnecessary tests in the subst_to_string
case but I think they should get optimized away if it were inlined --
we're dealing with diagnostics formatting here anyway so performance
is probably not a big factor).
Patch OK with that change. Thanks!
Cheers,
Adam
---
gcc/cp/error.c | 73
--
1 file changed, 46 insertions(+), 27 deletions(-)
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index c82a0ce..3d1e173 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1363,6 +1363,47 @@ find_typenames (tree t)
return ft.typenames;
}
+/* Output the [with ...] clause for a template instantiation T iff
+ TEMPLATE_PARMS, TEMPLATE_ARGS and FLAGS are suitable. T may be NULL if
+ formatting a deduction/substitution diagnostic rather than an
+ instantiation. */
+
+static void
+maybe_dump_with_clause (cxx_pretty_printer *pp,
+ tree t, tree template_parms, tree template_args,
+ int flags)
+{
+ if (template_parms != NULL_TREE template_args != NULL_TREE
+ !(flags TFF_NO_TEMPLATE_BINDINGS))
+{
+ vectree, va_gc *typenames = t ? find_typenames (t) : NULL;
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp-translate_string (with);
+ pp_cxx_whitespace (pp);
+ dump_template_bindings (pp, template_parms, template_args, typenames);
+ pp_cxx_right_bracket (pp);
+}
+}
+
+/* Dump the lambda function FN including its 'mutable' qualifier and any
+ template bindings. */
+
+static void
+dump_lambda_function (cxx_pretty_printer *pp,
+ tree fn, tree template_parms, tree template_args,
+ int flags)
+{
+ /* A lambda's signature is essentially its type. */
+ dump_type (pp, DECL_CONTEXT (fn), flags);
+ if (!(TYPE_QUALS (class_of_this_parm (TREE_TYPE (fn))) TYPE_QUAL_CONST))
+{
+ pp-padding = pp_before;
+ pp_c_ws_string (pp, mutable);
+}
+ maybe_dump_with_clause (pp, fn, template_parms, template_args, flags);
+}
+
/* Pretty print a function decl. There are several ways we want to print a
function declaration. The TFF_ bits in FLAGS tells us how to behave.
As error can only apply the '#' flag once to give 0 and 1 for V, there
@@ -1379,15 +1420,6 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
int show_return = flags TFF_RETURN_TYPE || flags TFF_DECL_SPECIFIERS;
int do_outer_scope = ! (flags TFF_UNQUALIFIED_NAME);
tree exceptions;
- vectree, va_gc *typenames = NULL;
-
- if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
-{
- /* A lambda's signature is essentially its type, so defer. */
- gcc_assert (LAMBDA_TYPE_P (DECL_CONTEXT (t)));
- dump_type (pp, DECL_CONTEXT (t), flags);
- return;
-}
flags = ~(TFF_UNQUALIFIED_NAME | TFF_TEMPLATE_NAME);
if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -1409,10 +1441,12 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
{
template_parms = DECL_TEMPLATE_PARMS (tmpl);
t = tmpl;
- typenames = find_typenames (t);
}
}
+ if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
+return dump_lambda_function (pp, t, template_parms, template_args,
flags);
+
fntype = TREE_TYPE (t);
parmtypes = FUNCTION_FIRST_USER_PARMTYPE (t);
@@ -1476,17 +1510,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
if (show_return)
dump_type_suffix (pp, TREE_TYPE (fntype), flags);
- /* If T is a template instantiation, dump
Re: [PATCH, committed] Support dumping type bindings and 'mutable' qualifier in lambda diagnostics.
On Thu, Aug 29, 2013 at 3:57 PM, Adam Butcher a...@jessamine.co.uk wrote:
From: abutcher abutcher@138bc75d-0d04-0410-961f-82ee72b054a4
* error.c (dump_lambda_function): New function, dependent on ...
(dump_substitution): ... this new function, factored out of ...
(subst_to_string): ... here and ...
(dump_function_decl): ... here. Updated to early-out with call to
dump_lambda_function after determining template bindings.
Please go ahead and commit it. You do have SVN commit access, right?
-- Gaby
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@202087
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/cp/ChangeLog | 8 +++
gcc/cp/error.c | 73
+++-
2 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index bf49198..f848b81 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2013-08-29 Adam Butcher a...@jessamine.co.uk
+
+ * error.c (dump_lambda_function): New function, dependent on ...
+ (dump_substitution): ... this new function, factored out of ...
+ (subst_to_string): ... here and ...
+ (dump_function_decl): ... here. Updated to early-out with call to
+ dump_lambda_function after determining template bindings.
+
2013-08-28 Paolo Carlini paolo.carl...@oracle.com
PR c++/58255
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index c82a0ce..af71000 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1363,6 +1363,47 @@ find_typenames (tree t)
return ft.typenames;
}
+/* Output the [with ...] clause for a template instantiation T iff
+ TEMPLATE_PARMS, TEMPLATE_ARGS and FLAGS are suitable. T may be NULL if
+ formatting a deduction/substitution diagnostic rather than an
+ instantiation. */
+
+static void
+dump_substitution (cxx_pretty_printer *pp,
+ tree t, tree template_parms, tree template_args,
+ int flags)
+{
+ if (template_parms != NULL_TREE template_args != NULL_TREE
+ !(flags TFF_NO_TEMPLATE_BINDINGS))
+{
+ vectree, va_gc *typenames = t ? find_typenames (t) : NULL;
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp-translate_string (with);
+ pp_cxx_whitespace (pp);
+ dump_template_bindings (pp, template_parms, template_args, typenames);
+ pp_cxx_right_bracket (pp);
+}
+}
+
+/* Dump the lambda function FN including its 'mutable' qualifier and any
+ template bindings. */
+
+static void
+dump_lambda_function (cxx_pretty_printer *pp,
+ tree fn, tree template_parms, tree template_args,
+ int flags)
+{
+ /* A lambda's signature is essentially its type. */
+ dump_type (pp, DECL_CONTEXT (fn), flags);
+ if (!(TYPE_QUALS (class_of_this_parm (TREE_TYPE (fn))) TYPE_QUAL_CONST))
+{
+ pp-padding = pp_before;
+ pp_c_ws_string (pp, mutable);
+}
+ dump_substitution (pp, fn, template_parms, template_args, flags);
+}
+
/* Pretty print a function decl. There are several ways we want to print a
function declaration. The TFF_ bits in FLAGS tells us how to behave.
As error can only apply the '#' flag once to give 0 and 1 for V, there
@@ -1379,15 +1420,6 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
int show_return = flags TFF_RETURN_TYPE || flags TFF_DECL_SPECIFIERS;
int do_outer_scope = ! (flags TFF_UNQUALIFIED_NAME);
tree exceptions;
- vectree, va_gc *typenames = NULL;
-
- if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
-{
- /* A lambda's signature is essentially its type, so defer. */
- gcc_assert (LAMBDA_TYPE_P (DECL_CONTEXT (t)));
- dump_type (pp, DECL_CONTEXT (t), flags);
- return;
-}
flags = ~(TFF_UNQUALIFIED_NAME | TFF_TEMPLATE_NAME);
if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -1409,10 +1441,12 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
{
template_parms = DECL_TEMPLATE_PARMS (tmpl);
t = tmpl;
- typenames = find_typenames (t);
}
}
+ if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
+return dump_lambda_function (pp, t, template_parms, template_args,
flags);
+
fntype = TREE_TYPE (t);
parmtypes = FUNCTION_FIRST_USER_PARMTYPE (t);
@@ -1476,17 +1510,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
if (show_return)
dump_type_suffix (pp, TREE_TYPE (fntype), flags);
- /* If T is a template instantiation, dump the parameter binding. */
- if (template_parms != NULL_TREE template_args != NULL_TREE
- !(flags TFF_NO_TEMPLATE_BINDINGS))
- {
- pp_cxx_whitespace (pp);
- pp_cxx_left_bracket (pp);
- pp_cxx_ws_string (pp, M_(with));
- pp_cxx_whitespace (pp);
- dump_template_bindings (pp,
Re: [PATCH, committed] Support dumping type bindings and 'mutable' qualifier in lambda diagnostics.
On Thu, Aug 29, 2013 at 4:06 PM, Adam Butcher a...@jessamine.co.uk wrote:
On 29.08.2013 21:59, Gabriel Dos Reis wrote:
On Thu, Aug 29, 2013 at 3:57 PM, Adam Butcher a...@jessamine.co.uk
wrote:
From: abutcher abutcher@138bc75d-0d04-0410-961f-82ee72b054a4
* error.c (dump_lambda_function): New function, dependent on ...
(dump_substitution): ... this new function, factored out of ...
(subst_to_string): ... here and ...
(dump_function_decl): ... here. Updated to early-out with call
to
dump_lambda_function after determining template bindings.
Please go ahead and commit it. You do have SVN commit access, right?
Yes, this is the committed patch. I was under the impression that I should
ping the list with the commit once pushed.
yes, you are right; my confusion.
Adam
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@202087
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/cp/ChangeLog | 8 +++
gcc/cp/error.c | 73
+++-
2 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index bf49198..f848b81 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2013-08-29 Adam Butcher a...@jessamine.co.uk
+
+ * error.c (dump_lambda_function): New function, dependent on ...
+ (dump_substitution): ... this new function, factored out of ...
+ (subst_to_string): ... here and ...
+ (dump_function_decl): ... here. Updated to early-out with call
to
+ dump_lambda_function after determining template bindings.
+
2013-08-28 Paolo Carlini paolo.carl...@oracle.com
PR c++/58255
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index c82a0ce..af71000 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1363,6 +1363,47 @@ find_typenames (tree t)
return ft.typenames;
}
+/* Output the [with ...] clause for a template instantiation T iff
+ TEMPLATE_PARMS, TEMPLATE_ARGS and FLAGS are suitable. T may be NULL
if
+ formatting a deduction/substitution diagnostic rather than an
+ instantiation. */
+
+static void
+dump_substitution (cxx_pretty_printer *pp,
+ tree t, tree template_parms, tree template_args,
+ int flags)
+{
+ if (template_parms != NULL_TREE template_args != NULL_TREE
+ !(flags TFF_NO_TEMPLATE_BINDINGS))
+{
+ vectree, va_gc *typenames = t ? find_typenames (t) : NULL;
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp-translate_string (with);
+ pp_cxx_whitespace (pp);
+ dump_template_bindings (pp, template_parms, template_args,
typenames);
+ pp_cxx_right_bracket (pp);
+}
+}
+
+/* Dump the lambda function FN including its 'mutable' qualifier and any
+ template bindings. */
+
+static void
+dump_lambda_function (cxx_pretty_printer *pp,
+ tree fn, tree template_parms, tree template_args,
+ int flags)
+{
+ /* A lambda's signature is essentially its type. */
+ dump_type (pp, DECL_CONTEXT (fn), flags);
+ if (!(TYPE_QUALS (class_of_this_parm (TREE_TYPE (fn)))
TYPE_QUAL_CONST))
+{
+ pp-padding = pp_before;
+ pp_c_ws_string (pp, mutable);
+}
+ dump_substitution (pp, fn, template_parms, template_args, flags);
+}
+
/* Pretty print a function decl. There are several ways we want to print
a
function declaration. The TFF_ bits in FLAGS tells us how to behave.
As error can only apply the '#' flag once to give 0 and 1 for V,
there
@@ -1379,15 +1420,6 @@ dump_function_decl (cxx_pretty_printer *pp, tree
t, int flags)
int show_return = flags TFF_RETURN_TYPE || flags
TFF_DECL_SPECIFIERS;
int do_outer_scope = ! (flags TFF_UNQUALIFIED_NAME);
tree exceptions;
- vectree, va_gc *typenames = NULL;
-
- if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
-{
- /* A lambda's signature is essentially its type, so defer. */
- gcc_assert (LAMBDA_TYPE_P (DECL_CONTEXT (t)));
- dump_type (pp, DECL_CONTEXT (t), flags);
- return;
-}
flags = ~(TFF_UNQUALIFIED_NAME | TFF_TEMPLATE_NAME);
if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -1409,10 +1441,12 @@ dump_function_decl (cxx_pretty_printer *pp, tree
t, int flags)
{
template_parms = DECL_TEMPLATE_PARMS (tmpl);
t = tmpl;
- typenames = find_typenames (t);
}
}
+ if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
+return dump_lambda_function (pp, t, template_parms, template_args,
flags);
+
fntype = TREE_TYPE (t);
parmtypes = FUNCTION_FIRST_USER_PARMTYPE (t);
@@ -1476,17 +1510,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree
t, int flags)
if (show_return)
dump_type_suffix (pp, TREE_TYPE (fntype), flags);
- /* If T is a template instantiation, dump the parameter binding.
*/
- if (template_parms
Re: [PATCH] Support dumping type bindings and 'mutable' qualifier in lambda diagnostics.
On Tue, Aug 27, 2013 at 7:54 PM, Adam Butcher a...@jessamine.co.uk wrote:
* error.c (dump_function_decl): Use standard diagnostic flow to dump a
lambda diagnostic, albeit without stating the function name or
duplicating the parameter spec (which is dumped as part of the type).
Rather than qualifying the diagnostic with 'const' for plain lambdas,
qualify with 'mutable' if non-const.
---
Okay to commit?
This looks good to me. I have one suggestion. See below.
---
gcc/cp/error.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index c82a0ce..a8ca269 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1380,14 +1380,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
int do_outer_scope = ! (flags TFF_UNQUALIFIED_NAME);
tree exceptions;
vectree, va_gc *typenames = NULL;
-
- if (DECL_NAME (t) LAMBDA_FUNCTION_P (t))
-{
- /* A lambda's signature is essentially its type, so defer. */
- gcc_assert (LAMBDA_TYPE_P (DECL_CONTEXT (t)));
- dump_type (pp, DECL_CONTEXT (t), flags);
- return;
-}
+ bool lambda_p = false;
flags = ~(TFF_UNQUALIFIED_NAME | TFF_TEMPLATE_NAME);
if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -1449,21 +1442,31 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
int flags)
else if (cname)
{
dump_type (pp, cname, flags);
- pp_cxx_colon_colon (pp);
+ if (LAMBDA_TYPE_P (cname))
+ lambda_p = true;
Instead of introducing the local variable lambda_p and then
weave your way through the existing maze, do this instead:
(1) Introduce a new function, call it dump_lambda_expr
(or something like that) that does only the formatting
of a lambda expression, and nothing else.
(2) when you arrive here where you test LAMBDA_TYPE_P
then do a
return dump_lambda_expr (pp, flags);
The result is much simpler and cleaner that way.
Thanks,
+ else
+ pp_cxx_colon_colon (pp);
}
else
dump_scope (pp, CP_DECL_CONTEXT (t), flags);
- dump_function_name (pp, t, flags);
+ /* A lambda's signature is essentially its type, which has already been
+ dumped. */
+ if (!lambda_p)
+dump_function_name (pp, t, flags);
if (!(flags TFF_NO_FUNCTION_ARGUMENTS))
{
- dump_parameters (pp, parmtypes, flags);
+ if (!lambda_p)
+ dump_parameters (pp, parmtypes, flags);
if (TREE_CODE (fntype) == METHOD_TYPE)
{
pp-padding = pp_before;
- pp_cxx_cv_qualifier_seq (pp, class_of_this_parm (fntype));
+ if (!lambda_p)
+ pp_cxx_cv_qualifier_seq (pp, class_of_this_parm (fntype));
+ else if (!(TYPE_QUALS (class_of_this_parm (fntype))
TYPE_QUAL_CONST))
+ pp_c_ws_string (pp, mutable);
dump_ref_qualifier (pp, fntype, flags);
}
--
1.8.4
Re: ELF interposition and One Definition Rule
On Mon, Aug 26, 2013 at 7:58 PM, Jason Merrill ja...@redhat.com wrote: On 08/26/2013 11:21 AM, Jan Hubicka wrote: Our default behaviour special case inline functions that are always AVAIL_AVAILABLE and, via decl_replaceable_p, also any COMDAT (that may be for functions since all COMDATs are also inlines, but makes difference for variables I think). Not all COMDAT functions are inlines; template instantiations are also COMDAT. My understanding of C++ One Definition Rule, in a strict sense, does not a allow in to define two functions of the same name and different semantics in a valid program. Strictly speaking, the same is true of C: If an identifier declared with external linkage is used in an expression (other than as part of the operand of a sizeof or _Alignof operator whose result is an integer constant), somewhere in the entire program there shall be exactly one external definition for the identifier; otherwise, there shall be no more than one. I also think that all DSOs eventually linked together or dlopenned are part of the same program. So theoretically, for C++ produced code, we may go with AVAIL_AVAILABLE everywhere. And for C code as well. C++'s ODR is much stronger than C's though. For example, C allows definitions for the same inline function to differ -- the out-of-line extern definition does not need to be the same. That is anathema in C++. ELF symbol replacement is an extension to C/C++ semantics, which does not include DSOs; it seems to me that it is up to us to define what assumptions we want the compiler to be able to make. On IRC we got into an agreement that we may disallow interposition for virtuals, since replacing these seems fishy - they don't even have address well defined and compiler can freely duplicate and/or unify them. I think same apply for C++ constructors and destructors now. But it would be simple to create a DSO which replaces a virtual function or a constructor, by simply providing a new definition. Since interposition happens outside the language, I don't think reasoning based on things that happen within the language makes much sense. Of course I would be happier with a stronger rule - for instance allowing interposition only on plain functions not on methods. Existing uses of interposition apply to plain functions, but that doesn't mean someone might not want to use it for member functions as well. Agreed. I would be happy with an even stronger default that optimizes on the assumption that no interposition occurs; typically interposition is overriding a symbol found in a dynamic library (i.e. malloc) rather than a symbol defined in the same translation unit as the use. Jason Again, strongly agreed. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 8:17 PM, Jason Merrill ja...@redhat.com wrote:
On 08/26/2013 10:45 AM, Gabriel Dos Reis wrote:
Hmm, let's not make it a default. Replacing global operator new (e.g. for
tracing purposes) is a valid C++ programming idiom.
Absolutely. What strikes me as vanishingly unlikely is the idea that the
replacement operator new would expose pointers to returned memory *and* that
code would refer to the memory both via one of those pointers and via the
value of the new-expression in the same function. That is,
void *last_new_ptr;
void *operator new (size_t) noexcept(false) {
last_new_ptr = ...;
return last_new_ptr;
}
int main()
{
int *a = new int;
int *b = (int*)last_new_ptr;
*a = 42;
*b = 24;
if (*a != 24) abort();
}
I'm happy to let this code break by assuming that the store to b couldn't
have affected *a.
Amen!
-- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 2:44 AM, Mike Stump mikest...@comcast.net wrote: but I fear you won't understand this and how it applies. You must be right. I cannot match the entertaining value of your message, so you win. -- Gaby
Clean up pretty printers [18/n]
Same topic as patch 17/n. For more expressing printers.
Tested on an x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-26 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::unary_expression): Now a
virtual member function.
(c_pretty_printer::multiplicative_expression): Likewise.
(c_pretty_printer::conditional_expression): Likewise.
(c_pretty_printer::assignment_expression): Likewise.
(c_pretty_printer::expression): Likewise.
(pp_unary_expression): Adjust.
(pp_multiplicative_expression): Likewise.
(pp_assignment_expression): Likewise.
(pp_conditional_expression): Likewise.
(pp_expression): Likewise.
* c-pretty-print.c (c_pretty_printer::unary_expression): Rename
from pp_c_unary_expression. Adjust.
(c_pretty_printer::multiplicative_expression): Rename from
pp_c_multiplicative_expression. Adjust.
(c_pretty_printer::conditional_expression): Rename from
pp_c_conditional_expression. Adjust.
(c_pretty_printer::assignment_expression): Rename from
pp_c_assignment_expression. Adjust.
(c_pretty_printer::expression): Rename from pp_c_expression. Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
unary_expression, multiplicative_expression,
conditional_expression, expression.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::unary_expression):
Declare as overrider.
(cxx_pretty_printer::multiplicative_expression): Likewise.
(cxx_pretty_printer::conditional_expression): Likewise.
(cxx_pretty_printer::assignment_expression): Likewise.
(cxx_pretty_printer::expression): Likewise.
* cxx-pretty-print.c (cxx_pretty_printer::unary_expression):
Rename from pp_cxx_unary_expression. Adjust.
(cxx_pretty_printer::multiplicative_expression): Rename from
pp_cxx_multiplicative_expression. Adjust.
(cxx_pretty_printer::conditional_expression): Rename from
pp_cxx_conditional_expression. Adjust.
(cxx_pretty_printer::assignment_expression): Rename from
pp_cxx_assignment_expression. Adjust.
(cxx_pretty_printer::expression): Rename from pp_cxx_expression.
Adjust.
(cxx_pretty_printer::cxx_pretty_printer): Dot not assign to
unary_expression, multiplicative_expression,
conditional_expression, assignment_expression, expression.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201985)
+++ c-family/c-pretty-print.c (working copy)
@@ -50,7 +50,6 @@
static void pp_c_initializer_list (c_pretty_printer *, tree);
static void pp_c_brace_enclosed_initializer_list (c_pretty_printer *, tree);
-static void pp_c_multiplicative_expression (c_pretty_printer *, tree);
static void pp_c_additive_expression (c_pretty_printer *, tree);
static void pp_c_shift_expression (c_pretty_printer *, tree);
static void pp_c_relational_expression (c_pretty_printer *, tree);
@@ -59,8 +58,6 @@
static void pp_c_exclusive_or_expression (c_pretty_printer *, tree);
static void pp_c_inclusive_or_expression (c_pretty_printer *, tree);
static void pp_c_logical_and_expression (c_pretty_printer *, tree);
-static void pp_c_conditional_expression (c_pretty_printer *, tree);
-static void pp_c_assignment_expression (c_pretty_printer *, tree);
/* declarations. */
@@ -1255,7 +1252,7 @@
if (TREE_OPERAND (e, 2))
{
pp_separate_with (this, ',');
- pp_c_expression (this, TREE_OPERAND (e, 2));
+ expression (TREE_OPERAND (e, 2));
}
pp_c_right_paren (this);
break;
@@ -1619,7 +1616,7 @@
break;
case MEM_REF:
- pp_c_expression (this, e);
+ expression (e);
break;
case COMPLEX_CST:
@@ -1641,7 +1638,7 @@
case VA_ARG_EXPR:
pp_c_ws_string (this, __builtin_va_arg);
pp_c_left_paren (this);
- pp_assignment_expression (this, TREE_OPERAND (e, 0));
+ assignment_expression (TREE_OPERAND (e, 0));
pp_separate_with (this, ',');
pp_type_id (this, TREE_TYPE (e));
pp_c_right_paren (this);
@@ -1721,15 +1718,15 @@
__imag__ unary-expression */
void
-pp_c_unary_expression (c_pretty_printer *pp, tree e)
+c_pretty_printer::unary_expression (tree e)
{
enum tree_code code = TREE_CODE (e);
switch (code)
{
case PREINCREMENT_EXPR:
case PREDECREMENT_EXPR:
- pp_string (pp, code == PREINCREMENT_EXPR ? ++ : --);
- pp_c_unary_expression (pp, TREE_OPERAND (e, 0));
+ pp_string (this, code == PREINCREMENT_EXPR ? ++ : --);
+ unary_expression (TREE_OPERAND (e, 0));
break;
case ADDR_EXPR:
@@ -1740,53 +1737,53 @@
case CONJ_EXPR:
/* String literal are used by address. */
if (code == ADDR_EXPR
Re: Clean up pretty printers [18/n]
Paolo Carlini paolo.carl...@oracle.com writes: | Hi Gaby. | | On 08/26/2013 10:42 AM, Gabriel Dos Reis wrote: | Same topic as patch 17/n. For more expressing printers. | Tested on an x86_64-suse-linux. Applied to mainline. | Just got this: | | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c: In function ‘bool | c_tree_printer(pretty_printer*, text_info*, const char*, int, bool, | bool, bool)’: | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c:123:31: error: | ‘pp_c_expression’ was not declared in this scope | make[2]: *** [c/c-objc-common.o] Error 1 | | Yesterday things were fine. What happened was I inadvertently committed only a fraction of the patch. Now, all changes should be in. Sorry for the breakage. I guess it is time to go to bed. -- Gaby
Re: Clean up pretty printers [18/n]
Paolo Carlini paolo.carl...@oracle.com writes: | Hi Gaby, | | bootstrap is currently broken: | | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c: In function ‘bool | c_tree_printer(pretty_printer*, text_info*, const char*, int, bool, | bool, bool)’: | /scratch/Gcc/svn-dirs/trunk/gcc/c/c-objc-common.c:123:31: error: | ‘pp_c_expression’ was not declared in this scope | make[2]: *** [c/c-objc-common.o] Error 1 | | If I don't hear from you I'm going to commit as obvious the below. You need more than that. The rest of patch is in trunk now. Sorry for the breakage -- as you probably guessed, I specified only c-family directory on the commit command line. -- Gaby
[c++-concepts] MErge from trunk
at revision 201992. There were conflicts with the recent pretty printing patches. Fixed. (Mostly due to the new concept-specific nodes printing.) Modulo the asan stuff. -- Gaby
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. -- Gaby
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:00 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? It seems to reproduce quite uniformlny and doesn't seem to be specific to my firefox builds I am doing right now… I think I may have an idea about which existing leak is making itself noticed but I am looking for a (semi-)automatic way to regtest it. Thanks! Honza -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 9:38 AM, Jan Hubicka hubi...@ucw.cz wrote: On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. I'll ask to reconsider the priority at the next meeting. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. That sounds good to me. Of course it would be really nice to go for this by default and let users overwritting toplevel new to use an option (like we have -freestanding). Hmm, let's not make it a default. Replacing global operator new (e.g. for tracing purposes) is a valid C++ programming idiom. Can we consider the original patch (that does not change malloc attributes) and then I can make this as an followup? Also if any of these runtime parts are known to be cold in a way that is not obvious to the backend (i.e. that it is reachable only by EH edges or leading to noreturn), it may be good idea to add cold attribute annotations or use use PREDICT_EXPR to mark the unlikely path. I never tought too much about this, but especially the EH machinery seems to generate a lot of code that is executed only for EH. I would like to try to get reorder-blocks to split those parts away to cold text section just as we do with the profile. Thank you, Honza Jason
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:32 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 9:00 AM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Aug 26, 2013 at 7:24 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, it seems to be couple weeks I am not able to compile big ltrans unit with -fdump-tree-all because the compiler eventually runs out of memory. I think it is one of your patches introducing serious memory leak. Can you, please, take a look on this? It makes my life harder - it is not fun to wait for 15 minutes for a dump and then see compiler to ICE... Thanks, Honza The existing pretty printers have always had memory leaks, and part of the work is to reduce that in fact :-( For example, instead of printing directly to the output buffer, many of them construct a string for a given formatter, then send that string to the stream, and that string may or may not be reclaimed. Please give me actionable instructions so I can reproduce your situation. I think it should be easiert to build with bootstrap-lto and then add -fdump-tree-all into the final link command line options. modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . I got this part. The question I asked relates to the when you said then add -fdump-tree-all into the final link command line options. You need plugin enabled buinutils for that. ah. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza I think the problem is in tree gimple dumpers. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Mon, Aug 26, 2013 at 9:07 AM, Jason Merrill ja...@redhat.com wrote: On 08/22/2013 12:45 PM, Gabriel Dos Reis wrote: If the user-supplied operator new returns a, then it must also ensure that 'a' is not used anywhere else -- e.g. I you can't do lvalue-to-value conversion on 'a' to see what is written there. Because its storage has been reused. That is, aliasing is framed in terms of object lifetime and uniqueness of ownership. Do you have a reference for this? 3.8/1 says that the object lifetime ends when the storage is reused. So, just returning the pointer itself is not the problem, but returning the problem and reusing the storage (e.g. through construction) may be the problem. The wording in 3.8 seems to only restrict how a pointer is used when there is no object in the storage, it doesn't say anything about using a pointer to access a different object at the same location. This issue seems to be core 1338: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1338 which has priority 2, so it's not likely to get resolved any time soon. Ugh, priority 2 isn't good. I'll ask to reconsider the priority at the next meeting. Yes, please. We probably can go with -fno-user-overwritten-new or something similar? I'd name it something like -fno-aliased-global-new, which would add the malloc attribute to the built-in declarations. Jason
Re: Clean up pretty printers [18/n]
On Mon, Aug 26, 2013 at 9:32 AM, Jan Hubicka hubi...@ucw.cz wrote: modify ALL_LINKERFLAGS or BUILD_LINKERFLAGS? configure --with-build-config=bootstrap-lto . You need plugin enabled buinutils for that. I think there however should be a lot easier way to reproduce it by just dropping some really large source file. Here LTO only runs the backend on very many functions. Honza by the way, we should be leaking less now that we release the memory acquire in obstack for local pretty printers. and discharged pretty printers. However, I think we still have problems with: (1) global pretty printers, (2) when we do ggc_strdup on the formatted text. One of the local patches I have avoids the copy and GGC involvement. Please let me know which of the above Make linker variables I should modify. -- Gaby
Re: Clean up pretty printers [15/n]
Joseph S. Myers jos...@codesourcery.com writes:
| On Sun, 25 Aug 2013, Gabriel Dos Reis wrote:
|
| Instead of defining the same macro several times in different
| translation units, we can just make it a function and use it where
| needed.
|
| Have you made sure that po/exgettext still extracts the relevant messages
| for translation (I'm not sure how good it is at identifying C++ functions
| with arguments called gmsgid, or how good xgettext then is at identifying
| relevant C++ function calls)? In particular, even if other cases get
| identified properly, conditional expressions such as
I trusted xgettext documentation that says that it handles C++ input
source files.
Looking at the documentation, I notice this:
By default the language is guessed depending on the input
file name extension.
I don't know whether after the switch C++ xgettext is being invoked
explicitly with -C or --c++, or whether we are still relying on xgettext
to pick the language based on the file extension. This is probably
another reason why we might want to rename files to use apppropriate
extensions. In the meantime, we might want to explicitly specify the
language for the input source file.
po/exgettext only looks whether the parameter name ends with 'msgid'.
|
| @@ -379,15 +375,15 @@
|switch (code)
| {
| case INTEGER_TYPE:
| - pp_string (pp, (TYPE_UNSIGNED (t)
| - ? M_(unnamed-unsigned:)
| - : M_(unnamed-signed:)));
| + pp-translate_string (TYPE_UNSIGNED (t)
| +? unnamed-unsigned:
| +: unnamed-signed:);
|
| may need each case of the conditional expression to be marked for
| extraction for translation, or to be separated into two separate calls
| using if (we've had that issue before with conditional expressions in
| diagnostics).
Hmm, why would that be needed now, and not before?
(not that I am found of the conditional, but only by curiosity.)
-- Gaby
Re: [C++ patch] Move FINAL flag to middle-end trees.
On Sun, Aug 25, 2013 at 10:38 AM, Jan Hubicka hubi...@ucw.cz wrote:
On 08/24/2013 05:18 AM, Jan Hubicka wrote:
In the next step I would like to introduce the
DECL_CPP_CONSTRUCTOR/DESTRUCTOR macro.
The catch I run into is that these flags are tested on TEMPLATE_DECL so the
middle-end
macro bombs on type checking. I wonder what is best approach here.
I think fix the front end to use STRIP_TEMPLATE to make sure we're
checking/setting the flag on a FUNCTION_DECL.
Thank you!
I did not know the FUNCTION_DECLs are already there. The following patch
seems
to work. Of course if it seems cleaner, I can update the users of the CPP
macros into the expanded variants. I want to have CPP in name in middle-end
to
signify that we actually understand language specific properties of these
functions (so constructors/destructors in other languages probably don't
want to do that).
Bootstrapped/regtested ppc64-linux, OK?
* tree.h (tree_decl_with_vis): Add cpp_constructor and cpp_destructor.
(DECL_CPP_CONSTRUCTOR_P, DECL_CPP_DESTRUCTOR_P): New macros.
* cp-tree.h (DECL_CONSTRUCTOR_P, DECL_DESTRUCTOR_P): Change to
strip templates and set the middle-end flag.
Index: tree.h
===
--- tree.h (revision 201977)
+++ tree.h (working copy)
@@ -3232,8 +3232,12 @@ struct GTY(()) tree_decl_with_vis {
/* Used by C++ only. Might become a generic decl flag. */
unsigned shadowed_for_var_p : 1;
/* Belong to FUNCTION_DECL exclusively. */
+ unsigned cpp_constructor : 1;
+ /* Belong to FUNCTION_DECL exclusively. */
+ unsigned cpp_destructor : 1;
+ /* Belong to FUNCTION_DECL exclusively. */
Naming suggestion: we already use cpp_ as prefix for the C preprocessor.
Could we please not use it for C++, given that tree is used by so
many clients for just about anything. cxx_ is a good alternative.
unsigned final : 1;
- /* 13 unused bits. */
+ /* 11 unused bits. */
};
extern tree decl_debug_expr_lookup (tree);
@@ -3483,6 +3487,18 @@ extern vectree, va_gc **decl_debug_arg
#define DECL_FUNCTION_VERSIONED(NODE)\
(FUNCTION_DECL_CHECK (NODE)-function_decl.versioned_function)
+/* In FUNCTION_DECL, this is set if this function is a C++ constructor.
+ Devirtualization machinery uses this knowledge for determing type of the
+ object constructed. Also we assume that constructor address is not
+ important. */
+#define DECL_CPP_CONSTRUCTOR_P(NODE)\
Same here.
+ (FUNCTION_DECL_CHECK (NODE)-decl_with_vis.cpp_constructor)
+
+/* In FUNCTION_DECL, this is set if this function is a C++ destructor.
+ Devirtualization machinery uses this to track types in destruction. */
+#define DECL_CPP_DESTRUCTOR_P(NODE)\
+ (FUNCTION_DECL_CHECK (NODE)-decl_with_vis.cpp_destructor)
+
Likewise.
/* In FUNCTION_DECL that represent an virtual method this is set when
the method is final. */
#define DECL_FINAL_P(NODE)\
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 201977)
+++ cp/cp-tree.h(working copy)
@@ -2121,9 +2121,10 @@ struct GTY((variable_size)) lang_decl {
#define SET_DECL_LANGUAGE(NODE, LANGUAGE) \
(DECL_LANG_SPECIFIC (NODE)-u.base.language = (LANGUAGE))
-/* For FUNCTION_DECLs: nonzero means that this function is a constructor. */
+/* For FUNCTION_DECLs and TEMPLATE_DECLs: nonzero means that this function
+ is a constructor. */
#define DECL_CONSTRUCTOR_P(NODE) \
- (LANG_DECL_FN_CHECK (NODE)-constructor_attr)
+ DECL_CPP_CONSTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a constructor for a complete
object. */
@@ -2152,9 +2153,10 @@ struct GTY((variable_size)) lang_decl {
#define DECL_MOVE_CONSTRUCTOR_P(NODE) \
(DECL_CONSTRUCTOR_P (NODE) move_fn_p (NODE))
-/* Nonzero if NODE is a destructor. */
+/* Nonzero if NODE (a FUNCTION_DECL or TEMPLATE_DECL)
+ is a destructor. */
#define DECL_DESTRUCTOR_P(NODE)\
- (LANG_DECL_FN_CHECK (NODE)-destructor_attr)
+ DECL_CPP_DESTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a destructor, but not the
specialized in-charge constructor, in-charge deleting constructor,
Re: Clean up pretty printers [15/n]
Joseph S. Myers jos...@codesourcery.com writes:
[...]
| | @@ -379,15 +375,15 @@
| |switch (code)
| | {
| | case INTEGER_TYPE:
| | - pp_string (pp, (TYPE_UNSIGNED (t)
| | - ? M_(unnamed-unsigned:)
| | - : M_(unnamed-signed:)));
| | + pp-translate_string (TYPE_UNSIGNED (t)
| | +? unnamed-unsigned:
| | +: unnamed-signed:);
| |
| | may need each case of the conditional expression to be marked for
| | extraction for translation, or to be separated into two separate calls
| | using if (we've had that issue before with conditional expressions in
| | diagnostics).
|
| Hmm, why would that be needed now, and not before?
| (not that I am found of the conditional, but only by curiosity.)
|
| Previously, each string was inside a separate call to M_() - the strings
| themselves were the msgid parameters. Now, the msgid parameter is not a
| single string but a more complicated expression and xgettext may not
| handle such expressions properly the way it handles having just a single
| string as parameter.
OK, thanks the explanation.
Do you think the same issue arise with diagnostic_set_info,
diagnostic_append_note?
-- Gaby
Clean up pretty printers [16/n]
Same topic as patch 14/n. This time for primary_expression.
-- Gaby
2013-08-25 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::primary_expression): Now a
virtua member function.
(pp_primary_expression): Adjust.
(pp_c_primary_expression): Remove.
* c-pretty-print.c (c_pretty_printer::primary_expression): Rename
from pp_c_primary_expression. Adjust.
(pp_c_initializer_list): Use pp_primary_expression.
(c_pretty_printer::c_pretty_printer): Do not assign to
primary_expression.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::primary_expression): Now
an overrider of c_pretty_printer::primary_expression.
* cxx-pretty-print.c (cxx_pretty_printer::primary_expression):
Rename from pp_cxx_primary_expression. Adjust.
(pp_cxx_postfix_expression): Use pp_primary_expression.
(pp_cxx_ctor_initializer): Likewise.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
primary_expression.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201977)
+++ c-family/c-pretty-print.c (working copy)
@@ -1212,7 +1212,7 @@
( expression ) */
void
-pp_c_primary_expression (c_pretty_printer *pp, tree e)
+c_pretty_printer::primary_expression (tree e)
{
switch (TREE_CODE (e))
{
@@ -1222,49 +1222,49 @@
case CONST_DECL:
case FUNCTION_DECL:
case LABEL_DECL:
- pp_c_tree_decl_identifier (pp, e);
+ pp_c_tree_decl_identifier (this, e);
break;
case IDENTIFIER_NODE:
- pp_c_tree_identifier (pp, e);
+ pp_c_tree_identifier (this, e);
break;
case ERROR_MARK:
- pp-translate_string (erroneous-expression);
+ translate_string (erroneous-expression);
break;
case RESULT_DECL:
- pp-translate_string (return-value);
+ translate_string (return-value);
break;
case INTEGER_CST:
case REAL_CST:
case FIXED_CST:
case STRING_CST:
- pp_constant (pp, e);
+ constant (e);
break;
case TARGET_EXPR:
- pp_c_ws_string (pp, __builtin_memcpy);
- pp_c_left_paren (pp);
- pp_ampersand (pp);
- pp_primary_expression (pp, TREE_OPERAND (e, 0));
- pp_separate_with (pp, ',');
- pp_ampersand (pp);
- pp_initializer (pp, TREE_OPERAND (e, 1));
+ pp_c_ws_string (this, __builtin_memcpy);
+ pp_c_left_paren (this);
+ pp_ampersand (this);
+ primary_expression (TREE_OPERAND (e, 0));
+ pp_separate_with (this, ',');
+ pp_ampersand (this);
+ pp_initializer (this, TREE_OPERAND (e, 1));
if (TREE_OPERAND (e, 2))
{
- pp_separate_with (pp, ',');
- pp_c_expression (pp, TREE_OPERAND (e, 2));
+ pp_separate_with (this, ',');
+ pp_c_expression (this, TREE_OPERAND (e, 2));
}
- pp_c_right_paren (pp);
+ pp_c_right_paren (this);
break;
default:
/* FIXME: Make sure we won't get into an infinite loop. */
- pp_c_left_paren (pp);
- pp_expression (pp, e);
- pp_c_right_paren (pp);
+ pp_c_left_paren (this);
+ pp_expression (this, e);
+ pp_c_right_paren (this);
break;
}
}
@@ -1356,7 +1356,7 @@
if (code == RECORD_TYPE || code == UNION_TYPE)
{
pp_c_dot (pp);
- pp_c_primary_expression (pp, TREE_PURPOSE (init));
+ pp_primary_expression (pp, TREE_PURPOSE (init));
}
else
{
@@ -2119,7 +2119,7 @@
Implementation note: instead of going through the usual recursion
chain, I take the liberty of dispatching nodes to the appropriate
functions. This makes some redundancy, but it worths it. That also
- prevents a possible infinite recursion between pp_c_primary_expression ()
+ prevents a possible infinite recursion between pp_primary_expression ()
and pp_c_expression (). */
void
@@ -2344,7 +2344,6 @@
statement = pp_c_statement;
- primary_expression= pp_c_primary_expression;
postfix_expression= pp_c_postfix_expression;
unary_expression = pp_c_unary_expression;
initializer = pp_c_initializer;
Index: c-family/c-pretty-print.h
===
--- c-family/c-pretty-print.h (revision 201977)
+++ c-family/c-pretty-print.h (working copy)
@@ -56,6 +56,7 @@
virtual void constant (tree);
virtual void id_expression (tree);
+ virtual void primary_expression (tree);
/* Points to the first element of an array of offset-list.
Not used yet. */
int *offset_list;
@@ -81,7 +82,6 @@
c_pretty_print_fn statement;
- c_pretty_print_fn primary_expression;
c_pretty_print_fn postfix_expression;
c_pretty_print_fn unary_expression
Ada PATCH: Fix ada/58239 by linking with xg++, not xgcc
Hi, My earlier patch that formally desclared pretty_printer as polymorphic uncovered a latent bug in the existing build machinery of the Ada component. It had been using xgcc to link against C++ source files. It should use xg++ instead. This patch fixes that by introducing GXX_LINK which is GCC_LINK except that CXX (e.g. xg++) instead of CC (e.g. xgcc) is invoked. Eric, are there other executables that need to be linked with GXX_LINK too but aren't triggered yet? Thanks, -- Gaby ada/ 2013-08-25 Gabriel Dos Reis g...@integrable-solutions.net * gcc-interface/Makefile.in (GXX_LINK): New. Same as GCC_LINK but with xgcc replaced with xgcc. (common-tools): Use it. (../../gnatmake$(exeext)): Likewise. (../../gnatlink$(exeext)): Likewise. Index: ada/gcc-interface/Makefile.in === --- ada/gcc-interface/Makefile.in (revision 201977) +++ ada/gcc-interface/Makefile.in (working copy) @@ -2398,6 +2398,7 @@ GNATBIND=$(GNATBIND) GCC_LINK=$(CC) $(GCC_LINK_FLAGS) $(ADA_INCLUDES) +GXX_LINK=$(CXX) $(GCC_LINK_FLAGS) $(ADA_INCLUDES) # Build directory for the tools. Let's copy the target-dependent # sources using the same mechanism as for gnatlib. The other sources are @@ -2462,23 +2463,23 @@ gnatchop gnatcmd gnatkr gnatls gnatprep gnatxref gnatfind gnatname \ gnatclean -bargs $(ADA_INCLUDES) $(GNATBIND_FLAGS) $(GNATLINK) -v gnatcmd -o ../../gnat$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatchop -o ../../gnatchop$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatkr -o ../../gnatkr$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatls -o ../../gnatls$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatprep -o ../../gnatprep$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatxref -o ../../gnatxref$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatfind -o ../../gnatfind$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatname -o ../../gnatname$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) $(GNATLINK) -v gnatclean -o ../../gnatclean$(exeext) \ - --GCC=$(GCC_LINK) $(TOOLS_LIBS) + --GCC=$(GXX_LINK) $(TOOLS_LIBS) ../../gnatsym$(exeext): ../stamp-tools $(GNATMAKE) -c $(ADA_INCLUDES) gnatsym --GCC=$(CC) $(ALL_ADAFLAGS) @@ -2519,11 +2520,11 @@ # Likewise for the tools ../../gnatmake$(exeext): $(P) b_gnatm.o $(GNATMAKE_OBJS) - +$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatm.o $(GNATMAKE_OBJS) \ + +$(GXX_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatm.o $(GNATMAKE_OBJS) \ $(TOOLS_LIBS) ../../gnatlink$(exeext): $(P) b_gnatl.o $(GNATLINK_OBJS) - +$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatl.o $(GNATLINK_OBJS) \ + +$(GXX_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatl.o $(GNATLINK_OBJS) \ $(TOOLS_LIBS) ../stamp-gnatlib-$(RTSDIR):
Clean up pretty printers [17/n]
Same topic as patch 15/n, but postfix_expression.
Tested on an x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-25 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::postfix_expression): Now a
virtual member function.
(pp_postfix_expression): Adjust.
(pp_c_postfix_expression): Remove.
* c-pretty-print.c (c_pretty_printer::postfix_expression): Rename
from pp_c_postfix_expression. Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
postfix_expression.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::postfix_expression):
Declare as overrider.
* cxx-pretty-print.c (cxx_pretty_printer::postfix_expression):
Rename from pp_cxx_postfix_expression. Adjust.
(pp_cxx_expression): Use pp_postfix_expression.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
postfix_expression.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201984)
+++ c-family/c-pretty-print.c (working copy)
@@ -1463,112 +1463,112 @@
( type-name ) { initializer-list , } */
void
-pp_c_postfix_expression (c_pretty_printer *pp, tree e)
+c_pretty_printer::postfix_expression (tree e)
{
enum tree_code code = TREE_CODE (e);
switch (code)
{
case POSTINCREMENT_EXPR:
case POSTDECREMENT_EXPR:
- pp_postfix_expression (pp, TREE_OPERAND (e, 0));
- pp_string (pp, code == POSTINCREMENT_EXPR ? ++ : --);
+ postfix_expression (TREE_OPERAND (e, 0));
+ pp_string (this, code == POSTINCREMENT_EXPR ? ++ : --);
break;
case ARRAY_REF:
- pp_postfix_expression (pp, TREE_OPERAND (e, 0));
- pp_c_left_bracket (pp);
- pp_expression (pp, TREE_OPERAND (e, 1));
- pp_c_right_bracket (pp);
+ postfix_expression (TREE_OPERAND (e, 0));
+ pp_c_left_bracket (this);
+ pp_expression (this, TREE_OPERAND (e, 1));
+ pp_c_right_bracket (this);
break;
case ARRAY_NOTATION_REF:
- pp_postfix_expression (pp, ARRAY_NOTATION_ARRAY (e));
- pp_c_left_bracket (pp);
- pp_expression (pp, ARRAY_NOTATION_START (e));
- pp_colon (pp);
- pp_expression (pp, ARRAY_NOTATION_LENGTH (e));
- pp_colon (pp);
- pp_expression (pp, ARRAY_NOTATION_STRIDE (e));
- pp_c_right_bracket (pp);
+ postfix_expression (ARRAY_NOTATION_ARRAY (e));
+ pp_c_left_bracket (this);
+ pp_expression (this, ARRAY_NOTATION_START (e));
+ pp_colon (this);
+ pp_expression (this, ARRAY_NOTATION_LENGTH (e));
+ pp_colon (this);
+ pp_expression (this, ARRAY_NOTATION_STRIDE (e));
+ pp_c_right_bracket (this);
break;
case CALL_EXPR:
{
call_expr_arg_iterator iter;
tree arg;
- pp_postfix_expression (pp, CALL_EXPR_FN (e));
- pp_c_left_paren (pp);
+ postfix_expression (CALL_EXPR_FN (e));
+ pp_c_left_paren (this);
FOR_EACH_CALL_EXPR_ARG (arg, iter, e)
{
- pp_expression (pp, arg);
+ pp_expression (this, arg);
if (more_call_expr_args_p (iter))
- pp_separate_with (pp, ',');
+ pp_separate_with (this, ',');
}
- pp_c_right_paren (pp);
+ pp_c_right_paren (this);
break;
}
case UNORDERED_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? isunordered
: __builtin_isunordered);
goto two_args_fun;
case ORDERED_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !isunordered
: !__builtin_isunordered);
goto two_args_fun;
case UNLT_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !isgreaterequal
: !__builtin_isgreaterequal);
goto two_args_fun;
case UNLE_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !isgreater
: !__builtin_isgreater);
goto two_args_fun;
case UNGT_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !islessequal
: !__builtin_islessequal);
goto two_args_fun;
case UNGE_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !isless
: !__builtin_isless);
goto two_args_fun;
case UNEQ_EXPR:
- pp_c_ws_string (pp, flag_isoc99
+ pp_c_ws_string (this, flag_isoc99
? !islessgreater
: !__builtin_islessgreater);
goto
Clean up pretty printers [13/n]
Most of the specialized pretty printing functions from the C-family
languages are really virtual functions. This patchlet makes the first
explicitly so.
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
c-family/
2013-08-24 Gabriel Dos Reis g...@integrable-solutions.net
* c-pretty-print.h (c_pretty_printer::constant): Now a virtual
member function.
(pp_constant): Adjust.
(pp_c_constant): Remove.
* c-pretty-print.c (c_pretty_printer::constant): Rename from
pp_c_constant. Adjust.
(pp_c_constant)
(pp_c_primary_expression): Call pp_constant in lieu of pp_c_constant.
(c_pretty_printer::c_pretty_printer): Remove assignment to constant.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::constant): Now a member
function, overriding c_pretty_printer::constant.
* cxx-pretty-print.c (cxx_pretty_printer::constant): Rename from
pp_cxx_constant. Adjust.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to constant.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201968)
+++ c-family/c-pretty-print.c (working copy)
@@ -1130,7 +1130,7 @@
character-constant */
void
-pp_c_constant (c_pretty_printer *pp, tree e)
+c_pretty_printer::constant (tree e)
{
const enum tree_code code = TREE_CODE (e);
@@ -1140,38 +1140,38 @@
{
tree type = TREE_TYPE (e);
if (type == boolean_type_node)
- pp_c_bool_constant (pp, e);
+ pp_c_bool_constant (this, e);
else if (type == char_type_node)
- pp_c_character_constant (pp, e);
+ pp_c_character_constant (this, e);
else if (TREE_CODE (type) == ENUMERAL_TYPE
- pp_c_enumeration_constant (pp, e))
+ pp_c_enumeration_constant (this, e))
;
else
- pp_c_integer_constant (pp, e);
+ pp_c_integer_constant (this, e);
}
break;
case REAL_CST:
- pp_c_floating_constant (pp, e);
+ pp_c_floating_constant (this, e);
break;
case FIXED_CST:
- pp_c_fixed_constant (pp, e);
+ pp_c_fixed_constant (this, e);
break;
case STRING_CST:
- pp_c_string_literal (pp, e);
+ pp_c_string_literal (this, e);
break;
case COMPLEX_CST:
/* Sometimes, we are confused and we think a complex literal
is a constant. Such thing is a compound literal which
grammatically belongs to postfix-expr production. */
- pp_c_compound_literal (pp, e);
+ pp_c_compound_literal (this, e);
break;
default:
- pp_unsupported_tree (pp, e);
+ pp_unsupported_tree (this, e);
break;
}
}
@@ -1236,7 +1236,7 @@
case REAL_CST:
case FIXED_CST:
case STRING_CST:
- pp_c_constant (pp, e);
+ pp_constant (pp, e);
break;
case TARGET_EXPR:
@@ -1357,7 +1357,7 @@
{
pp_c_left_bracket (pp);
if (TREE_PURPOSE (init))
- pp_c_constant (pp, TREE_PURPOSE (init));
+ pp_constant (pp, TREE_PURPOSE (init));
pp_c_right_bracket (pp);
}
pp_c_whitespace (pp);
@@ -2339,7 +2339,6 @@
statement = pp_c_statement;
- constant = pp_c_constant;
id_expression = pp_c_id_expression;
primary_expression= pp_c_primary_expression;
postfix_expression= pp_c_postfix_expression;
Index: c-family/c-pretty-print.h
===
--- c-family/c-pretty-print.h (revision 201968)
+++ c-family/c-pretty-print.h (working copy)
@@ -51,6 +51,7 @@
{
c_pretty_printer ();
+ virtual void constant (tree);
/* Points to the first element of an array of offset-list.
Not used yet. */
int *offset_list;
@@ -76,7 +77,6 @@
c_pretty_print_fn statement;
- c_pretty_print_fn constant;
c_pretty_print_fn id_expression;
c_pretty_print_fn primary_expression;
c_pretty_print_fn postfix_expression;
@@ -109,7 +109,7 @@
#define pp_statement(PP, S) (PP)-statement (PP, S)
-#define pp_constant(PP, E) (PP)-constant (PP, E)
+#define pp_constant(PP, E) (PP)-constant (E)
#define pp_id_expression(PP, E) (PP)-id_expression (PP, E)
#define pp_primary_expression(PP, E)(PP)-primary_expression (PP, E)
#define pp_postfix_expression(PP, E)(PP)-postfix_expression (PP, E)
@@ -169,7 +169,6 @@
void pp_c_postfix_expression (c_pretty_printer *, tree);
void pp_c_primary_expression (c_pretty_printer *, tree);
void pp_c_init_declarator (c_pretty_printer *, tree);
-void pp_c_constant (c_pretty_printer *, tree);
void pp_c_id_expression (c_pretty_printer *, tree);
void pp_c_ws_string (c_pretty_printer *, const char *);
void pp_c_identifier
Clean up pretty printers [14/n]
Same as previous topic; for id_expression.
-- Gaby
2013-08-24 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::id_expression): Now a
virtual function.
(pp_c_id_expression): Remove.
(pp_id_expression): Adjust.
* c-pretty-print.c (c_pretty_printer::id_expression): Rename from
pp_c_id_expression. Adjust.
(pp_c_postfix_expression): Use pp_id_expression.
(c_pretty_printer::c_pretty_printer): Do not assign to id_expression.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::id_expression): Declare.
* cxx-pretty-print.c (cxx_pretty_printer::id_expression): Rename
from pp_cxx_id_expression. Adjust.
(pp_cxx_userdef_literal): Use pp_id_expression.
(pp_cxx_primary_expression): Likewise.
(pp_cxx_direct_declarator): Likewise.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
id_expression.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201969)
+++ c-family/c-pretty-print.c (working copy)
@@ -1422,7 +1422,7 @@
identifier */
void
-pp_c_id_expression (c_pretty_printer *pp, tree t)
+c_pretty_printer::id_expression (tree t)
{
switch (TREE_CODE (t))
{
@@ -1433,15 +1433,15 @@
case FUNCTION_DECL:
case FIELD_DECL:
case LABEL_DECL:
- pp_c_tree_decl_identifier (pp, t);
+ pp_c_tree_decl_identifier (this, t);
break;
case IDENTIFIER_NODE:
- pp_c_tree_identifier (pp, t);
+ pp_c_tree_identifier (this, t);
break;
default:
- pp_unsupported_tree (pp, t);
+ pp_unsupported_tree (this, t);
break;
}
}
@@ -1645,7 +1645,7 @@
case ADDR_EXPR:
if (TREE_CODE (TREE_OPERAND (e, 0)) == FUNCTION_DECL)
{
- pp_c_id_expression (pp, TREE_OPERAND (e, 0));
+ pp_id_expression (pp, TREE_OPERAND (e, 0));
break;
}
/* else fall through. */
@@ -2339,7 +2339,6 @@
statement = pp_c_statement;
- id_expression = pp_c_id_expression;
primary_expression= pp_c_primary_expression;
postfix_expression= pp_c_postfix_expression;
unary_expression = pp_c_unary_expression;
Index: c-family/c-pretty-print.h
===
--- c-family/c-pretty-print.h (revision 201969)
+++ c-family/c-pretty-print.h (working copy)
@@ -52,6 +52,7 @@
c_pretty_printer ();
virtual void constant (tree);
+ virtual void id_expression (tree);
/* Points to the first element of an array of offset-list.
Not used yet. */
int *offset_list;
@@ -77,7 +78,6 @@
c_pretty_print_fn statement;
- c_pretty_print_fn id_expression;
c_pretty_print_fn primary_expression;
c_pretty_print_fn postfix_expression;
c_pretty_print_fn unary_expression;
@@ -110,7 +110,7 @@
#define pp_statement(PP, S) (PP)-statement (PP, S)
#define pp_constant(PP, E) (PP)-constant (E)
-#define pp_id_expression(PP, E) (PP)-id_expression (PP, E)
+#define pp_id_expression(PP, E) (PP)-id_expression (E)
#define pp_primary_expression(PP, E)(PP)-primary_expression (PP, E)
#define pp_postfix_expression(PP, E)(PP)-postfix_expression (PP, E)
#define pp_unary_expression(PP, E) (PP)-unary_expression (PP, E)
@@ -169,7 +169,6 @@
void pp_c_postfix_expression (c_pretty_printer *, tree);
void pp_c_primary_expression (c_pretty_printer *, tree);
void pp_c_init_declarator (c_pretty_printer *, tree);
-void pp_c_id_expression (c_pretty_printer *, tree);
void pp_c_ws_string (c_pretty_printer *, const char *);
void pp_c_identifier (c_pretty_printer *, const char *);
void pp_c_string_literal (c_pretty_printer *, tree);
Index: cp/cxx-pretty-print.c
===
--- cp/cxx-pretty-print.c (revision 201969)
+++ cp/cxx-pretty-print.c (working copy)
@@ -355,15 +355,15 @@
unqualified-id
qualified-id */
-static inline void
-pp_cxx_id_expression (cxx_pretty_printer *pp, tree t)
+void
+cxx_pretty_printer::id_expression (tree t)
{
if (TREE_CODE (t) == OVERLOAD)
t = OVL_CURRENT (t);
if (DECL_P (t) DECL_CONTEXT (t))
-pp_cxx_qualified_id (pp, t);
+pp_cxx_qualified_id (this, t);
else
-pp_cxx_unqualified_id (pp, t);
+pp_cxx_unqualified_id (this, t);
}
/* user-defined literal:
@@ -373,7 +373,7 @@
pp_cxx_userdef_literal (cxx_pretty_printer *pp, tree t)
{
pp_constant (pp, USERDEF_LITERAL_VALUE (t));
- pp_cxx_id_expression (pp, USERDEF_LITERAL_SUFFIX_ID (t));
+ pp_id_expression (pp, USERDEF_LITERAL_SUFFIX_ID (t));
}
@@ -436,7 +436,7 @@
case OVERLOAD:
case CONST_DECL:
case TEMPLATE_DECL:
- pp_cxx_id_expression (pp, t);
+ pp_id_expression (pp, t);
break
Clean up pretty printers [15/n]
Instead of defining the same macro several times in different
translation units, we can just make it a function and use it where
needed.
Tested on an x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-25 Gabriel Dos Reis g...@integrable-solutions.net
c-family/
* c-pretty-print.h (c_pretty_printer::translate_string): Declare.
* c-pretty-print.c (M_): Remove.
(c_pretty_printer::translate_string): Define.
(pp_c_type_specifier): Use it.
(pp_c_primary_expression): Likewise.
(pp_c_expression): Likewise.
cp/
* cxx-pretty-print.c (M_): Remove.
(pp_cxx_unqualified_id): Use translate_string instead of M_.
(pp_cxx_canonical_template_parameter): Likewise.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201973)
+++ c-family/c-pretty-print.c (working copy)
@@ -29,10 +29,6 @@
#include tree-iterator.h
#include diagnostic.h
-/* Translate if being used for diagnostics, but not for dump files or
- __PRETTY_FUNCTION. */
-#define M_(msgid) (pp_translate_identifiers (pp) ? _(msgid) : (msgid))
-
/* The pretty-printer code is primarily designed to closely follow
(GNU) C and C++ grammars. That is to be contrasted with spaghetti
codes we used to have in the past. Following a structured
@@ -341,7 +337,7 @@
switch (code)
{
case ERROR_MARK:
- pp_c_ws_string (pp, M_(type-error));
+ pp-translate_string (type-error);
break;
case IDENTIFIER_NODE:
@@ -379,15 +375,15 @@
switch (code)
{
case INTEGER_TYPE:
- pp_string (pp, (TYPE_UNSIGNED (t)
- ? M_(unnamed-unsigned:)
- : M_(unnamed-signed:)));
+ pp-translate_string (TYPE_UNSIGNED (t)
+? unnamed-unsigned:
+: unnamed-signed:);
break;
case REAL_TYPE:
- pp_string (pp, M_(unnamed-float:));
+ pp-translate_string (unnamed-float:);
break;
case FIXED_POINT_TYPE:
- pp_string (pp, M_(unnamed-fixed:));
+ pp-translate_string (unnamed-fixed:);
break;
default:
gcc_unreachable ();
@@ -402,7 +398,7 @@
if (DECL_NAME (t))
pp_id_expression (pp, t);
else
- pp_c_ws_string (pp, M_(typedef-error));
+ pp-translate_string (typedef-error);
break;
case UNION_TYPE:
@@ -415,12 +411,12 @@
else if (code == ENUMERAL_TYPE)
pp_c_ws_string (pp, enum);
else
- pp_c_ws_string (pp, M_(tag-error));
+ pp-translate_string (tag-error);
if (TYPE_NAME (t))
pp_id_expression (pp, TYPE_NAME (t));
else
- pp_c_ws_string (pp, M_(anonymous));
+ pp-translate_string (anonymous);
break;
default:
@@ -1187,6 +1183,15 @@
pp-padding = pp_before;
}
+void
+c_pretty_printer::translate_string (const char *gmsgid)
+{
+ if (pp_translate_identifiers (this))
+pp_c_ws_string (this, _(gmsgid));
+ else
+pp_c_ws_string (this, gmsgid);
+}
+
/* Pretty-print an IDENTIFIER_NODE, which may contain UTF-8 sequences
that need converting to the locale encoding, preceded by whitespace
is necessary. */
@@ -1225,11 +1230,11 @@
break;
case ERROR_MARK:
- pp_c_ws_string (pp, M_(erroneous-expression));
+ pp-translate_string (erroneous-expression);
break;
case RESULT_DECL:
- pp_c_ws_string (pp, M_(return-value));
+ pp-translate_string (return-value);
break;
case INTEGER_CST:
@@ -2155,7 +2160,7 @@
!DECL_ARTIFICIAL (SSA_NAME_VAR (e)))
pp_c_expression (pp, SSA_NAME_VAR (e));
else
- pp_c_ws_string (pp, M_(unknown));
+ pp-translate_string (unknown);
break;
case POSTINCREMENT_EXPR:
Index: c-family/c-pretty-print.h
===
--- c-family/c-pretty-print.h (revision 201973)
+++ c-family/c-pretty-print.h (working copy)
@@ -51,6 +51,9 @@
{
c_pretty_printer ();
+ // Format string, possibly translated.
+ void translate_string (const char *);
+
virtual void constant (tree);
virtual void id_expression (tree);
/* Points to the first element of an array of offset-list.
Index: cp/cxx-pretty-print.c
===
--- cp/cxx-pretty-print.c (revision 201973)
+++ cp/cxx-pretty-print.c (working copy)
@@ -27,10 +27,6 @@
#include cxx-pretty-print.h
#include tree-pretty-print.h
-/* Translate if being used for diagnostics, but not for dump files or
- __PRETTY_FUNCTION. */
-#define M_(msgid) (pp_translate_identifiers (pp) ? _(msgid) : (msgid))
-
static void
Clean up pretty printers [11/n]
This patchlet is an easy low hanging fruit in the pile of local patches
I have. It turns old style emulation of inline functions into real inline
functions.
Tested on x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-23 Gabriel Dos Reis g...@integrable-solutions.net
* pretty-print.h (pp_newline_and_flush): Declare. Remove macro
definition.
(pp_newline_and_indent): Likewise.
(pp_separate_with): Likewise.
* pretty-print.c (pp_newline_and_flush): Define.
(pp_newline_and_indent): Likewise.
(pp_separate_with): Likewise.
Index: pretty-print.c
===
--- pretty-print.c (revision 201939)
+++ pretty-print.c (working copy)
@@ -902,6 +902,37 @@
pp-padding = pp_none;
}
}
+
+// Add a newline to the pretty printer PP and flush formatted text.
+
+void
+pp_newline_and_flush (pretty_printer *pp)
+{
+ pp_newline (pp);
+ pp_flush (pp);
+ pp_needs_newline (pp) = false;
+}
+
+// Add a newline to the pretty printer PP, followed by indentation.
+
+void
+pp_newline_and_indent (pretty_printer *pp, int n)
+{
+ pp_indentation (pp) += n;
+ pp_newline (pp);
+ pp_indent (pp);
+ pp_needs_newline (pp) = false;
+}
+
+// Add separator C, followed by a single whitespace.
+
+void
+pp_separate_with (pretty_printer *pp, char c)
+{
+ pp_character (pp, c);
+ pp_space (pp);
+}
+
/* The string starting at P has LEN (at least 1) bytes left; if they
start with a valid UTF-8 sequence, return the length of that
Index: pretty-print.h
===
--- pretty-print.h (revision 201939)
+++ pretty-print.h (working copy)
@@ -246,26 +246,8 @@
#define pp_backquote(PP)pp_character (PP, '`')
#define pp_doublequote(PP) pp_character (PP, '')
#define pp_underscore(PP) pp_character (PP, '_')
-#define pp_newline_and_flush(PP) \
- do { \
-pp_newline (PP); \
-pp_flush (PP); \
-pp_needs_newline (PP) = false; \
- } while (0)
-#define pp_newline_and_indent(PP, N) \
- do { \
-pp_indentation (PP) += N;\
-pp_newline (PP); \
-pp_indent (PP); \
-pp_needs_newline (PP) = false; \
- } while (0)
#define pp_maybe_newline_and_indent(PP, N) \
if (pp_needs_newline (PP)) pp_newline_and_indent (PP, N)
-#define pp_separate_with(PP, C) \
- do { \
- pp_character (PP, C); \
- pp_space (PP); \
- } while (0)
#define pp_scalar(PP, FORMAT, SCALAR)\
do \
{\
@@ -298,6 +280,9 @@
extern const char *pp_last_position_in_text (const pretty_printer *);
extern void pp_emit_prefix (pretty_printer *);
extern void pp_append_text (pretty_printer *, const char *, const char *);
+extern void pp_newline_and_flush (pretty_printer *);
+extern void pp_newline_and_indent (pretty_printer *, int);
+extern void pp_separate_with (pretty_printer *, char);
/* If we haven't already defined a front-end-specific diagnostics
style, use the generic one. */
Clean up pretty printers [12/n]
The primary function of this patch is to formally make the class
pretty_printer polymorphic (in the OO sense.) It has always been
conceptually polymorphic -- we just didn't have linguistic support to
say so in C. As a secondary change, the patch makes a systematic use of
pp_buffer -- making it easier to search for a pretty printer's output
buffer access.
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-08-23 Gabriel Dos Reis g...@integrable-solutions.net
* diagnostic.c (diagnostic_set_caret_max_width): Use pp_buffer.
* gimple-pretty-print.c (gimple_dump_bb_buff): Likewise.
* pretty-print.c (pp_formatted_text_data): Likewise.
(pp_write_text_to_stream): Likewise.
(pp_write_text_as_dot_label_to_stream): Likewise.
(pp_append_r): Likewise.
(pp_format): Likewise.
(pp_flush): Likewise.
(pp_clear_output_area): Likewise.
(pp_append_text): Likewise.
(pp_formatted_text): Likewise.
(pp_remaining_character_count_for_line): Likewise.
(pp_newline): Likewise.
(pp_character): Likewise.
(output_buffer::~output_buffer): Define.
(pretty_printer::~pretty_printer): Destruct output buffer.
* pretty-print.h (output_buffer::~output_buffer): Declare.
(pretty_printer::~pretty_printer): Declare virtual.
c/
* c-objc-common.c (c_tree_printer): Document the nature of the cast.
(c_initialize_diagnostics): Call a destructor for the early printer.
cp/
* cp-objcp-common.c (cxx_initialize_diagnostics): Call a
destructor for the early printer.
* error.c (type_to_string): Use pp_buffer.
Index: c/c-objc-common.c
===
--- c/c-objc-common.c (revision 201955)
+++ c/c-objc-common.c (working copy)
@@ -92,6 +92,7 @@
{
tree t = NULL_TREE;
tree name;
+ // FIXME: the next cast should be a dynamic_cast, when it is permitted.
c_pretty_printer *cpp = (c_pretty_printer *) pp;
pp-padding = pp_none;
@@ -192,6 +193,7 @@
context-printer = new (pp) c_pretty_printer ();
/* It is safe to free this object because it was previously XNEW()'d. */
+ base-~pretty_printer ();
XDELETE (base);
}
Index: cp/cp-objcp-common.c
===
--- cp/cp-objcp-common.c(revision 201955)
+++ cp/cp-objcp-common.c(working copy)
@@ -140,6 +140,7 @@
context-printer = new (pp) cxx_pretty_printer ();
/* It is safe to free this object because it was previously XNEW()'d. */
+ base-~pretty_printer ();
XDELETE (base);
}
Index: cp/error.c
===
--- cp/error.c (revision 201955)
+++ cp/error.c (working copy)
@@ -2881,7 +2881,7 @@
!uses_template_parms (typ))
{
int aka_start; char *p;
- struct obstack *ob = cxx_pp-buffer-obstack;
+ struct obstack *ob = pp_buffer (cxx_pp)-obstack;
/* Remember the end of the initial dump. */
int len = obstack_object_size (ob);
tree aka = strip_typedefs (typ);
Index: diagnostic.c
===
--- diagnostic.c(revision 201955)
+++ diagnostic.c(working copy)
@@ -104,7 +104,7 @@
{
/* One minus to account for the leading empty space. */
value = value ? value - 1
-: (isatty (fileno (context-printer-buffer-stream))
+: (isatty (fileno (pp_buffer (context-printer)-stream))
? getenv_columns () - 1: INT_MAX);
if (value = 0)
Index: gimple-pretty-print.c
===
--- gimple-pretty-print.c (revision 201955)
+++ gimple-pretty-print.c (working copy)
@@ -2249,7 +2249,7 @@
pp_newline_and_flush (buffer);
gcc_checking_assert (DECL_STRUCT_FUNCTION (current_function_decl));
dump_histograms_for_stmt (DECL_STRUCT_FUNCTION (current_function_decl),
- buffer-buffer-stream, stmt);
+ pp_buffer(buffer)-stream, stmt);
}
dump_implicit_edges (buffer, bb, indent, flags);
Index: pretty-print.c
===
--- pretty-print.c (revision 201955)
+++ pretty-print.c (working copy)
@@ -46,9 +46,17 @@
obstack_init (chunk_obstack);
}
+// Release resources owned by an output buffer at the end of lifetime.
+
+output_buffer::~output_buffer ()
+{
+ obstack_free (chunk_obstack, obstack_finish (chunk_obstack));
+ obstack_free (formatted_obstack, obstack_finish (formatted_obstack));
+}
+
/* A pointer to the formatted diagnostic message. */
#define pp_formatted_text_data(PP) \
- ((const char *) obstack_base ((PP)-buffer-obstack))
+ ((const char *) obstack_base (pp_buffer (PP)-obstack))
/* Format an integer given by va_arg (ARG, type-specifier T) where
type-specifier
Re: [C++ patch] Set attributes for C++ runtime library calls
On Fri, Aug 23, 2013 at 7:32 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 22, 2013, at 7:16 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
But even so, in light of this, I don't think you original assertion is
definitive.
Nothing is ever definitive. Now, if you want to say I quoted something
wrong, or that I am reading the standard wrong, or that it doesn't apply as I
think it does, feel free to point this out. I do view this as as a two way
street, despite the certainty I might profess.
Ha!
If you quoted the standard to back up your assertions, I would have been
able to feel free to point this out :-)
The thing is I am still trying to figure out what (1) what you would have liked;
(2) what you believe the standards mandate, with appropriate quote; and
(3) what you consider QoI.
(3) would be a matter of GCC design choice discussion: for example,
what we would like to guarantee under certain flags, etc.
You need to separate those three things so we can make progress.
Really?
statement, or, is it merely changing the value?
That is an assignment to an existing int storage.
And what if we do a memcpy (ip, j, sizeof (int));
Well, the case of 'memcpy' isn't defined by the standard
Odd, in the c++98 standard memcpy was defined by the standard, as well as the
open code that memcpy would be. I find memcpy in [diff.library]. I can
quote all the requirements that makes the open code work as well. It is
defined.
When I say the case of 'memcpy' isn't defined, I was not saying that
the function memcpy itself isn't defined by the standard. I was discussing
its effect in the scope of this discussion, e.g. lifetime vs. assignment.
That isn't defined in the standard.
You can see evidence that we meant for it to work here:
2 For any complete POD object type T, whether or not the object holds a
valid value of type T, the underlying bytes (_intro.memory_) making up
the object can be copied into an array of char or unsigned char.36) If
the content of the array of char or unsigned char is copied back into
the object, the object shall subsequently hold its original value.
[Example:
#define N sizeof(T)
char buf[N];
T obj; // obj initialized to its original value
memcpy(buf, obj, N); // between these two calls to memcpy,
// obj might be modified
memcpy(obj, buf, N); // at this point, each subobject of obj of
scalar type
// holds its original value
thought, I view this as completely redundant with the standard and should
just be a note. From the base standard, in C99 is it defined in 7.24.4.2.3
and I suspect remains largely unchanged from previous standards.
This does not say whether the effect of memcpy is assigment or copy
construction.
Which was the point I was making.
Is that reused, or merely changing the value.
The current proposal is that it constructs an int value, therefore
is moral equivalent of copy-constructor call.
I'd be interested in the final resolution. You have a DR number for the
issue or a paper where they talk about the issues?
This came out of a discussion of a larger issue on the SG12 mailing list.
I do not have the paper number yet since it is to be part of the Chicago
mailing list.
I think the most logical line of reasoning is that when the requirements of
[basic.lval] are met, the, this is a change of value of an object, not a
modification to it's lifetime.
Why?
Because if you end the lifetime of the original int, you destroy the semantic
that everyone knows for C. This cannot be done.
But:
(1) we are not talking about C
(2) C does not have a notion of lifetime -- at least not in the sense of C++.
So, whatever notion of semantics you think everyone knows of C
is largely irrelevant.
So, in the case quoted, since the type of the accesses are both int, we
don't reuse the storage, since the requirements of [basic.lval] are met.
Consider:
struct S {
S(int);
~S();
// …
};
int main() {
S s(8);
new (s) S(9); // #1
}
Is #1 a reuse of storage to create a new object or assignment?
Behaves as assignment, s exists post the new, til the closing }.
What does not mean concretely? Note that the // … could
have declared S::operator= to behave differently or be entirely deleted.
Indeed, the programmer expects that they can access i after *ip = j; and
that the _value_ that object, while changed from the original 1, will be 2
just after the *ip = j; statement.
Since we know that i must be 3 at the end, we then know what the wording,
reused, must mean, cause other meanings that could possibly make it work
for you in the case you are considering, would destroy this property of
pointers, and everyone knows the semantics of pointers, they are
undisputed. Or put another way, you cannot
Re: [C++ patch] Set attributes for C++ runtime library calls
On Fri, Aug 23, 2013 at 10:34 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 23, 2013, at 5:53 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
If you quoted the standard to back up your assertions, I would have been
able to feel free to point this out :-)
The thing is I am still trying to figure out what (1) what you would have
liked;
I've directly stated it, not sure how you missed it. The life of the
original object does not end.
You made a statement. It was not clear whether it was what you want
or whether it is what the standards mandate.
(2) what you believe the standards mandate, with appropriate quote; and
The life of the original object does not end. See below for the quote.
Your assertion or a quote from the standards?
Really?
statement, or, is it merely changing the value?
That is an assignment to an existing int storage.
And what if we do a memcpy (ip, j, sizeof (int));
Well, the case of 'memcpy' isn't defined by the standard
Odd, in the c++98 standard memcpy was defined by the standard, as well as
the open code that memcpy would be. I find memcpy in [diff.library]. I
can quote all the requirements that makes the open code work as well. It
is defined.
When I say the case of 'memcpy' isn't defined, I was not saying that
the function memcpy itself isn't defined by the standard. I was discussing
its effect in the scope of this discussion, e.g. lifetime vs. assignment.
That isn't defined in the standard.
And yet memcpy is exactly defined and all interactions with the entire rest
of the standard are as specified. Those semantics that are defined and
required, are, by definition. You can't make them go away by claiming they
are undefined.
I have no idea what exactly you are talking about.
If you have a rule in that standards that say that mempcy is assignment,
please share it with us.
int i = 1, j = 2;
{
memcpy (i, j)
// here i exists and is 2.
}
3.7.1 Static storage duration [basic.stc.static]
1 All objects which neither have dynamic storage duration nor are local
have static storage duration. The storage for these objects shall
last for the duration of the program (_basic.start.init_,
_basic.start.term_).
This covers why i exists.
The storage duration of an object is not necessarily the same
as its lifetime.
As for why i has the value 2, gosh:
7.21.2.1 The memcpy function
Synopsis
[#1]
#include lt;string.h
void *memcpy(void * restrict s1,
const void * restrict s2,
size_t n);
Description
[#2] The memcpy function copies n characters from the object
pointed to by s2 into the object pointed to by s1. If
copying takes place between objects that overlap, the
behavior is undefined.
1.6 The C++ memory model [intro.memory]
1 The fundamental storage unit in the C++ memory model is the byte.
An object of POD4) type
(_basic.types_) shall occupy contiguous bytes of storage.
5.3.3 Sizeof[expr.sizeof]
1 The sizeof operator yields the number of bytes in the object represen-
tation of its operand. The operand is either an expression, which is
not evaluated, or a parenthesized type-id. The sizeof operator shall
4 The object representation of an object of type T is the sequence of N
unsigned char objects taken up by the object of type T, where N equals
sizeof(T). The value representation of an object is the set of bits
that hold the value of type T. For POD types, the value representa-
tion is a set of bits in the object representation that determines a
value, which is one discrete element of an implementation-defined set
of values.37)
Yes, but what is the relevance?
You can see evidence that we meant for it to work here:
2 For any complete POD object type T, whether or not the object holds a
valid value of type T, the underlying bytes (_intro.memory_) making up
the object can be copied into an array of char or unsigned char.36) If
the content of the array of char or unsigned char is copied back into
the object, the object shall subsequently hold its original value.
[Example:
#define N sizeof(T)
char buf[N];
T obj; // obj initialized to its original value
memcpy(buf, obj, N); // between these two calls to memcpy,
// obj might be modified
memcpy(obj, buf, N); // at this point, each subobject of obj of
scalar type
// holds its original value
thought, I view this as completely redundant with the standard and should
just be a note. From the base standard, in C99 is it defined in 7.24.4.2.3
and I suspect remains
Clean up pretty printers [10/n]
This patch consolidates initialization routines of various pretty printers -- at the moment, we have unfortunately very few (basic, C, C++). It adds a default constructor to pretty_printer (which in turn made obvious we needed one for output_buffer), c_pretty_printer, and cxx_pretty_printer. The rest of the patch is just propagating these things by removing direct calls to pp_construct, pp_c_pretty_printer_init, and pp_cxx_pretty_printer_init. Now, we are guaranteed that initialization of any pretty printer derived from pretty_printer will initialize the base subobjects (good!) Our existing XNEW placebo doesn't do what is expected by a C++ programmer, which is to call the constructor to initialize the object. So, I use placement-new at select places to ensure the Right Thing is done. As you know, placement-new doesn't allocate memory; its only effect is to call the right constructor. We need to revisit our design and policy with regard to XNEW and friends. But that is a debate for a separate thread. Finally, we really need to get rid of pretty printers with static storage duration. I removed almost all of them in previous patches except two: one is left in tree-pretty-printer.c , and one in cp/error.c. Tested on an x86_64-suse-linux. Applied to trunk. -- Gaby 2013-08-22 Gabriel Dos Reis g...@integrable-solutions.net * pretty-print.h (output_buffer::output_buffer): Declare. (pretty_printer::pretty_printer): Likewise. (pp_construct): Remove. * pretty-print.c (output_buffer::output_buffer): Define. (pretty_printer::pretty_printer): Rename from pp_construct. Simplify. * gimple-pretty-print.c (print_gimple_stmt): Do not call pp_construct. (print_gimple_expr): Likewise. (print_gimple_seq): Likewise. (gimple_dump_bb): Likewise. * sched-vis.c (dump_value_slim): Likewise. (dump_insn_slim): Likewise. (dump_rtl_slim): Likewise. (str_pattern_slim): Likewise. * tree-mudflap.c (mf_varname_tree): Likewise. * graph.c (print_graph_cfg): Likewise. (start_graph_dump): Likewise. * tree-pretty-print.c (maybe_init_pretty_print): Likewise. Use placement-new. * diagnostic.c (diagnostic_initialize): Simplify early diagnostic pretty printer initialization. * coretypes.h (diagnostic_context): Remove superflous type alias declaration. (pretty_printer): Likewise. Declare directly as a class. (pretty_print_info): Remove declaration as class. * asan.c (asan_emit_stack_protection): Remove call to pp_construct and pp_clear_output_area. (asan_add_global): Likewise. c/ * c-objc-common.c (c_initialize_diagnostics): Simplify C pretty printer initialization. c-family/ * c-pretty-print.h (pp_c_pretty_printer_init): Remove. (c_pretty_printer::c_pretty_printer): Declare. * c-pretty-print.c (pretty_printer::c_pretty_printer): Rename from c_pretty_printer_init. Adjust. (print_c_tree): Do not call c_pretty_printer_init. * c-ada-spec.c (dump_ads): Remove call to pp_construct. cp/ * error.c (init_error): Remove calls to pp_construct and pp_cxx_pretty_printer_init. Initialize cxx_pp with placement-new. * cxx-pretty-print.h (cxx_pretty_printer::cxx_pretty_printer): Declare. (cxx_pretty_printer_init): Remove. * cxx-pretty-print.c (cxx_pretty_printer::cxx_pretty_printer): Rename from cxx_pretty_printer_init. Adjust. * cp-objcp-common.c (cxx_initialize_diagnostics): Simplify initialization of C++ diagnostics pretty printer. Index: asan.c === --- asan.c (revision 201904) +++ asan.c (working copy) @@ -938,9 +938,7 @@ /* First of all, prepare the description string. */ pretty_printer asan_pp; - pp_construct (asan_pp, /* prefix */NULL, /* line-width */0); - - pp_clear_output_area (asan_pp); + if (DECL_NAME (current_function_decl)) pp_tree_identifier (asan_pp, DECL_NAME (current_function_decl)); else @@ -1963,9 +1961,7 @@ vecconstructor_elt, va_gc *vinner = NULL; pretty_printer asan_pp; - pp_construct (asan_pp, /* prefix */NULL, /* line-width */0); - pp_clear_output_area (asan_pp); if (DECL_NAME (decl)) pp_tree_identifier (asan_pp, DECL_NAME (decl)); else Index: c/c-objc-common.c === --- c/c-objc-common.c (revision 201904) +++ c/c-objc-common.c (working copy) @@ -30,6 +30,8 @@ #include langhooks.h #include c-objc-common.h +#include new // For placement new. + static bool c_tree_printer (pretty_printer *, text_info *, const char *, int, bool, bool, bool); @@ -183,16 +185,11 @@ void c_initialize_diagnostics (diagnostic_context *context
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 8:19 AM, Jan Hubicka hubi...@ucw.cz wrote: Hi, this patch started as a work to make cxa_pure_virtual as noreturn. This is good for middle-end to figure out that it should not care about devirtualizing to it and it should devirtualize speculative where there is only one variant. I ended up switching the function building stuff to use ecf_flags that is handy to set other attributes, too. I tried to go across all functions build and assign them correct attributes. I expecitely listed them all in changelog to make it easier to double check: I am not expert on C++ runtime. I have three questions: - I tried to track functions that lead to terminate() and not mark them as ECF_LEAF. This is because user can set handler. If the handler can resonably expect the static vars defined in its unit to be in the final form, we can not consider it ECF_LEAF. Perhaps there are cases where terminate() is called only for programs already after undefined effect? - I would like to recall issue if we can make NEW_EXPR annotated with MALLOC attribute. Without it, it is basically impossible to track any dynamically allocated objects in the middle-end operator new is replaceable by user program. - Is do_end_catch nothrow? It does not seem to be declared so in libsupc++ Bootstrapped/regtested x86_64-linux, OK? * class.c (build_vtbl_initializer): Make __cxa_deleted_virtual ECF_NORETURN * cp-tree.h (build_library_fn_ptr, build_cp_library_fn_ptr, push_library_fn, push_void_library_fn): Update prototype. * decl.c (build_library_fn_1): Remove. (push_cp_library_fn, build_cp_library_fn): Update to take ECF flags. (cxx_init_decl_processing): Update; global_delete_fndecl is ECF_NOTROW; __cxa_pure_virtual is ECF_NORETURN | ECF_NORETURN. (build_library_fn_1): Add ecf_flags argument; rename to ... (build_library_fn): ... this one. (build_cp_library_fn): Take ecf_flags; do not copy NOTHROW flag. (build_library_fn_ptr): Take ecf_flags. (build_cp_library_fn_ptr): Likewise. (push_library_fn): Likewise. (push_cp_library_fn): Likewise. (push_void_library_fn): Likewise. (push_throw_library_fn): All throws are ECF_NORETURN. (__cxa_atexit, __cxa_thread_atexit): Add ECF_LEAF | ECF_NOTHROW attributes. (expand_static_init): __cxa_guard_acquire, __cxa_guard_release, __cxa_guard_abort are ECF_NOTHROW | ECF_LEAF. * except.c (init_exception_processing): terminate is ECF_NOTHROW | ECF_NORETURN. (declare_nothrow_library_fn): Add ecf_flags parameter. (__cxa_get_exception_ptr): Is ECF_NOTHROW | ECF_PURE | ECF_LEAF | ECF_TM_PURE. (do_begin_catch): cxa_begin_catch and _ITM_cxa_begin_catch are ECF_NOTHROW. (do_end_catch): __cxa_end_catch and _ITM_cxa_end_catch is nothing. (do_allocate_exception): _cxa_allocate_exception and _ITM_cxa_allocate_exception are ECF_NOTHROW | ECF_MALLOC (do_free_exception): __cxa_free_exception is ECF_NOTHROW | ECF_LEAF. * rtti.c (build_dynamic_cast_1): __dynamic_cast is ECF_LEAF | ECF_PURE | ECF_NOTHROW. Index: cp/class.c === *** cp/class.c (revision 201910) --- cp/class.c (working copy) *** build_vtbl_initializer (tree binfo, *** 8857,8863 if (!get_global_value_if_present (fn, fn)) fn = push_library_fn (fn, (build_function_type_list (void_type_node, NULL_TREE)), ! NULL_TREE); if (!TARGET_VTABLE_USES_DESCRIPTORS) init = fold_convert (vfunc_ptr_type_node, build_fold_addr_expr (fn)); --- 8857,8863 if (!get_global_value_if_present (fn, fn)) fn = push_library_fn (fn, (build_function_type_list (void_type_node, NULL_TREE)), ! NULL_TREE, ECF_NORETURN); if (!TARGET_VTABLE_USES_DESCRIPTORS) init = fold_convert (vfunc_ptr_type_node, build_fold_addr_expr (fn)); Index: cp/cp-tree.h === *** cp/cp-tree.h(revision 201910) --- cp/cp-tree.h(working copy) *** extern void check_goto (tree); *** 5173,5182 extern bool check_omp_return (void); extern tree make_typename_type(tree, tree, enum tag_types, tsubst_flags_t); extern tree make_unbound_class_template (tree, tree, tree, tsubst_flags_t); ! extern tree
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 9:38 AM, Alexander Monakov amona...@ispras.ru wrote: On Thu, 22 Aug 2013, Gabriel Dos Reis wrote: - I would like to recall issue if we can make NEW_EXPR annotated with MALLOC attribute. Without it, it is basically impossible to track any dynamically allocated objects in the middle-end operator new is replaceable by user program. But so is malloc? no, malloc isn't replaceable. As I understand, the reason why malloc attribute is not applicable to operator new is placement new, which returns aliased memory. whether 'operator new' returns aliases memory depends on the user's implementation, but then user has to be careful so that all other aliasing assumptions still holds. It is not obvious it is *exactly* like malloc, which is why we need to exercise caution. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 9:56 AM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Aug 22, 2013 at 09:53:35AM -0500, Gabriel Dos Reis wrote: On Thu, Aug 22, 2013 at 9:38 AM, Alexander Monakov amona...@ispras.ru wrote: On Thu, 22 Aug 2013, Gabriel Dos Reis wrote: - I would like to recall issue if we can make NEW_EXPR annotated with MALLOC attribute. Without it, it is basically impossible to track any dynamically allocated objects in the middle-end operator new is replaceable by user program. But so is malloc? no, malloc isn't replaceable. Is it such a good idea to replace something gcc during bootstrap can optimize well (malloc/xmalloc etc.) with something that it can't (operator new)? I think we may have terminology problems here. A placement-new, like the one I used in my recent patch, is operationally the identity function (in addition to informing the alias analyzer about the beginning of a new object lifetime). 'operator new' is an allocator function. I didn't use it. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 10:21 AM, Jan Hubicka hubi...@ucw.cz wrote: On Thu, 22 Aug 2013, Gabriel Dos Reis wrote: - I would like to recall issue if we can make NEW_EXPR annotated with MALLOC attribute. Without it, it is basically impossible to track any dynamically allocated objects in the middle-end operator new is replaceable by user program. But so is malloc? As I understand, the reason why malloc attribute is not applicable to operator new is placement new, which returns aliased memory. placement new is optimized to nothing, so it should not affect anything here. Yup. MALLOC attribute makes the assumtion that you can not get to the return value of the function from something that you had previously (i.e. you can not have existing pointer to that block of memory). So you can not really implement malloc in the same compilation unit you are using it. Exactly. The same goes for any user allocators -- many of which tend to be inline, delegating the hard work to an off-line function. So the existing program needs to overwrite libsup++ symbol like we do with malloc? Of course with user defineed malloc function we should not propagate the attribute, but I think we could have it when we end up calling the runtime. I suspect the question is whether our current infrastructure permits to distinguish between declaration of 'operator new' we supply, and 'operator new' defined by user. The way we currently arrange for user-defined 'operator new' to take over is that it is something that is done at linktime, (so is LTO prepared for this?) and ours is searched last. Honza Alexander
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 10:38 AM, Alexander Monakov amona...@ispras.ru wrote:
On Thu, 22 Aug 2013, Jan Hubicka wrote:
On Thu, 22 Aug 2013, Gabriel Dos Reis wrote:
- I would like to recall issue if we can make NEW_EXPR annotated with
MALLOC attribute. Without it, it is basically impossible to track
any dynamically allocated objects in the middle-end
operator new is replaceable by user program.
But so is malloc? As I understand, the reason why malloc attribute is
not
applicable to operator new is placement new, which returns aliased
memory.
placement new is optimized to nothing, so it should not affect anything here.
Which means you cannot annotate NEW_EXPR with attribute malloc when
-fno-inline is in effect, right? Because then placement new is no longer
optimized out. Testcase, compile with -O2 -fno-inline -fno-ipa-sra:
#include new
int foo(void *c)
{
return *(new (c) int);
}
This should be a C++ front-end bug. The specific operator new
from new used in this placement-new should be folded,
even if -fno-inline. Jason, is this something easily fixable?
After overload resolution, we know exactly which operator new
we are picking, and we know it is the one coming from the
implementation with its definition. So we should be good to go.
-- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 10:39 AM, Jan Hubicka hubi...@ucw.cz wrote: So the existing program needs to overwrite libsup++ symbol like we do with malloc? Of course with user defineed malloc function we should not propagate the attribute, but I think we could have it when we end up calling the runtime. I suspect the question is whether our current infrastructure permits to distinguish between declaration of 'operator new' we supply, and 'operator new' defined by user. The way we currently arrange for user-defined 'operator new' to take over is that it is something that is done at linktime, (so is LTO prepared for this?) I think so (I am not expert though :)) The operator new we supply is called _Znwm and I want the particular decl with assembler name _Znwm to have malloc attribute. I get it right, it is the declaration we build by newtype = cp_build_type_attribute_variant (ptr_ftype_sizetype, newattrs); newtype = build_exception_variant (newtype, new_eh_spec); deltype = cp_build_type_attribute_variant (void_ftype_ptr, extvisattr); deltype = build_exception_variant (deltype, empty_except_spec); push_cp_library_fn (NEW_EXPR, newtype); push_cp_library_fn (VEC_NEW_EXPR, newtype); User's new will be different declarations with different assembler name. It is up to user to care about malloc attributes or whatever. For functions that are replaceable, what counts is the provenance of the *definition*, not just of one its declarations. The sequence of code that you show is mandated by C++. Quoting C++11 3.7.4/2 The library provides default definitions for the global allocation and deallocation functions. Some global allocation and deallocation functions are replaceable (18.6.1). A C++ program shall provide at most one definition of a replaceable allocation or deallocation function. Any such function definition replaces the default version provided in the library (17.6.4.6). The following allocation and deallocation functions (18.6) are implicitly declared in global scope in each translation unit of a program. void* operator new(std::size_t); void* operator new[](std::size_t); void operator delete(void*); void operator delete[](void*); These implicit declarations introduce only the function names operator new, operator new[], operator delete, and operator delete[]. So, I think we do need to know the definition site of _Znwm. If it comes from the implementation, then we are good to go. Otherwise, we would need to proceed carefully. Aren't user allocator also allowed to make their own exceptions in addition to throw (std::bad_alloc);? I suspect so, though I don't think the replacement allocation definitions are allowed to throw arbitrary exceptions. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 11:16 AM, Jan Hubicka hubi...@ucw.cz wrote:
On Thu, Aug 22, 2013 at 10:39 AM, Jan Hubicka hubi...@ucw.cz wrote:
So the existing program needs to overwrite libsup++ symbol like we do
with malloc?
Of course with user defineed malloc function we should not propagate
the attribute,
but I think we could have it when we end up calling the runtime.
I suspect the question is whether our current infrastructure permits
to distinguish
between declaration of 'operator new' we supply, and 'operator new'
defined by
user. The way we currently arrange for user-defined 'operator new' to
take
over is that it is something that is done at linktime, (so is LTO
prepared for this?)
I think so (I am not expert though :))
The operator new we supply is called _Znwm and I want the particular decl
with
assembler name _Znwm to have malloc attribute. I get it right, it is the
declaration we build by
newtype = cp_build_type_attribute_variant (ptr_ftype_sizetype,
newattrs);
newtype = build_exception_variant (newtype, new_eh_spec);
deltype = cp_build_type_attribute_variant (void_ftype_ptr, extvisattr);
deltype = build_exception_variant (deltype, empty_except_spec);
push_cp_library_fn (NEW_EXPR, newtype);
push_cp_library_fn (VEC_NEW_EXPR, newtype);
User's new will be different declarations with different assembler name.
It is
up to user to care about malloc attributes or whatever.
For functions that are replaceable, what counts is the provenance of the
*definition*, not just of one its declarations. The sequence of code that
you
show is mandated by C++. Quoting C++11 3.7.4/2
The library provides default definitions for the global allocation and
deallocation functions. Some global allocation and deallocation functions
are replaceable (18.6.1). A C++ program shall provide at most one
definition of a replaceable allocation or deallocation function. Any such
function definition replaces the default version provided in the
library (17.6.4.6).
The following allocation and deallocation functions (18.6) are implicitly
declared in global scope in each translation unit of a program.
This is quite unfortunate. Thre is nothing standard promise about return
value if this default new?
Oh, sure, there is. Again, I am advocating *caution* (in order to support
existing idioms); I am not advocating against. Concretely, that means we
need to refine your original proposal. My previous analysis and suggestion
for LTO to be provenance-aware were made in that sense.
I.e. can I have something like
int a;
test()
{
int *b=new (int);
}
with custom implementation of new that returns a?
If the user-supplied operator new returns a, then it must
also ensure that 'a' is not used anywhere else -- e.g. I you can't
do lvalue-to-value conversion on 'a' to see what is written there.
Because its storage has been reused. That is, aliasing is framed
in terms of object lifetime and uniqueness of ownership.
While compiling given object file we do not know if the user program happens
to
define it somewhere else or not. Even with LTO I do not think we have
mechanism to recognize statically or dynamically linked libsupc++ (we would
have to LTO _Zwnm that will need to stabilize LTO format to start with).
Can we augment _DECLs and LTO to be provenance-aware? For example,
in libstdc++ we already have pragmas saying that we are in a system header
so we don't warn about certain constructs there. I'm sure my fellow libstdc++
maintainers could be convinced to add a pragma system implementation to
implementation files if they are assured that there are tangible
benefits to ripple
from doing that.
There are lot of opportunities for optimizing C++ programs if we can
do that, because
the standard library adds certain restrictions and make certain semantics
guarantees that we can't always expect from arbitrary user program fragments.
We probably can go with -fno-user-overwritten-new or something similar?
-fno-user-supplied-allocation-function is possible but this is such a particular
case of a huge opportunity that it would be pity if we could just go
the extra mile.
-- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 11:08 AM, Jan Hubicka hubi...@ucw.cz wrote: This should be a C++ front-end bug. The specific operator new from new used in this placement-new should be folded, even if -fno-inline. Jason, is this something easily fixable? After overload resolution, we know exactly which operator new we are picking, and we know it is the one coming from the implementation with its definition. So we should be good to go. Great! I think this is quite important starting point to get some heap optimizations done in middle-end :) In the placement-form, no allocation is done, so the fact that we are not emitting identity opcode is a bug :-) The ground for this is (quoting C++11, but this has been true forever) 18.6.1.3/1: These functions are reserved, a C++ program may not define functions that displace the versions in the Standard C++ library (17.6.4). The provisions of (3.7.4) do not apply to these reserved placement forms of operator new and operator delete. -- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 4:14 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 22, 2013, at 9:45 AM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
I.e. can I have something like
int a;
test()
{
int *b=new (int);
}
with custom implementation of new that returns a?
If the user-supplied operator new returns a, then it must
also ensure that 'a' is not used anywhere else -- e.g. I you can't
do lvalue-to-value conversion on 'a' to see what is written there.
This is wrong, in the c++97 standard there is no such limitation or
restriction.
Please, elaborate.
Because its storage has been reused. That is, aliasing is framed
in terms of object lifetime and uniqueness of ownership.
Nope, this is wrong. Example:
int i, j;
main() {
i = 1;
j = i;
i = 2;
char *cpi = (char*)i;
char *cpj = (char*)j;
for (k= 0; k sizeof (int); ++k)
cpj[k] = cpi[k];
}
This is well defined. i and j exist, as do the character objects that are
pointed to by cpi and cpj. One can use them and interleave them, they can
alias, and the character objects are not unique from i and j.
but, this isn't what we are talking about -- that pointers to
character types can
alias pretty much anything is admitted and isn't under debate; GCC already copes
with that.
-- Gaby
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 6:16 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 22, 2013, at 2:28 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
On Thu, Aug 22, 2013 at 4:14 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 22, 2013, at 9:45 AM, Gabriel Dos Reis
g...@integrable-solutions.net wrote:
I.e. can I have something like
int a;
test()
{
int *b=new (int);
}
with custom implementation of new that returns a?
If the user-supplied operator new returns a, then it must
also ensure that 'a' is not used anywhere else -- e.g. I you can't
do lvalue-to-value conversion on 'a' to see what is written there.
This is wrong, in the c++97 standard there is no such limitation or
restriction.
Please, elaborate.
Sure, there is no wording that limits as you describe. There is a limit for
polymorphic classes and classes with virtual bases (See [class.cdtor], but
that is due to the vtable pointer lifetime and vbase pointer (offset)
lifetime. Essentially, you can't use viable pointers or do vbase conversions
before they are set, and they are only set at a particular time. The
standard keeps it simple and expands to non-POD, but I'd argue that from a
QOI we should not make things that can work, not work. See below on QOI
issues.
I think we must distinguish what is wrong according to the standards
we are implementing from what is wrong from a QoI point of view.
My reasoning (for C++98, but the same is true for C++11) is based
on 3.8/1:
[…]
The lifetime of an object of type T ends when:
-- if T is a class type with a non-trivial destructor (12.4),
the destructor call starts, or
-- the storage which the object occupies is reused or released.
Doing a placement-new on the storage occupied by 'a' is reusing
its storage, therefore ending its lifetime.
-- Gaby
I can't quote it, since the limitation doesn't exist. I can quote the
language that allows on an object, that you can then cast this to be of a
different type, and then dereference and use that type, if you want. I can
quote the object lifetime rules, that describe when it comes into existence,
and when it goes away. But, none of these are terribly surprising. If you
want to narrow done what part of the language you're interested in, I can
quote just that part.
int a;
a exists before the program runs, and exists till after the program is
finished running (See [basic.stc.static]). That's the lifetime of it.
During it's lifetime, you can use it in the ways the standard lets you. For
example, ++a;. In the below:
void foo() {
char *cp = a;
[ … ]
}
cp's lifetime is from the declaration of it, til the }. The character object
that cp points to has a lifetime. It's lifetime is from before the program
runs, til after the program finishes. (See [basic.life]) It can be used any
way that a lvalue character can be used. Since you can't use cp before it's
lifetime, to use this character object outside of the lifetime of cp, you'd
need another reference to it beyond just cp. Again, this isn't suppose to be
surprising.
Now, we do have wording like:
15If a program attempts to access the stored value of an object through
an lvalue of other than one of the following types the behavior is
undefined48):
and we do have latitude to do things that uses that as a basis, but, once one
ensures locking, say with atomics or volatile, to ensure the variable hits
memory, I will argue that we can't make as much use of that from a quality of
implementation viewpoint, despite the standard wording. Now, even without
volatile and locking, from a quality of implementation point of view, we
don't actually want to make full use of undefined. QOI still forces us to do
the right thing. undefined means, we get to kill the user. QOI means, even
though we can, we refrain from it, cause if we did, they would not like us.
For the case of a int, and a placement new on that int, of an int. The
behavior is mandated by the standard to work. For a allocation function,
they are free to play games with persistence (or unexec a la emacs), with
allocated objects and this too has to work. This means they can alias, and
the standard says they can write this code and the standard mandates that it
works.
Now, the user can use:
@item malloc
@cindex @code{malloc} attribute
The @code{malloc} attribute is used to tell the compiler that a function
may be treated as if any non-@code{NULL} pointer it returns cannot
alias any other pointer valid when the function returns and that the memory
has undefined content.
This often improves optimization.
Standard functions with this property include @code{malloc} and
@code{calloc}. @code{realloc}-like functions do not have this
property as the memory pointed to does not have undefined content.
on their allocation functions, if they want. And if they do, than what it
says
Re: [C++ patch] Set attributes for C++ runtime library calls
On Thu, Aug 22, 2013 at 8:51 PM, Mike Stump mikest...@comcast.net wrote:
On Aug 22, 2013, at 6:10 PM, Gabriel Dos Reis g...@integrable-solutions.net
wrote:
I think we must distinguish what is wrong according to the standards
we are implementing from what is wrong from a QoI point of view.
Not if they match, we don't.
In abstract possibly; but your previous assertion needs refinement to
be as categorical as you implied.
My reasoning (for C++98, but the same is true for C++11) is based
on 3.8/1:
[…]
The lifetime of an object of type T ends when:
-- if T is a class type with a non-trivial destructor (12.4),
the destructor call starts, or
-- the storage which the object occupies is reused or released.
Doing a placement-new on the storage occupied by 'a' is reusing
its storage, therefore ending its lifetime.
The problem is that reused is not well defined,
I am not quite so sure. But even so, in light of this, I don't think
you original assertion is definitive.
so, we are into the weeds right there.
int i, j;
int *ip = i;
i = 1;
j = 2;
*ip = j;
++i;
++j;
here, we sees the storage being reused in the *ip = j;
Really?
statement, or, is it merely changing the value?
That is an assignment to an existing int storage.
And what if we do a memcpy (ip, j, sizeof (int));
Well, the case of 'memcpy' isn't defined by the standard and is
precisely the subject of a issue identified by the C++ SG12
that will be discussed at the Chicago meeting.
Is that reused, or merely changing the value.
The current proposal is that it constructs an int value, therefore
is moral equivalent of copy-constructor call.
I think the most logical line of reasoning is that when the requirements of
[basic.lval] are met, the, this is a change of value of an object, not a
modification to it's lifetime.
Why?
So, in the case quoted, since the type of the accesses are both int, we
don't reuse the storage, since the requirements of [basic.lval] are met.
Consider:
struct S {
S(int);
~S();
// …
};
int main() {
S s(8);
new (s) S(9); // #1
}
Is #1 a reuse of storage to create a new object or assignment?
Indeed, the programmer expects that they can access i after *ip = j; and
that the _value_ that object, while changed from the original 1, will be 2
just after the *ip = j; statement.
Since we know that i must be 3 at the end, we then know what the wording,
reused, must mean, cause other meanings that could possibly make it work for
you in the case you are considering, would destroy this property of pointers,
and everyone knows the semantics of pointers, they are undisputed. Or put
another way, you cannot misread reused in this way.
And why do you assert that I misread 'reused' in this way?
-- Gaby
Clean up pretty printers [9/n]
This patch formalizes the observation that certain pretty printing
functions return formatted texts that have lifetime logically outlasting
the local instances of pretty printers. This has not been a problem so
far because we were using static storage, but once we switch local
pretty printers, it becomes a problem.
Tested on an x86_64-suse-linux. Applied to mainline.
-- Gaby
2013-08-20 Gabriel Dos Reis g...@integrable-solutions.net
* error.c (pp_ggc_formatted_text): New.
(type_as_string): Use it in lieu of pp_formatted_text.
(type_as_string_translate): Likewise.
(expr_as_string): Likewise.
(decl_as_string): Likewise.
(decl_as_string_translate): Likewise.
(lang_decl_name): Likewise.
(decl_to_string): Likewise.
(expr_to_string): Likewise.
(fndecl_to_string): Likewise.
(parm_to_string): Likewise.
(type_to_string): Likewise.
(args_to_string): Likewise.
(subst_to_string): Likewise.
Index: cp/error.c
===
--- cp/error.c (revision 201871)
+++ cp/error.c (working copy)
@@ -2632,7 +2632,15 @@
cxx_pp-enclosing_scope = current_function_decl;
}
+/* Same as pp_formatted_text, except the return string is a separate
+ copy and has a GGC storage duration, e.g. an indefinite lifetime. */
+inline const char *
+pp_ggc_formatted_text (pretty_printer *pp)
+{
+ return ggc_strdup (pp_formatted_text (pp));
+}
+
/* Exported interface to stringifying types, exprs and decls under TFF_*
control. */
@@ -2642,7 +2650,7 @@
reinit_cxx_pp ();
pp_translate_identifiers (cxx_pp) = false;
dump_type (cxx_pp, typ, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
const char *
@@ -2650,7 +2658,7 @@
{
reinit_cxx_pp ();
dump_type (cxx_pp, typ, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
const char *
@@ -2659,7 +2667,7 @@
reinit_cxx_pp ();
pp_translate_identifiers (cxx_pp) = false;
dump_expr (cxx_pp, decl, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
/* Wrap decl_as_string with options appropriate for dwarf. */
@@ -2683,7 +2691,7 @@
reinit_cxx_pp ();
pp_translate_identifiers (cxx_pp) = false;
dump_decl (cxx_pp, decl, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
const char *
@@ -2691,7 +2699,7 @@
{
reinit_cxx_pp ();
dump_decl (cxx_pp, decl, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
/* Wrap lang_decl_name with options appropriate for dwarf. */
@@ -2738,7 +2746,7 @@
else
dump_decl (cxx_pp, DECL_NAME (decl), TFF_PLAIN_IDENTIFIER);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
/* Return the location of a tree passed to %+ formats. */
@@ -2782,7 +2790,7 @@
reinit_cxx_pp ();
dump_decl (cxx_pp, decl, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
static const char *
@@ -2790,7 +2798,7 @@
{
reinit_cxx_pp ();
dump_expr (cxx_pp, decl, 0);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
static const char *
@@ -2804,7 +2812,7 @@
flags |= TFF_FUNCTION_DEFAULT_ARGUMENTS;
reinit_cxx_pp ();
dump_decl (cxx_pp, fndecl, flags);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
@@ -2844,7 +2852,7 @@
pp_string (cxx_pp, 'this');
else
pp_decimal_int (cxx_pp, p + 1);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
static const char *
@@ -2887,7 +2895,7 @@
if (memcmp (p, p+aka_start, len) == 0)
p[len] = '\0';
}
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
static const char *
@@ -2920,7 +2928,7 @@
if (TREE_CHAIN (p))
pp_separate_with_comma (cxx_pp);
}
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
/* Pretty-print a deduction substitution (from deduction_tsubst_fntype). P
@@ -2947,7 +2955,7 @@
pp_cxx_whitespace (cxx_pp);
dump_template_bindings (cxx_pp, tparms, targs, NULL);
pp_cxx_right_bracket (cxx_pp);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
static const char *
@@ -2956,7 +2964,7 @@
reinit_cxx_pp ();
cxx_pp-padding = v ? pp_before : pp_none;
pp_cxx_cv_qualifier_seq (cxx_pp, p);
- return pp_formatted_text (cxx_pp);
+ return pp_ggc_formatted_text (cxx_pp);
}
/* Langhook for print_error_function. */
Re: [PATCH v3 06/18] convert the C++ front end to automatic dependencies
On Tue, Aug 20, 2013 at 8:59 AM, Tom Tromey tro...@redhat.com wrote: This converts the C++ front end. This renames g++spec.o to cp/g++spec.o for uniformity. This lets us remove an explicit rule. This patch does not remove various *_H macros from cp/Make-lang.in. These are still needed by ObjC++. They're removed by a later patch. * Make-lang.in (g++spec.o): Remove. (CFLAGS-cp/g++spec.o): New variable. (GXX_OBJS): Reference cp/g++spec.o. (cc1plus-checksum.o, cp/lex.o, cp/cp-array-notation.o) (cp/cp-lang.o, cp/decl.o, cp/decl2.o, cp/cp-objcp-common.o) (cp/typeck2.o, cp/typeck.o, cp/class.o, cp/call.o) (cp/friend.o, cp/init.o, cp/method.o, cp/cvt.o, cp/search.o) (cp/tree.o, cp/ptree.o, cp/rtti.o, cp/except.o, cp/expr.o) (cp/pt.o, cp/error.o, cp/repo.o, cp/semantics.o, cp/dump.o) (cp/optimize.o, cp/mangle.o, cp/parser.o, cp/cp-gimplify.o) (cp/name-lookup.o, cp/cxx-pretty-print.o): Remove. OK. -- Gaby
Re: [C++ Patch, obvious] Use cp_parser_lookup_name_simple more
On Mon, Aug 19, 2013 at 6:04 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi, was having a look to c++/58187 (looks like we are missing a lookup) and noticed this. It seems obvious to me, I'll commit it later today barring objections. OK. Tested x86_64-linux. Thanks, Paolo.
[c++-concepts] Merge from trunk
The c++-concepts branch was synchronized with trunk at revision 201834. -- Gaby
Re: [RFC] Issues with intraprocedural devirtualization
On Sat, Aug 17, 2013 at 7:56 PM, Jason Merrill ja...@redhat.com wrote: I do not know if one can do something like having automatic variable of class A and use placement new to change it to class B. This is something of a grey area in the standard, with a few defect reports yet to be resolved. I think it should be undefined behavior. Jason, could you remind me of the CWG issue numbers for these? It is also of interest for SG12, even if it isn't going to come withe a different answer. -- Gaby
Clean up pretty printers [7/n]
Same topic as previous patch in this series.
-- Gaby
2013-08-16 Gabriel Dos Reis g...@integrable-solutions.net
* sched-vis.c (rtl_slim_pp_initialized): Remove.
(rtl_slim_pp): Likewise.
(init_rtl_slim_pretty_print): Likewise.
(dump_value_slim): Don't call it. Use local pretty printer.
(dump_insn_slim): Likewise.
(dump_rtl_slim): Likewise.
(str_pattern_slim): Likewise.
* tree-mudflap.c (mf_varname_tree): Use local pretty printer.
Simplify.
Index: sched-vis.c
===
--- sched-vis.c (revision 201784)
+++ sched-vis.c (working copy)
@@ -47,10 +47,6 @@
It is also possible to obtain a string for a single pattern as a string
pointer, via str_pattern_slim, but this usage is discouraged. */
-/* A pretty-printer for slim rtl printing. */
-static bool rtl_slim_pp_initialized = false;
-static pretty_printer rtl_slim_pp;
-
/* For insns we print patterns, and for some patterns we print insns... */
static void print_insn_with_notes (pretty_printer *, const_rtx);
@@ -760,24 +756,6 @@
}
}
-/* Return a pretty-print buffer set up to print to file F. */
-
-static pretty_printer *
-init_rtl_slim_pretty_print (FILE *f)
-{
- if (! rtl_slim_pp_initialized)
-{
- pp_construct (rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
- rtl_slim_pp_initialized = true;
-}
- else
-/* Clean out any data that str_insn_slim may have left here. */
-pp_clear_output_area (rtl_slim_pp);
-
- rtl_slim_pp.buffer-stream = f;
- return rtl_slim_pp;
-}
-
/* Print X, an RTL value node, to file F in slim format. Include
additional information if VERBOSE is nonzero.
@@ -787,9 +765,11 @@
void
dump_value_slim (FILE *f, const_rtx x, int verbose)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
- print_value (pp, x, verbose);
- pp_flush (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer-stream = f;
+ print_value (rtl_slim_pp, x, verbose);
+ pp_flush (rtl_slim_pp);
}
/* Emit a slim dump of X (an insn) to the file F, including any register
@@ -797,9 +777,11 @@
void
dump_insn_slim (FILE *f, const_rtx x)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
- print_insn_with_notes (pp, x);
- pp_flush (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer-stream = f;
+ print_insn_with_notes (rtl_slim_pp, x);
+ pp_flush (rtl_slim_pp);
}
/* Same as above, but stop at LAST or when COUNT == 0.
@@ -810,19 +792,21 @@
int count, int flags ATTRIBUTE_UNUSED)
{
const_rtx insn, tail;
- pretty_printer *pp = init_rtl_slim_pretty_print (f);
+ pretty_printer rtl_slim_pp;
+ pp_construct (rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ rtl_slim_pp.buffer-stream = f;
tail = last ? NEXT_INSN (last) : NULL_RTX;
for (insn = first;
(insn != NULL) (insn != tail) (count != 0);
insn = NEXT_INSN (insn))
{
- print_insn_with_notes (pp, insn);
+ print_insn_with_notes (rtl_slim_pp, insn);
if (count 0)
count--;
}
- pp_flush (pp);
+ pp_flush (rtl_slim_pp);
}
/* Dumps basic block BB to pretty-printer PP in slim form and without and
@@ -857,9 +841,10 @@
const char *
str_pattern_slim (const_rtx x)
{
- pretty_printer *pp = init_rtl_slim_pretty_print (NULL);
- print_pattern (pp, x, 0);
- return pp_formatted_text (pp);
+ pretty_printer rtl_slim_pp;
+ pp_construct (rtl_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ print_pattern (rtl_slim_pp, x, 0);
+ return ggc_strdup (pp_formatted_text (rtl_slim_pp));
}
/* Emit a slim dump of X (an insn) to stderr. */
Index: tree-mudflap.c
===
--- tree-mudflap.c (revision 201784)
+++ tree-mudflap.c (working copy)
@@ -106,20 +106,14 @@
static tree
mf_varname_tree (tree decl)
{
- static pretty_printer buf_rec;
- static int initialized = 0;
- pretty_printer *buf = buf_rec;
const char *buf_contents;
tree result;
gcc_assert (decl);
- if (!initialized)
-{
- pp_construct (buf, /* prefix */ NULL, /* line-width */ 0);
- initialized = 1;
-}
- pp_clear_output_area (buf);
+ pretty_printer buf;
+ pp_construct (buf, /* prefix */ NULL, /* line-width */ 0);
+ pp_clear_output_area (buf);
/* Add FILENAME[:LINENUMBER[:COLUMNNUMBER]]. */
{
@@ -134,17 +128,17 @@
if (sourcefile == NULL)
sourcefile = unknown file;
-pp_string (buf, sourcefile);
+pp_string (buf, sourcefile);
if (sourceline != 0)
{
-pp_colon (buf);
-pp_decimal_int (buf, sourceline);
+pp_colon (buf);
+pp_decimal_int (buf, sourceline);
if (sourcecolumn != 0)
{
-pp_colon (buf);
-pp_decimal_int (buf, sourcecolumn
Re: [PATCH 2/3] Support using 'auto' in a function parameter list to introduce an implicit template parameter.
On Wed, Aug 14, 2013 at 9:07 AM, Jason Merrill ja...@redhat.com wrote: On 08/12/2013 08:34 PM, Adam Butcher wrote: Currently it is the implicit function template code in pt.c that updates this; specifically add_implicit_template_parms and finish_fully_implicit_template. It is queried by the parser (both in parser.c and lambda.c) to decide whether a new [implicit] template parameter list as been started and to decide how to finish up. I had a look into this; it should be possible to move these out of pt.c and into parser.c (or some new file generic-parms.c, implicit-pt.c or some such) and possibly add a plain global counter for this state, rather than attribute it to each scope. Right, I was thinking it would make sense as a field of cp_parser, since it doesn't affect template instantiation context. Agreed. I think we should reduce the number of such object macro states. -- Gaby
Re: [PATCH, libstdc++]: Avoid -Wcast-qual warnings in src/c++98/compatibility.cc
On Wed, Aug 14, 2013 at 9:22 AM, Uros Bizjak ubiz...@gmail.com wrote: On Wed, Aug 14, 2013 at 4:05 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: I agree 'const void*' is preferable, and all the places you mention should be covered too. I already installed my original patch, so attached incremental patch implements preferred solution. 2013-08-14 Uros Bizjak ubiz...@gmail.com * src/c++98/compatibility.cc (_ZTIe): Use (const cast *) to avoid -Wcast-qual warnings. (_ZTIPe): Ditto. (ZTIPKe): Ditto. Tested on alphaev68-pc-linux-gnu and installed on mainline SVN. Is this solution also OK for other release branches? Uros. Sorry, when I said 'const void*' was OK, I implicitly assumed that you would use C++-style cast reinterpret_castconst void*(_Z….). Same for reinterpret_castconst void*(1); -- Gaby
Re: [PATCH, libstdc++]: Avoid -Wcast-qual warnings in src/c++98/compatibility.cc
On Wed, Aug 14, 2013 at 9:49 AM, Uros Bizjak ubiz...@gmail.com wrote: On Wed, Aug 14, 2013 at 4:36 PM, Uros Bizjak ubiz...@gmail.com wrote: On Wed, Aug 14, 2013 at 4:34 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Wed, Aug 14, 2013 at 9:22 AM, Uros Bizjak ubiz...@gmail.com wrote: On Wed, Aug 14, 2013 at 4:05 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: I agree 'const void*' is preferable, and all the places you mention should be covered too. I already installed my original patch, so attached incremental patch implements preferred solution. 2013-08-14 Uros Bizjak ubiz...@gmail.com * src/c++98/compatibility.cc (_ZTIe): Use (const cast *) to avoid -Wcast-qual warnings. (_ZTIPe): Ditto. (ZTIPKe): Ditto. Tested on alphaev68-pc-linux-gnu and installed on mainline SVN. Is this solution also OK for other release branches? Uros. Sorry, when I said 'const void*' was OK, I implicitly assumed that you would use C++-style cast reinterpret_castconst void*(_Z….). Same for reinterpret_castconst void*(1); Ah, OK. I'll add this in a moment. Like the attached patch? Again, build tested on alphaev68-pc-linux-gnu. Uros. Yes, OK if it passes tests. Thanks! -- Gaby
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On Thu, Aug 8, 2013 at 6:40 PM, Jason Merrill ja...@redhat.com wrote: On 08/08/2013 03:54 PM, Paolo Carlini wrote: the really interesting one is decltype28.C, which we don't reject anymore, we simply accept it. What is happening is that the overload which leads to excessive template instantiation depth is SFINAE-ed away and the other one wins! Thus, this is the core of my message: it seems that we behave wrt this issue - SFINAE vs template instantiation depth - in a different way vs current clang++ and icc: we produce hard error messages in SFINAE contexts. Is that intended? Yes, that is intended. Changing that could mean that the meaning of code depends on what max depth the user selected. that would be disturbing… -- gaby
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On Fri, Aug 9, 2013 at 2:13 AM, Florian Weimer fwei...@redhat.com wrote: On 08/09/2013 08:43 AM, Paolo Carlini wrote: Yes, that is intended. Changing that could mean that the meaning of code depends on what max depth the user selected. Indeed. Yesterday I wondered what would happen if the front-end had a way to detect, in some very specific and special cases only of course, really infinite recursions, in the sense that no increase in the depth would cure them. Would it be ok in that case to sfinae away? Eh, hopefully not. Otherwise, program behavior would depend on how well the compiler solves the halting problem. It's interesting question, but hopefully we can make all errors due to exceeded implementation limits hard errors, not subject to SFINAE. agreed. -- Gaby
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On Fri, Aug 9, 2013 at 2:33 AM, Florian Weimer fwei...@redhat.com wrote: On 08/09/2013 09:28 AM, Paolo Carlini wrote: I see. You know, I was trying to figure out the logic other compilers - two of them, actually - are following, because the really appear to sfinae away infinite recursions. Was trying to imagine cases in which it would be safe. Could their behavior just be bugs? Depending on their error recovery implementation, not flagging infinite recursion as a hard error in SFINAE context could be an easy mistake to make. Indeed. The fact we try to recreate template instantiations, as opposed to using what is already known at the point where we get the diagnostics is a worrisome aspect of our current infrastructure. That obviously manifests itself with the error re-entered… stuff. -- Gaby
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On Fri, Aug 9, 2013 at 2:13 AM, Paolo Carlini paolo.carl...@oracle.com wrote: .. another thought I had, less esoteric ;) is the following: we use tf_none for two rather different reasons: for SFINAE and to avoid recursive Error routines calls, when we call tsubst (... tf_none, ...) from dump_template_bindings. I understand, given your reply, that in general in the first case, thus SFINAE, we should avoid all hard errors *but* the one about too deep recursions (barring some sort of powerful infinite recursion detector). What about the second case, however? Should it be different? An error message is being produced in any case, for a reason or another, it shouldn't be prevented or made more difficult only because there is deep recursion somewhere. I think that in that second case we should suppress the error message about too deep recursion too. But how to tell it apart? Looks like we want some sort of separate tf_*, a tf_in_diagnostic, or something, very similar to tf_none, but truly with no exceptions. Actually this vague idea occured to me a number of times, I think having that would help in a number of situations. What do you think? Thanks, Paolo. (*) Maybe there is third one, like in some recent tweaks Jakub did, but let's leave it alone here. I think we should find ways to have the pretty printer in the diagnostic framework stop trying to redo most of the work done by the type checker. In its current form, that is fragile. -- Gaby
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On Fri, Aug 9, 2013 at 4:28 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi, On 08/09/2013 10:46 AM, Gabriel Dos Reis wrote: I think we should find ways to have the pretty printer in the diagnostic framework stop trying to redo most of the work done by the type checker. In its current form, that is fragile. -- Gaby Yeah. That tsubst (..., tf_none, ...) from dump_template_bindings, always seemed a little weird to me. Fact is, we have got quite a few serious diagnostic bugs where we print *very little* sensible before the Error reporting routines re-entered message. Like 54080 and 52875, but I'm sure there are more. Do you have any tips about a reasonable way to handle the latter in the short term? I arrived on the west coast with little sleep, and I can't find sleep just right now (and you may argue reading gcc list isn't a good way to find sleep either); let me think about it more. -- Gaby
[c++-concepts] Merge from trunk
trunk was merged into c++-concepts branch at revision 201560. I resolved some conflicts in cp caused by recent merges and ongoing work on trunk. -- Gaby
Clean up pretty printers [4/n]
This patchlet has print_c_tree use non-static local variable for its
pretty-printer object. The code is much simpler that way.
(A follow up will add destructor so we stop leaking storage.)
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-08-05 Gabriel Dos Reis g...@integrable-solutions.net
* c-pretty-print.c (print_c_tree): Simplify. Use non-static local
c_pretty_printer variable.
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201479)
+++ c-family/c-pretty-print.c (working copy)
@@ -2359,22 +2359,13 @@
void
print_c_tree (FILE *file, tree t)
{
- static c_pretty_printer pp_rec;
- static bool initialized = 0;
- c_pretty_printer *pp = pp_rec;
-
- if (!initialized)
-{
- initialized = 1;
- pp_construct (pp, NULL, 0);
- pp_c_pretty_printer_init (pp);
- pp_needs_newline (pp) = true;
-}
- pp-buffer-stream = file;
-
- pp_statement (pp, t);
-
- pp_newline_and_flush (pp);
+ c_pretty_printer pp;
+ pp_construct (pp, NULL, 0);
+ pp_c_pretty_printer_init (pp);
+ pp_needs_newline (pp) = true;
+ pp.buffer-stream = file;
+ pp_statement (pp, t);
+ pp_newline_and_flush (pp);
}
/* Print the tree T in full, on stderr. */
Clean up printer printers [5/n]
This patch stops the gimple printer from using global pretty printers.
Applied to trunk.
-- Gaby
2013-08-05 Gabriel Dos Reis g...@integrable-solutions.net
* gimple-pretty-print.c (buffer): Remove.
(initialized): Likewise.
(maybe_init_pretty_print): Likewise.
(print_gimple_stmt): Do not call it. Use non-static local
pretty_printer variable.
(print_gimple_expr): Likewise.
(print_gimple_seq): Likewise.
(gimple_dump_bb): Likewise.
Index: gimple-pretty-print.c
===
--- gimple-pretty-print.c (revision 201481)
+++ gimple-pretty-print.c (working copy)
@@ -36,9 +36,6 @@
#define INDENT(SPACE) \
do { int i; for (i = 0; i SPACE; i++) pp_space (buffer); } while (0)
-static pretty_printer buffer;
-static bool initialized = false;
-
#define GIMPLE_NIY do_niy (buffer,gs)
/* Try to print on BUFFER a default message for the unrecognized
@@ -52,22 +49,6 @@
}
-/* Initialize the pretty printer on FILE if needed. */
-
-static void
-maybe_init_pretty_print (FILE *file)
-{
- if (!initialized)
-{
- pp_construct (buffer, NULL, 0);
- pp_needs_newline (buffer) = true;
- initialized = true;
-}
-
- buffer.buffer-stream = file;
-}
-
-
/* Emit a newline and SPC indentation spaces to BUFFER. */
static void
@@ -93,7 +74,10 @@
void
print_gimple_stmt (FILE *file, gimple g, int spc, int flags)
{
- maybe_init_pretty_print (file);
+ pretty_printer buffer;
+ pp_construct (buffer, NULL, 0);
+ pp_needs_newline (buffer) = true;
+ buffer.buffer-stream = file;
pp_gimple_stmt_1 (buffer, g, spc, flags);
pp_newline_and_flush (buffer);
}
@@ -122,7 +106,10 @@
print_gimple_expr (FILE *file, gimple g, int spc, int flags)
{
flags |= TDF_RHS_ONLY;
- maybe_init_pretty_print (file);
+ pretty_printer buffer;
+ pp_construct (buffer, NULL, 0);
+ pp_needs_newline (buffer) = true;
+ buffer.buffer-stream = file;
pp_gimple_stmt_1 (buffer, g, spc, flags);
pp_flush (buffer);
}
@@ -155,7 +142,10 @@
void
print_gimple_seq (FILE *file, gimple_seq seq, int spc, int flags)
{
- maybe_init_pretty_print (file);
+ pretty_printer buffer;
+ pp_construct (buffer, NULL, 0);
+ pp_needs_newline (buffer) = true;
+ buffer.buffer-stream = file;
dump_gimple_seq (buffer, seq, spc, flags);
pp_newline_and_flush (buffer);
}
@@ -2279,7 +2269,10 @@
dump_gimple_bb_header (file, bb, indent, flags);
if (bb-index = NUM_FIXED_BLOCKS)
{
- maybe_init_pretty_print (file);
+ pretty_printer buffer;
+ pp_construct (buffer, NULL, 0);
+ pp_needs_newline (buffer) = true;
+ buffer.buffer-stream = file;
gimple_dump_bb_buff (buffer, bb, indent, flags);
}
dump_gimple_bb_footer (file, bb, indent, flags);
Clean up pretty printers [6/n]
Same topics as from previous patch; this time for the graphiz outputter.
-- Gaby
2013-08-05 Gabriel Dos Reis g...@integrable-solutions.net
* graph.c (init_graph_slim_pretty_print): Remove.
(print_graph_cfg): Do not call it. Use local pretty printer.
(start_graph_dump): Likewise.
Index: gcc/graph.c
===
--- gcc/graph.c (revision 201481)
+++ gcc/graph.c (working copy)
@@ -56,26 +56,6 @@
return fp;
}
-/* Return a pretty-print buffer for output to file FP. */
-
-static pretty_printer *
-init_graph_slim_pretty_print (FILE *fp)
-{
- static bool initialized = false;
- static pretty_printer graph_slim_pp;
-
- if (! initialized)
-{
- pp_construct (graph_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
- initialized = true;
-}
- else
-gcc_assert (! pp_last_position_in_text (graph_slim_pp));
-
- graph_slim_pp.buffer-stream = fp;
- return graph_slim_pp;
-}
-
/* Draw a basic block BB belonging to the function with FUNCDEF_NO
as its unique number. */
static void
@@ -297,7 +277,10 @@
{
const char *funcname = function_name (fun);
FILE *fp = open_graph_file (base, a);
- pretty_printer *pp = init_graph_slim_pretty_print (fp);
+ pretty_printer graph_slim_pp;
+ pp_construct (graph_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ graph_slim_pp.buffer-stream = fp;
+ pretty_printer *const pp = graph_slim_pp;
pp_printf (pp, subgraph \%s\ {\n
\tcolor=\black\;\n
\tlabel=\%s\;\n,
@@ -313,7 +296,10 @@
static void
start_graph_dump (FILE *fp, const char *base)
{
- pretty_printer *pp = init_graph_slim_pretty_print (fp);
+ pretty_printer graph_slim_pp;
+ pp_construct (graph_slim_pp, /*prefix=*/NULL, /*linewidth=*/0);
+ graph_slim_pp.buffer-stream = fp;
+ pretty_printer *const pp = graph_slim_pp;
pp_string (pp, digraph \);
pp_write_text_to_stream (pp);
pp_string (pp, base);
Re: C++ coding conventions: namespaces, references and getters (was Re: [PATCH 2/2] Introduce beginnings of a pipeline class.)
On Mon, Aug 5, 2013 at 6:24 AM, Martin Jambor mjam...@suse.cz wrote:
[…]
Note that as per
http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
we'll use pass_manager rather than pipeline, so this would look
like:
pass_manager get_passes () { gcc_assert (passes_); return *passes_; }
We were chatting about C++ references on IRC on Friday, and IIRC there
was a strong objection to passing *arguments* that are non-const
references,
I hope nobody is objecting to std::swap for example.
I'm not sure how I could object to std::swap or what good would that
be. Apart from being the standard, it is also rather exceptional due
to its nature and its name.
I suspect it is a mistake to think this situation is unique. I also think
that we can keep going on and a long list of examples, and that might
still be labelled special case. Reading into a variable is just another
example.
Despite this special example, I think the
rule of not passing arguments as non-const references is a good one,
and it seems I am not alone. Where I may differ from others is that I
also think that getter functions should (generally) return pointers or
const references too, and for similar reasons - the caller may
unintentionally modify data that the programmers thought were on the
stack but were in fact elsewhere else and belonged to someplace
else.
But the rule does not solve that problem -- 'const' does not mean this is
on stack, and absence of const does not mean this is on free store.
Being on stack does no necessarily mean don't change, and being on
free store does not mean you can freely change.
As a matter of fact, in the existing code base, we have many objects
on the stack that are passed around by non-const pointer. That is just
fine for the most part. The notion of const is largely orthogonal to the
notion lifetime (stack of free store.)
These are problems that depend on the specific algorithms being
implemented. Legislating on const reference or not won't solve
any problem there.
-- Gaby
Cleanup pretty printers [1/n]
This patch replaces uses of pp_string on operators and punctuators with
specialized pretty printing functions.
Tested on an x86_64-suse-linux. Applied to trunk.
-- Gaby
2013-08-03 Gabriel Dos Reis g...@integrable-solutions.net
* pretty-print.h (pp_bar_bar): New.
(pp_ampersand_ampersand): Likewise.
(pp_less_equal): Likewise.
(pp_greater_equal): Likewise.
* gimple-pretty-print.c (dump_ternary_rhs): Use specialized pretty
printer functions instead of pp_string or operators and punctuators.
(dump_gimple_call): Likewise.
(dump_gimple_omp_for): Likewise.
(dump_gimple_transaction): Likewise.
(dump_gimple_phi): Likewise.
(pp_gimple_stmt_1): Likewise.
* sched-vis.c (print_insn): Likewise.
* tree-mudflap.c (mf_varname_tree): Likewise.
* tree-pretty-print.c (dump_block_node): Likewise.
(dump_generic_node): Likewise.
c-family/
* c-ada-spec.c (pp_ada_tree_identifier): Use specialized pretty
printer functions instead of pp_string or operators and punctuators.
(dump_generic_ada_node): Likewise.
* c-pretty-print.c (pp_c_type_specifier): Likewise.
(pp_c_relational_expression): Likewise.
(pp_c_logical_or_expression): Likewise.
cp/
* error.c (dump_type_prefix): Use specialized pretty printer
functions instead of pp_string or operators and punctuators.
(dump_decl): Likewise.
(dump_expr): Likewise.
Index: c-family/c-ada-spec.c
===
--- c-family/c-ada-spec.c (revision 201473)
+++ c-family/c-ada-spec.c (working copy)
@@ -1266,7 +1266,7 @@
{
pp_string (buffer, Class_);
pp_string (buffer, s);
- pp_string (buffer, .);
+ pp_dot (buffer);
}
}
@@ -1626,7 +1626,7 @@
if (xloc.file)
{
pp_string (buffer, xloc.file);
- pp_string (buffer, :);
+ pp_colon (buffer);
pp_decimal_int (buffer, xloc.line);
}
}
@@ -1886,14 +1886,14 @@
bool first = true;
spc += INDENT_INCR;
newline_and_indent (buffer, spc - 1);
- pp_string (buffer, ();
+ pp_left_paren (buffer);
for (; value; value = TREE_CHAIN (value))
{
if (first)
first = false;
else
{
- pp_string (buffer, ,);
+ pp_comma (buffer);
newline_and_indent (buffer, spc);
}
@@ -1907,7 +1907,7 @@
dump_generic_ada_node
(buffer, DECL_NAME (type) ? type : TYPE_NAME (node), type,
cpp_check, spc, 0, true);
- pp_string (buffer, ));
+ pp_right_paren (buffer);
}
else
{
@@ -2032,7 +2032,7 @@
pp_string (buffer, pragma Convention (C, );
dump_generic_ada_node
(buffer, type, 0, cpp_check, spc, false, true);
- pp_string (buffer, ));
+ pp_right_paren (buffer);
}
}
else
Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c (revision 201473)
+++ c-family/c-pretty-print.c (working copy)
@@ -370,7 +370,7 @@
pp_c_type_specifier (pp, t);
if (TYPE_PRECISION (t) != prec)
{
- pp_string (pp, :);
+ pp_colon (pp);
pp_decimal_int (pp, prec);
}
}
@@ -393,7 +393,7 @@
gcc_unreachable ();
}
pp_decimal_int (pp, prec);
- pp_string (pp, );
+ pp_greater (pp);
}
}
break;
@@ -1920,9 +1920,9 @@
else if (code == GT_EXPR)
pp_greater (pp);
else if (code == LE_EXPR)
- pp_string (pp, =);
+ pp_less_equal (pp);
else if (code == GE_EXPR)
- pp_string (pp, =);
+ pp_greater_equal (pp);
pp_c_whitespace (pp);
pp_c_shift_expression (pp, TREE_OPERAND (e, 1));
break;
@@ -2032,7 +2032,7 @@
{
pp_c_logical_and_expression (pp, TREE_OPERAND (e, 0));
pp_c_whitespace (pp);
- pp_string (pp, );
+ pp_ampersand_ampersand (pp);
pp_c_whitespace (pp);
pp_c_inclusive_or_expression (pp, TREE_OPERAND (e, 1));
}
@@ -2052,7 +2052,7 @@
{
pp_c_logical_or_expression (pp, TREE_OPERAND (e, 0));
pp_c_whitespace (pp);
- pp_string (pp, ||);
+ pp_bar_bar (pp);
pp_c_whitespace (pp);
pp_c_logical_and_expression (pp, TREE_OPERAND (e, 1));
}
Index: cp/error.c
===
--- cp
Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Sun, Aug 4, 2013 at 8:37 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Wed, Jul 31, 2013 at 10:02:29PM -0500, Gabriel Dos Reis wrote: * declare the pointer to function fields as virtual functions -- that is what I meant with the (necessarily poor) emulation through the casts. I guess you'll work on this later in the patch series you're sending, but its worth noting making pretty_print_info::format_decoder a virtual function is non-trivial, it turns out to be important that some consumers can leave it null instead of making it what is currently default_tree_printer. This is because gcov and maybe other things link against diagnostic.c and pretty-print.c but not all the tree stuff that would be required for default_tree_printer. Trev -- Thanks for the piece of info. Writing OO programming in C is not fun at all; and undoing OO C programs is even less fun. I originally expected clients to derive from the pretty printers and add their own behavior. But, it turns out that since OO programming in C isn't fun, things have grown barnacles and entangled in ways I did not intend or anticipate. Hopefully, now that we have a better abstraction tool, we can express the intent much more directly. I will send in a patch that adds the inheritance chain from pretty_printer to cxx_pretty_printer. Unfortunately, it does not add the virtual functions. This is because I want to handle 'constructors' in a separate patch. -- Gaby
