diff mbox

[Annotalysis] Fix internal compiler error on template methods

Message ID 4E84DA18.3080009@google.com
State New
Headers show

Commit Message

Diego Novillo Sept. 29, 2011, 8:50 p.m. UTC
On 11-09-29 15:19 , Delesley Hutchins wrote:

> However, I'm not convinced that it will work in all cases.  I wish I
> could come up with a test case, but like I said, I don't understand
> enough about clones to understand what's happening here.  If you are
> confident that DECL_CLONED_FUNCTION_P is correct, then we can use
> that; I personally had no such confidence.

Yes, there is no case in which a TEMPLATE_DECL will match 
DECL_CLONED_FUNCTION_P.  I agree with you that the logic in 
decl_cloned_function_p seems like it may allow that, but clones of 
functions are created as FUNCTION_DECLs.

In looking at the code again, I think we could even simplify it a bit 
more using FOR_EACH_CLONE.  This does the same work as the if()/for() 
combination in a more compact way.


Diego.

        gcc_assert (decl && decl != error_mark_node);
@@ -19322,16 +19323,9 @@ cp_parser_late_parsing_attribute_arg_lis

        /* If decl has clones (when it is a ctor or a dtor), we need to
           modify the clones' attributes as well.  */
-      if (TREE_CODE (decl) == FUNCTION_DECL
-          && (DECL_CONSTRUCTOR_P (decl) || DECL_DESTRUCTOR_P (decl)))
-        {
-          tree clone;
-          for (clone = TREE_CHAIN (decl); clone; clone = TREE_CHAIN 
(clone))
-            {
-              if (DECL_CLONED_FUNCTION (clone) == decl)
-                DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
-            }
-        }
+      FOR_EACH_CLONE (clone, decl)
+        if (DECL_CLONED_FUNCTION (clone) == decl)
+          DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);

        pop_nested_class ();

Comments

Delesley Hutchins Sept. 29, 2011, 9:24 p.m. UTC | #1
That seems much cleaner!  LGTM.  Are you submitting the patch?  I can
submit the test case.

  -DeLesley

On Thu, Sep 29, 2011 at 1:50 PM, Diego Novillo <dnovillo@google.com> wrote:
> On 11-09-29 15:19 , Delesley Hutchins wrote:
>
>> However, I'm not convinced that it will work in all cases.  I wish I
>> could come up with a test case, but like I said, I don't understand
>> enough about clones to understand what's happening here.  If you are
>> confident that DECL_CLONED_FUNCTION_P is correct, then we can use
>> that; I personally had no such confidence.
>
> Yes, there is no case in which a TEMPLATE_DECL will match
> DECL_CLONED_FUNCTION_P.  I agree with you that the logic in
> decl_cloned_function_p seems like it may allow that, but clones of functions
> are created as FUNCTION_DECLs.
>
> In looking at the code again, I think we could even simplify it a bit more
> using FOR_EACH_CLONE.  This does the same work as the if()/for() combination
> in a more compact way.
>
>
> Diego.
>
> Index: cp/parser.c
> ===================================================================
> --- cp/parser.c (revision 179065)
> +++ cp/parser.c (working copy)
> @@ -19279,6 +19279,7 @@ cp_parser_late_parsing_attribute_arg_lis
>       cp_token_cache *tokens = (cp_token_cache *) TREE_VALUE
> (artificial_node);
>       tree ctype;
>       VEC(tree,gc) *vec;
> +      tree clone;
>
>       gcc_assert (tokens);
>       gcc_assert (decl && decl != error_mark_node);
> @@ -19322,16 +19323,9 @@ cp_parser_late_parsing_attribute_arg_lis
>
>       /* If decl has clones (when it is a ctor or a dtor), we need to
>          modify the clones' attributes as well.  */
> -      if (TREE_CODE (decl) == FUNCTION_DECL
> -          && (DECL_CONSTRUCTOR_P (decl) || DECL_DESTRUCTOR_P (decl)))
> -        {
> -          tree clone;
> -          for (clone = TREE_CHAIN (decl); clone; clone = TREE_CHAIN
> (clone))
> -            {
> -              if (DECL_CLONED_FUNCTION (clone) == decl)
> -                DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
> -            }
> -        }
> +      FOR_EACH_CLONE (clone, decl)
> +        if (DECL_CLONED_FUNCTION (clone) == decl)
> +          DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
>
>       pop_nested_class ();
>
>
Diego Novillo Sept. 29, 2011, 9:44 p.m. UTC | #2
On 11-09-29 17:24 , Delesley Hutchins wrote:
> That seems much cleaner!  LGTM.  Are you submitting the patch?  I can
> submit the test case.

I tested it on an unclean tree, sorry.  I'd rather leave the testing and 
final submit to you.


Thanks.  Diego.
Delesley Hutchins Sept. 29, 2011, 9:49 p.m. UTC | #3
No problem.  Will do.

  -DeLesley

On Thu, Sep 29, 2011 at 2:44 PM, Diego Novillo <dnovillo@google.com> wrote:
> On 11-09-29 17:24 , Delesley Hutchins wrote:
>>
>> That seems much cleaner!  LGTM.  Are you submitting the patch?  I can
>> submit the test case.
>
> I tested it on an unclean tree, sorry.  I'd rather leave the testing and
> final submit to you.
>
>
> Thanks.  Diego.
>
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 179065)
+++ cp/parser.c (working copy)
@@ -19279,6 +19279,7 @@  cp_parser_late_parsing_attribute_arg_lis
        cp_token_cache *tokens = (cp_token_cache *) TREE_VALUE 
(artificial_node);
        tree ctype;
        VEC(tree,gc) *vec;
+      tree clone;

        gcc_assert (tokens);