Re: [PATCH][RFC] Canonize names of attributes.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jun 13, 2017 at 2:32 PM, Martin Liška <mliska@suse.cz> wrote:
> Hello.
>
> After some discussions with Richi, I would like to propose patch that will
> come up with a canonical name of attribute names. That means __attribute__((__abi_tag__))
> will be given 'abi_tag' as IDENTIFIER_NAME of the attribute. The change can improve
> attribute name lookup and we can delete all the ugly code that compares strlen(i1)
> == strlen(i2) + 4, etc.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests (w/ default
> languages). I'm currently testing objc, obj-c++ and go.
>
> Ready to be installed?


+tree
+canonize_attr_name (tree attr_name)
+{

needs a comment.

+  if (l > 4 && s[0] == '_')
+    {
+      gcc_assert (s[1] == '_');
+      gcc_assert (s[l - 2] == '_');
+      gcc_assert (s[l - 1] == '_');
+      return get_identifier_with_length (s + 2, l - 4);
+    }

a single gcc_checking_assert please.  I think this belongs in attribs.[ch].

Seeing

diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index e1c8bdff986..6d0e9279ed6 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -316,6 +316,7 @@ c_common_has_attribute (cpp_reader *pfile)
     {
       attr_name = get_identifier ((const char *)
                                  cpp_token_as_text (pfile, token));
+      attr_name = canonize_attr_name (attr_name);

I wondered if we can save allocating the non-canonical identifier.  Like
with

tree
canonize_attr_name (const char *attr_name, size_t len)

as we can pass it IDENTIFIER_POINTER/LENGTH or the token.  OTOH
all other cases do have IDENTIFIERs already...

@ -24638,6 +24639,11 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
              else
                {
                  arguments = build_tree_list_vec (vec);
+                 tree tv;
+                 if (arguments != NULL_TREE
+                     && ((tv = TREE_VALUE (arguments)) != NULL_TREE)
+                     && TREE_CODE (tv) == IDENTIFIER_NODE)
+                     TREE_VALUE (arguments) = canonize_attr_name (tv);
                  release_tree_vector (vec);
                }

are you sure this is needed?  This seems to be solely arguments to
attributes.

The rest of the changes look good but please wait for input from FE maintainers.

Thanks,
Richard.

> Martin
>
>
> gcc/cp/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * parser.c (cp_parser_gnu_attribute_list): Canonize attribute
>         names.
>         (cp_parser_std_attribute): Likewise.
>
> gcc/go/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * go-gcc.cc (Gcc_backend::function): Use no_split_stack
>         instead of __no_split_stack__.
>
> gcc/c/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * c-parser.c (c_parser_attributes): Canonize attribute names.
>
> gcc/c-family/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * c-format.c (cmp_attribs): Simplify comparison of attributes.
>         * c-lex.c (c_common_has_attribute): Canonize attribute names.
>
> gcc/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * tree.c (cmp_attrib_identifiers): Simplify comparison of attributes.
>         (private_is_attribute_p): Likewise.
>         (private_lookup_attribute): Likewise.
>         (private_lookup_attribute_by_prefix): Likewise.
>         (remove_attribute): Likewise.
>         (canonize_attr_name): New function.
>         * tree.h: Declared here.
>
> gcc/testsuite/ChangeLog:
>
> 2017-06-09  Martin Liska  <mliska@suse.cz>
>
>         * g++.dg/cpp0x/pr65558.C: Change expected warning.
>         * gcc.dg/parm-impl-decl-1.c: Likewise.
>         * gcc.dg/parm-impl-decl-3.c: Likewise.
> ---
>  gcc/c-family/c-format.c                 |  13 ++--
>  gcc/c-family/c-lex.c                    |   1 +
>  gcc/c/c-parser.c                        |   9 +++
>  gcc/cp/parser.c                         |  11 +++-
>  gcc/go/go-gcc.cc                        |   2 +-
>  gcc/testsuite/g++.dg/cpp0x/pr65558.C    |   2 +-
>  gcc/testsuite/gcc.dg/parm-impl-decl-1.c |   2 +-
>  gcc/testsuite/gcc.dg/parm-impl-decl-3.c |   2 +-
>  gcc/tree.c                              | 108 +++++++++++---------------------
>  gcc/tree.h                              |   4 ++
>  10 files changed, 69 insertions(+), 85 deletions(-)
>
>