On 09/17/2012 11:35 AM, Dodji Seketeli wrote:
>And I wonder if we want to offer this as an optional warning for GNU
attribute syntax.
What option would be used to control this optional feature? Would
you accept this a separate patch?
Let's not worry about this for now.
+ found_attrs = true;
You don't need a separate found_attrs variable; you can just check
whether attrs is set or not. VEC_iterate sets the element pointer to
NULL at the end of the vector. You should also be able to share more of
the code between the cases where you do or do not already have the
appropriate namespace set up.
+static scoped_attributes_t*
+find_attribute_namespace (const char* ns)
+{
+ unsigned ix;
+ scoped_attributes_t *iter;
+
+ FOR_EACH_VEC_ELT (scoped_attributes_t, attributes_table, ix, iter)
+ if (ns == iter->ns
+ || (iter->ns != NULL
+ && ns != NULL
+ && !strcmp (iter->ns, ns)))
+ {
+ if (iter->attribute_hash == 0)
+ iter->attribute_hash =
+ htab_create (200, hash_attr, eq_attr, NULL);
+ return VEC_index (scoped_attributes_t, attributes_table, ix);
+ }
Rather than write the code to search the table in two places, let's call
this function from register_scoped_attributes.
You don't need to do another VEC_index since you already have the
pointer you want in 'iter'.
+attributes_array_length (const struct attribute_spec *attributes_array)
Rather than calculate this first and then pass it to
register_scoped_attributes, why not just have register_scoped_attributes
stop at a null entry?
+ if (name_space->attribute_hash == NULL)
+ name_space->attribute_hash = htab_create (200, hash_attr, eq_attr, NULL);
Why do we need this both here and in find_attribute_namespace?
}
+/*
+ If in c++-11, check if the c++-11 alignment constraint with respect
Need a blank line after the }.
+check_cxx_fundamental_alignment_constraints (tree node,
In this function it would be nice to print out the requested and
maximum alignments in the error.
Done.
I don't see that:
+ if (alignment_too_large_p)
+ error ("requested alignment is too large");
This seems like a good change in general; I'd be inclined to drop the
check for C++11 syntax. If there are multiple conflicting alignments
specified for a declaration, the only things that make sense are to
choose the largest alignment or give an error; the current behavior of
choosing the most recently-specified alignment is just broken.
Done. I realized that I forgot to check for DECL_USER_ALIGNED there.
Fixed too.
I don't see this either:
+ else if (flags & ATTR_FLAG_CXX11
+ && DECL_ALIGN (decl) > (1U << i) * BITS_PER_UNIT)
Did you send me an old patch by mistake?
Jason
