Message ID | 20100616034331.GM27105@codesourcery.com |
---|---|
State | New |
Headers | show |
On 06/15/2010 11:43 PM, Nathan Froyd wrote: > I tried the below, which I think implements your idea. But > g++.old-deja/g++.other/defarg8.C fails with: > > defarg8.C:15:30: error: the default argument for parameter 0 of 'static int AA::Baz(int)' has not yet been parsed Hmm? That error is expected by the testcase. Jason
On Wed, Jun 16, 2010 at 10:13:44AM -0400, Jason Merrill wrote: > On 06/15/2010 11:43 PM, Nathan Froyd wrote: >> I tried the below, which I think implements your idea. But >> g++.old-deja/g++.other/defarg8.C fails with: >> >> defarg8.C:15:30: error: the default argument for parameter 0 of 'static int AA::Baz(int)' has not yet been parsed > > Hmm? That error is expected by the testcase. Whoops, sorry, got the testcase totally wrong. How about g++.dg/parse/defarg8.C, failing with: defarg8.C:8:23: error: 'i' was not declared in this scope -Nathan
On Wed, Jun 16, 2010 at 07:48:09AM -0700, Nathan Froyd wrote: > On Wed, Jun 16, 2010 at 10:13:44AM -0400, Jason Merrill wrote: > > On 06/15/2010 11:43 PM, Nathan Froyd wrote: > >> I tried the below, which I think implements your idea. But > >> g++.old-deja/g++.other/defarg8.C fails with: > >> > >> defarg8.C:15:30: error: the default argument for parameter 0 of 'static int AA::Baz(int)' has not yet been parsed > > > > Hmm? That error is expected by the testcase. > > Whoops, sorry, got the testcase totally wrong. How about > g++.dg/parse/defarg8.C, failing with: > > defarg8.C:8:23: error: 'i' was not declared in this scope Ping. Where does this patch stand? Should I: - Commit the original patch? - Hold off while you consider what might be wrong with your suggestion? and/or - Debug your suggestion myself in hopes of figuring out what's wrong? -Nathan
On 06/28/2010 08:27 AM, Nathan Froyd wrote: >> defarg8.C:8:23: error: 'i' was not declared in this scope > > Ping. Where does this patch stand? Should I: > > - Commit the original patch? > > - Hold off while you consider what might be wrong with your suggestion? > and/or > > - Debug your suggestion myself in hopes of figuring out what's wrong? The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f? Jason
On Wed, Jun 30, 2010 at 02:33:20PM -0400, Jason Merrill wrote: > On 06/28/2010 08:27 AM, Nathan Froyd wrote: >> Ping. Where does this patch stand? Should I: >> >> - Commit the original patch? >> >> - Hold off while you consider what might be wrong with your suggestion? >> and/or >> >> - Debug your suggestion myself in hopes of figuring out what's wrong? > > The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f? Apparently not: (gdb) break parser.c:19635 Breakpoint 1 at 0x5620b6: file ../../gcc-tree/gcc/cp/parser.c, line 19635. (gdb) run Starting program: /home/froydnj/src/gcc-build/gcc/cc1plus -fpreprocessed defarg8.ii -quiet -dumpbase defarg8.C -mtune=generic -march=x86-64 -ansi -auxbase defarg8 -pedantic-errors -ansi -version -o defarg8.s GNU C++ (GCC) version 4.6.0 20100630 (experimental) (x86_64-unknown-linux-gnu) compiled by GNU C version 4.6.0 20100630 (experimental), GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.1 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 GNU C++ (GCC) version 4.6.0 20100630 (experimental) (x86_64-unknown-linux-gnu) compiled by GNU C version 4.6.0 20100630 (experimental), GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.1 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 Compiler executable checksum: 5ae6b5a9d000c38dae216c68720503f3 Breakpoint 1, cp_parser_save_default_args (decl=0x7ffff7567b00, parser=<value optimized out>) at ../../gcc-tree/gcc/cp/parser.c:19626 19626 cp_parser_save_default_args (cp_parser* parser, tree decl) (gdb) list 19631 probe; 19632 probe = TREE_CHAIN (probe)) 19633 if (TREE_PURPOSE (probe)) 19634 { 19635 TREE_PURPOSE (parser->unparsed_functions_queues) 19636 = tree_cons (current_class_type, decl, 19637 TREE_PURPOSE (parser->unparsed_functions_queues)); 19638 break; 19639 } 19640 } (gdb) call debug_tree (decl) <function_decl 0x7ffff7567b00 f type <function_type 0x7ffff756b3f0 type <void_type 0x7ffff7e99e70 void VOID align 8 symtab 0 alias set -1 canonical type 0x7ffff7e99e70 pointer_to_this <pointer_type 0x7ffff7e99f18>> QI size <integer_cst 0x7ffff7e814b0 constant 8> unit size <integer_cst 0x7ffff7e814d8 constant 1> align 8 symtab 0 alias set -1 canonical type 0x7ffff7eb7e70 arg-types <tree_list 0x7ffff7558938 purpose <default_arg 0x7ffff75588e8> value <integer_type 0x7ffff7e99498 int> chain <tree_list 0x7ffff7eab348 value <void_type 0x7ffff7e99e70 void>>>> public external QI file gcc.git/gcc/testsuite/g++.dg/parse/defarg8.C line 8 col 15 align 8 arguments <parm_decl 0x7ffff7e8a990 D.1388 type <integer_type 0x7ffff7e99498 int public SI size <integer_cst 0x7ffff7e816e0 constant 32> unit size <integer_cst 0x7ffff7e813e8 constant 4> align 32 symtab 0 alias set -1 canonical type 0x7ffff7e99498 precision 32 min <integer_cst 0x7ffff7e81668 -2147483648> max <integer_cst 0x7ffff7e81690 2147483647> pointer_to_this <pointer_type 0x7ffff7eac540>> SI file gcc.git/gcc/testsuite/g++.dg/parse/defarg8.C line 8 col 23 size <integer_cst 0x7ffff7e816e0 32> unit size <integer_cst 0x7ffff7e813e8 4> align 32 context <function_decl 0x7ffff7567b00 f> arg-type <integer_type 0x7ffff7e99498 int>> full-name "void f(int = <unparsed>)" chain <type_decl 0x7ffff756d000 S>> (gdb) p scope_chain->class_type $3 = (tree) 0x7ffff756b2a0 (gdb) call debug_tree (scope_chain->class_type) <record_type 0x7ffff756b2a0 S type_5 type_6 VOID align 8 symtab 0 alias set -1 canonical type 0x7ffff756b2a0 fields <var_decl 0x7ffff7ea51e0 i type <integer_type 0x7ffff7e99498 int public SI size <integer_cst 0x7ffff7e816e0 constant 32> unit size <integer_cst 0x7ffff7e813e8 constant 4> align 32 symtab 0 alias set -1 canonical type 0x7ffff7e99498 precision 32 min <integer_cst 0x7ffff7e81668 -2147483648> max <integer_cst 0x7ffff7e81690 2147483647> pointer_to_this <pointer_type 0x7ffff7eac540>> public static external decl_3 decl_5 decl_6 SI file gcc.git/gcc/testsuite/g++.dg/parse/defarg8.C line 6 col 14 size <integer_cst 0x7ffff7e816e0 32> unit size <integer_cst 0x7ffff7e813e8 4> align 32 context <record_type 0x7ffff756b2a0 S> chain <type_decl 0x7ffff756d0b8 S type <record_type 0x7ffff756b348 S> nonlocal decl_4 VOID file gcc.git/gcc/testsuite/g++.dg/parse/defarg8.C line 5 col 10 align 1 context <record_type 0x7ffff756b2a0 S> result <record_type 0x7ffff756b2a0 S> >> full-name "struct S" n_parents=0 use_template=0 interface-unknown chain <type_decl 0x7ffff756d000 S>> (gdb) p decl->decl_minimal.context $4 = (tree) 0x0 -Nathan
On 06/30/2010 03:10 PM, Nathan Froyd wrote: > On Wed, Jun 30, 2010 at 02:33:20PM -0400, Jason Merrill wrote: >> On 06/28/2010 08:27 AM, Nathan Froyd wrote: >>> Ping. Where does this patch stand? Should I: >>> >>> - Commit the original patch? >>> >>> - Hold off while you consider what might be wrong with your suggestion? >>> and/or >>> >>> - Debug your suggestion myself in hopes of figuring out what's wrong? >> >> The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f? > > Apparently not: > > (gdb) p decl->decl_minimal.context > $4 = (tree) 0x0 That's DECL_CONTEXT, not DECL_FRIEND_CONTEXT; the latter would be decl->decl_common.lang_specific->u.fn.context, I believe. In any case, the problem seems to be in do_friend: > /* Friends must all go through the overload machinery, > even though they may not technically be overloaded. > > Note that because classes all wind up being top-level > in their scope, their friend wind up in top-level scope as well. */ > if (funcdef_flag) > SET_DECL_FRIEND_CONTEXT (decl, current_class_type); What happens if you remove the if? Jason
On Wed, Jun 30, 2010 at 03:40:13PM -0400, Jason Merrill wrote: > On 06/30/2010 03:10 PM, Nathan Froyd wrote: >> On Wed, Jun 30, 2010 at 02:33:20PM -0400, Jason Merrill wrote: >>> The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f? >> >> Apparently not: >> >> (gdb) p decl->decl_minimal.context >> $4 = (tree) 0x0 > > That's DECL_CONTEXT, not DECL_FRIEND_CONTEXT; the latter would be > decl->decl_common.lang_specific->u.fn.context, I believe. Whoops, sorry about that. Fortunately that's not getting set either. :) > In any case, the problem seems to be in do_friend: > >> /* Friends must all go through the overload machinery, >> even though they may not technically be overloaded. >> >> Note that because classes all wind up being top-level >> in their scope, their friend wind up in top-level scope as well. */ >> if (funcdef_flag) >> SET_DECL_FRIEND_CONTEXT (decl, current_class_type); > > What happens if you remove the if? If I set funcdef_flag to true at the start of do_friend when f is getting defined, things work as expected. grokdeclarator attempts to grok f in FIELD context, which explains why funcdef_flag doesn't get set in grokdeclarator. What's the right fix? Presumably the test shouldn't be removed in do_friend...should grokdeclarator be checking declarator->kind? -Nathan
On 06/30/2010 03:55 PM, Nathan Froyd wrote: >>> /* Friends must all go through the overload machinery, >>> even though they may not technically be overloaded. >>> >>> Note that because classes all wind up being top-level >>> in their scope, their friend wind up in top-level scope as well. */ >>> if (funcdef_flag) >>> SET_DECL_FRIEND_CONTEXT (decl, current_class_type); >> >> What happens if you remove the if? > > If I set funcdef_flag to true at the start of do_friend when f is > getting defined, things work as expected. grokdeclarator attempts to > grok f in FIELD context, which explains why funcdef_flag doesn't get set > in grokdeclarator. And it shouldn't get set, because f is not being defined, only declared. > What's the right fix? Presumably the test shouldn't be removed in > do_friend...should grokdeclarator be checking declarator->kind? Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify that it's only set for definitions. But I would be sure to test that we give the appropriate errors if you have a friend declaration followed by a global definition that refers to static members of the type without explicit qualification. Jason
Jason Merrill wrote: >> What's the right fix? Presumably the test shouldn't be removed in >> do_friend...should grokdeclarator be checking declarator->kind? > > Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify > that it's only set for definitions. Indeed, but I think it should specify that, probably. For example, push_access_scope honors it -- and that only makes sense for definitions, I think? Also, what about duplicate_decls? If we merge two declarations of the same function, declared as a friend in two different classes, what should DECL_FRIEND_CONTEXT be?
On 07/01/2010 01:09 AM, Mark Mitchell wrote: > Jason Merrill wrote: > >>> What's the right fix? Presumably the test shouldn't be removed in >>> do_friend...should grokdeclarator be checking declarator->kind? >> >> Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify >> that it's only set for definitions. > > Indeed, but I think it should specify that, probably. For example, > push_access_scope honors it -- and that only makes sense for > definitions, I think? > > Also, what about duplicate_decls? If we merge two declarations of the > same function, declared as a friend in two different classes, what > should DECL_FRIEND_CONTEXT be? Right. I was thinking that we could clear DECL_FRIEND_CONTEXT after we're done processing the class, but I guess it's safer to just update the comment and go with Nathan's earlier patch. Jason
Index: parser.c =================================================================== --- parser.c (revision 160819) +++ parser.c (working copy) @@ -1514,6 +1514,21 @@ cp_parser_context_new (cp_parser_context return context; } +/* An entry in a stack for member functions of local classes. */ + +typedef struct GTY(()) cp_unparsed_functions_entry_d { + /* Functions with default arguments that require post-processing. + Functions appear in this list in declaration order. */ + VEC(tree,gc) *funs_with_default_args; + + /* Functions with defintions that require post-processing. Functions + appear in this list in declaration order. */ + VEC(tree,gc) *funs_with_definitions; +} cp_unparsed_functions_entry; + +DEF_VEC_O(cp_unparsed_functions_entry); +DEF_VEC_ALLOC_O(cp_unparsed_functions_entry,gc); + /* The cp_parser structure represents the C++ parser. */ typedef struct GTY(()) cp_parser { @@ -1640,21 +1655,10 @@ typedef struct GTY(()) cp_parser { issued as an error message if a type is defined. */ const char *type_definition_forbidden_message; - /* A list of lists. The outer list is a stack, used for member - functions of local classes. At each level there are two sub-list, - one on TREE_VALUE and one on TREE_PURPOSE. Each of those - sub-lists has a FUNCTION_DECL or TEMPLATE_DECL on their - TREE_VALUE's. The functions are chained in reverse declaration - order. - - The TREE_PURPOSE sublist contains those functions with default - arguments that need post processing, and the TREE_VALUE sublist - contains those functions with definitions that need post - processing. - - These lists can only be processed once the outermost class being - defined is complete. */ - tree unparsed_functions_queues; + /* A stack used for member functions of local classes. The lists + contained in an individual entry can only be processed once the + outermost class being defined is complete. */ + VEC(cp_unparsed_functions_entry,gc) *unparsed_queues; /* The number of classes whose definitions are currently in progress. */ @@ -1665,6 +1669,30 @@ typedef struct GTY(()) cp_parser { unsigned num_template_parameter_lists; } cp_parser; +/* Managing the unparsed function queues. */ + +#define unparsed_funs_with_default_args \ + VEC_last (cp_unparsed_functions_entry, parser->unparsed_queues)->funs_with_default_args +#define unparsed_funs_with_definitions \ + VEC_last (cp_unparsed_functions_entry, parser->unparsed_queues)->funs_with_definitions + +static void +push_unparsed_function_queues (cp_parser *parser) +{ + VEC_safe_push (cp_unparsed_functions_entry, gc, + parser->unparsed_queues, NULL); + unparsed_funs_with_default_args = make_tree_vector (); + unparsed_funs_with_definitions = make_tree_vector (); +} + +static void +pop_unparsed_function_queues (cp_parser *parser) +{ + release_tree_vector (unparsed_funs_with_definitions); + release_tree_vector (unparsed_funs_with_default_args); + VEC_pop (cp_unparsed_functions_entry, parser->unparsed_queues); +} + /* Prototypes. */ /* Constructors and destructors. */ @@ -3151,7 +3179,7 @@ cp_parser_new (void) parser->in_function_body = false; /* The unparsed function queue is empty. */ - parser->unparsed_functions_queues = build_tree_list (NULL_TREE, NULL_TREE); + push_unparsed_function_queues (parser); /* There are no classes being defined. */ parser->num_classes_being_defined = 0; @@ -16248,10 +16276,10 @@ cp_parser_class_specifier (cp_parser* pa there is no need to delay the parsing of `A::B::f'. */ if (--parser->num_classes_being_defined == 0) { - tree queue_entry; tree fn; tree class_type = NULL_TREE; tree pushed_scope = NULL_TREE; + unsigned ix; /* In a first pass, parse default arguments to the functions. Then, in a second pass, parse the bodies of the functions. @@ -16263,20 +16291,20 @@ cp_parser_class_specifier (cp_parser* pa }; */ - for (TREE_PURPOSE (parser->unparsed_functions_queues) - = nreverse (TREE_PURPOSE (parser->unparsed_functions_queues)); - (queue_entry = TREE_PURPOSE (parser->unparsed_functions_queues)); - TREE_PURPOSE (parser->unparsed_functions_queues) - = TREE_CHAIN (TREE_PURPOSE (parser->unparsed_functions_queues))) - { - fn = TREE_VALUE (queue_entry); + for (ix = 0; + VEC_iterate (tree, unparsed_funs_with_default_args, ix, fn); + ix++) + { + tree ctype = (DECL_FUNCTION_MEMBER_P (fn) + ? DECL_CONTEXT (fn) + : DECL_FRIEND_CONTEXT (fn)); /* If there are default arguments that have not yet been processed, take care of them now. */ - if (class_type != TREE_PURPOSE (queue_entry)) + if (class_type != ctype) { if (pushed_scope) pop_scope (pushed_scope); - class_type = TREE_PURPOSE (queue_entry); + class_type = ctype; pushed_scope = push_scope (class_type); } /* Make sure that any template parameters are in scope. */ @@ -16288,18 +16316,13 @@ cp_parser_class_specifier (cp_parser* pa } if (pushed_scope) pop_scope (pushed_scope); + VEC_truncate (tree, unparsed_funs_with_default_args, 0); /* Now parse the body of the functions. */ - for (TREE_VALUE (parser->unparsed_functions_queues) - = nreverse (TREE_VALUE (parser->unparsed_functions_queues)); - (queue_entry = TREE_VALUE (parser->unparsed_functions_queues)); - TREE_VALUE (parser->unparsed_functions_queues) - = TREE_CHAIN (TREE_VALUE (parser->unparsed_functions_queues))) - { - /* Figure out which function we need to process. */ - fn = TREE_VALUE (queue_entry); - /* Parse the function. */ - cp_parser_late_parsing_for_member (parser, fn); - } + for (ix = 0; + VEC_iterate (tree, unparsed_funs_with_definitions, ix, fn); + ix++) + cp_parser_late_parsing_for_member (parser, fn); + VEC_truncate (tree, unparsed_funs_with_definitions, 0); } /* Put back any saved access checks. */ @@ -19152,9 +19175,7 @@ cp_parser_template_declaration_after_exp if (member_p && decl && (TREE_CODE (decl) == FUNCTION_DECL || DECL_FUNCTION_TEMPLATE_P (decl))) - TREE_VALUE (parser->unparsed_functions_queues) - = tree_cons (NULL_TREE, decl, - TREE_VALUE (parser->unparsed_functions_queues)); + VEC_safe_push (tree, gc, unparsed_funs_with_definitions, decl); } /* Perform the deferred access checks from a template-parameter-list. @@ -19424,9 +19445,7 @@ cp_parser_save_member_function_body (cp_ DECL_INITIALIZED_IN_CLASS_P (fn) = 1; /* Add FN to the queue of functions to be parsed later. */ - TREE_VALUE (parser->unparsed_functions_queues) - = tree_cons (NULL_TREE, fn, - TREE_VALUE (parser->unparsed_functions_queues)); + VEC_safe_push (tree, gc, unparsed_funs_with_definitions, fn); return fn; } @@ -19557,8 +19576,7 @@ cp_parser_late_parsing_for_member (cp_pa classes. We want to handle them right away, but we don't want them getting mixed up with functions that are currently in the queue. */ - parser->unparsed_functions_queues - = tree_cons (NULL_TREE, NULL_TREE, parser->unparsed_functions_queues); + push_unparsed_function_queues (parser); /* Make sure that any template parameters are in scope. */ maybe_begin_member_template_processing (member_function); @@ -19610,8 +19628,7 @@ cp_parser_late_parsing_for_member (cp_pa maybe_end_member_template_processing (); /* Restore the queue. */ - parser->unparsed_functions_queues - = TREE_CHAIN (parser->unparsed_functions_queues); + pop_unparsed_function_queues (parser); } /* If DECL contains any default args, remember it on the unparsed @@ -19627,9 +19644,7 @@ cp_parser_save_default_args (cp_parser* probe = TREE_CHAIN (probe)) if (TREE_PURPOSE (probe)) { - TREE_PURPOSE (parser->unparsed_functions_queues) - = tree_cons (current_class_type, decl, - TREE_PURPOSE (parser->unparsed_functions_queues)); + VEC_safe_push (tree, gc, unparsed_funs_with_default_args, decl); break; } } @@ -19649,8 +19664,7 @@ cp_parser_late_parsing_default_args (cp_ statement expression extension) encounter more classes. We want to handle them right away, but we don't want them getting mixed up with default args that are currently in the queue. */ - parser->unparsed_functions_queues - = tree_cons (NULL_TREE, NULL_TREE, parser->unparsed_functions_queues); + push_unparsed_function_queues (parser); /* Local variable names (and the `this' keyword) may not appear in a default argument. */ @@ -19722,8 +19736,7 @@ cp_parser_late_parsing_default_args (cp_ parser->local_variables_forbidden_p = saved_local_variables_forbidden_p; /* Restore the queue. */ - parser->unparsed_functions_queues - = TREE_CHAIN (parser->unparsed_functions_queues); + pop_unparsed_function_queues (parser); } /* Parse the operand of `sizeof' (or a similar operator). Returns