Re: [PATCH 3/4] New attribute lookup function addition

[Martin Liška <mliska at suse dot cz>]

On 05/30/2014 06:37 PM, Jeff Law wrote:
> On 05/30/14 00:49, Martin Liška wrote:
> > Hi,
> >     this patch introduces a new function lookup_attribute_starting that
> > can find all attributes starting with a specified string. Purpose of the
> > function is to be able to identify e.g. if a function has any 'omp'
> > attribute.
> >
> > Bootstrapped and tested on x86_64-linux.
> > OK for trunk?
> >
> > Thanks,
> > Martin
> >
> > 2014-05-29  Martin Liska  <mliska@suse.cz>
> >
> >      * tree.h (private_lookup_attribute_starting): New function.
> >      (lookup_attribute_starting): Likewise.
> >      * tree.c (private_lookup_attribute_starting): Likewise.
> private_lookup_attribute_starting needs a block comment.

Added.
> > +tree
> > +private_lookup_attribute_starting (const char *attr_name, size_t
> > attr_len, tree list)
> Long line needs to be wrapped?   Please review the patch for lines that need wrapping at 80 columns.
Fixed too.
> So it's really a lookup by prefix, so I'd probably use a name like
> lookup_attribute_by_prefix.  Why "private_" in the function name?
I used the same construction as for function 'private_is_attribute_p'; I hope the construction is fine?
> It appears it just returns the first attribute from LIST with the given prefix.  Presumably you use it iteratively.
>
> > +/* Given an attribute name ATTR_NAME and a list of attributes LIST,
> > +   return a pointer to the attribute's list element if the attribute
> > +   starts with ATTR_NAME. ATTR_NAME must be in the form 'text' (not
> > +   '__text__').  */
> > +
> > +static inline tree
> > +lookup_attribute_starting (const char *attr_name, tree list)
> > +{
> > +  gcc_checking_assert (attr_name[0] != '_');
> > +  /* In most cases, list is NULL_TREE.  */
> > +  if (list == NULL_TREE)
> > +    return NULL_TREE;
> > +  else
> > +    return private_lookup_attribute_starting (attr_name, strlen
> > (attr_name), list);
> > +}
> So again, I prefer "prefix" rather than "starting".  Similarly this is meant to be called iteratively since you only get the first attribute with the given prefix, right?
I added a comment that it returns just such first argument.

Is the reworked patch OK for trunk?

Martin
> OK with the nit fixes mentioned above.
>
>
> Jeff

>From be3ab469ee70ff3de434f5326c1a2eabf07da3ed Mon Sep 17 00:00:00 2001
Message-Id: <be3ab469ee70ff3de434f5326c1a2eabf07da3ed.1401718733.git.mliska@suse.cz>
In-Reply-To: <e245d67afb610a2f210b83382b49f75731ba68b8.1401718733.git.mliska@suse.cz>
References: <e245d67afb610a2f210b83382b49f75731ba68b8.1401718733.git.mliska@suse.cz>
From: mliska <mliska@suse.cz>
Date: Thu, 29 May 2014 17:18:34 +0200
Subject: [PATCH 3/4] New attribute lookup function addition
To: gcc-patches@gcc.gnu.org

Hi,
   this patch introduces a new function lookup_attribute_starting that can find all attributes starting with a specified string. Purpose of the function is to be able to identify e.g. if a function has any 'omp' attribute.

Bootstrapped and tested on x86_64-linux.
OK for trunk?

Thanks,
Martin

2014-05-29  Martin Liska  <mliska@suse.cz>

	* tree.h (private_lookup_attribute_starting): New function.
	(lookup_attribute_starting): Likewise.
	* tree.c (private_lookup_attribute_starting): Likewise.

diff --git a/gcc/tree.c b/gcc/tree.c
index cf7e362..f983408 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -5758,6 +5758,44 @@ private_lookup_attribute (const char *attr_name, size_t attr_len, tree list)
   return list;
 }
 
+/* Given an attribute name ATTR_NAME and a list of attributes LIST,
+   return a pointer to the attribute's list first element if the attribute
+   starts with ATTR_NAME. ATTR_NAME must be in the form 'text' (not
+   '__text__').  */
+
+tree
+private_lookup_attribute_by_prefix (const char *attr_name, size_t attr_len,
+				    tree list)
+{
+  while (list)
+    {
+      size_t ident_len = IDENTIFIER_LENGTH (get_attribute_name (list));
+
+      if (attr_len > ident_len)
+	{
+	  list = TREE_CHAIN (list);
+	  continue;
+	}
+
+      const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
+
+      if (strncmp (attr_name, p, attr_len) == 0)
+	break;
+
+      /* TODO: If we made sure that attributes were stored in the
+	 canonical form without '__...__' (ie, as in 'text' as opposed
+	 to '__text__') then we could avoid the following case.  */
+      if (p[0] == '_' && p[1] == '_' &&
+	  strncmp (attr_name, p + 2, attr_len) == 0)
+	break;
+
+      list = TREE_CHAIN (list);
+    }
+
+  return list;
+}
+
+
 /* A variant of lookup_attribute() that can be used with an identifier
    as the first argument, and where the identifier can be either
    'text' or '__text__'.
diff --git a/gcc/tree.h b/gcc/tree.h
index 9fe7360..e592280 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3731,6 +3731,10 @@ extern tree merge_type_attributes (tree, tree);
    and you should never call it directly.  */
 extern tree private_lookup_attribute (const char *, size_t, tree);
 
+/* This function is a private implementation detail
+   of lookup_attribute_by_prefix() and you should never call it directly.  */
+extern tree private_lookup_attribute_by_prefix (const char *, size_t, tree);
+
 /* Given an attribute name ATTR_NAME and a list of attributes LIST,
    return a pointer to the attribute's list element if the attribute
    is part of the list, or NULL_TREE if not found.  If the attribute
@@ -3753,6 +3757,24 @@ lookup_attribute (const char *attr_name, tree list)
     return private_lookup_attribute (attr_name, strlen (attr_name), list);
 }
 
+/* Given an attribute name ATTR_NAME and a list of attributes LIST,
+   return a pointer to the attribute's list first element if the attribute
+   starts with ATTR_NAME. ATTR_NAME must be in the form 'text' (not
+   '__text__').  */
+
+static inline tree
+lookup_attribute_by_prefix (const char *attr_name, tree list)
+{
+  gcc_checking_assert (attr_name[0] != '_');
+  /* In most cases, list is NULL_TREE.  */
+  if (list == NULL_TREE)
+    return NULL_TREE;
+  else
+    return private_lookup_attribute_by_prefix (attr_name, strlen (attr_name),
+					       list);
+}
+
+
 /* This function is a private implementation detail of
    is_attribute_p() and you should never call it directly.  */
 extern bool private_is_attribute_p (const char *, size_t, const_tree);