C++ PATCH for c++/68703 (dependent vector length)

C++ PATCH for c++/68703 (dependent vector length)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

When we defer dependent attributes, we need to take that into account
for determining whether something is dependent.  Furthermore, such an
attribute means that the TREE_TYPE is incomplete, so we shouldn't use
it at all.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: 68703.diff --]
[-- Type: text/plain, Size: 5895 bytes --]

commit 931555dc7ab081ea7af1e4b570256d5e9729c19f
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Aug 9 17:02:57 2016 -0400

    	PR c++/68703 - bogus error with dependent vector length
    
    gcc/c-family/
    	* c-common.c (c_common_attribute_table): vector_size affects type
    	identity.
    gcc/cp/
    	* decl2.c (any_dependent_type_attributes_p): New.
    	* pt.c (dependent_type_p_r, type_dependent_expression_p): Check it.
    	* semantics.c (finish_id_expression): Check it.
    	* typeck.c (finish_class_member_access_expr): Check it.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 569f000..7fd84ee 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -754,7 +754,7 @@ const struct attribute_spec c_common_attribute_table[] =
   { "deprecated",             0, 1, false, false, false,
 			      handle_deprecated_attribute, false },
   { "vector_size",	      1, 1, false, true, false,
-			      handle_vector_size_attribute, false },
+			      handle_vector_size_attribute, true },
   { "visibility",	      1, 1, false, false, false,
 			      handle_visibility_attribute, false },
   { "tls_model",	      1, 1, true,  false, false,
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f32613c..f98b1c4 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5852,6 +5852,7 @@ extern tree grokfield (const cp_declarator *, cp_decl_specifier_seq *,
 		       tree, bool, tree, tree);
 extern tree grokbitfield (const cp_declarator *, cp_decl_specifier_seq *,
 			  tree, tree);
+extern bool any_dependent_type_attributes_p	(tree);
 extern tree cp_reconstruct_complex_type		(tree, tree);
 extern bool attributes_naming_typedef_ok	(tree);
 extern void cplus_decl_attributes		(tree *, tree, int);
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 1daa9f5..55bb987 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1305,6 +1305,22 @@ save_template_attributes (tree *attr_p, tree *decl_p)
     }
 }
 
+/* True if ATTRS contains any dependent attributes that affect type
+   identity.  */
+
+bool
+any_dependent_type_attributes_p (tree attrs)
+{
+  for (tree a = attrs; a; a = TREE_CHAIN (a))
+    if (ATTR_IS_DEPENDENT (a))
+      {
+	const attribute_spec *as = lookup_attribute_spec (TREE_PURPOSE (a));
+	if (as && as->affects_type_identity)
+	  return true;
+      }
+  return false;
+}
+
 /* Return true iff ATTRS are acceptable attributes to be applied in-place
    to a typedef which gives a previously unnamed class or enum a name for
    linkage purposes.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3884082..2638564 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -22673,6 +22673,9 @@ dependent_type_p_r (tree type)
   if (TREE_CODE (type) == TYPE_PACK_EXPANSION)
     return true;
 
+  if (any_dependent_type_attributes_p (TYPE_ATTRIBUTES (type)))
+    return true;
+
   /* The standard does not specifically mention types that are local
      to template functions or local classes, but they should be
      considered dependent too.  For example:
@@ -23225,6 +23228,12 @@ type_dependent_expression_p (tree expression)
 
   gcc_assert (TREE_CODE (expression) != TYPE_DECL);
 
+  /* Dependent type attributes might not have made it from the decl to
+     the type yet.  */
+  if (DECL_P (expression)
+      && any_dependent_type_attributes_p (DECL_ATTRIBUTES (expression)))
+    return true;
+
   return (dependent_type_p (TREE_TYPE (expression)));
 }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 4bffe6d..bffdddb 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3548,6 +3548,12 @@ finish_id_expression (tree id_expression,
 	 resolve the name at instantiation time.  */
       if (dependent_p)
 	{
+	  if (DECL_P (decl)
+	      && any_dependent_type_attributes_p (DECL_ATTRIBUTES (decl)))
+	    /* Dependent type attributes on the decl mean that the TREE_TYPE is
+	       wrong, so just return the identifier.  */
+	    return id_expression;
+
 	  /* If we found a variable, then name lookup during the
 	     instantiation will always resolve to the same VAR_DECL
 	     (or an instantiation thereof).  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index bedc453..a591d29 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2853,6 +2853,11 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p,
 	    }
 	  if (member == error_mark_node)
 	    return error_mark_node;
+	  if (DECL_P (member)
+	      && any_dependent_type_attributes_p (DECL_ATTRIBUTES (member)))
+	    /* Dependent type attributes on the decl mean that the TREE_TYPE is
+	       wrong, so don't use it.  */
+	    goto dependent;
 	  if (TREE_CODE (member) == USING_DECL && DECL_DEPENDENT_P (member))
 	    goto dependent;
 	}
diff --git a/gcc/testsuite/g++.dg/ext/vector32.C b/gcc/testsuite/g++.dg/ext/vector32.C
new file mode 100644
index 0000000..8901d0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/vector32.C
@@ -0,0 +1,18 @@
+// PR c++/68703
+
+template <int N>
+struct D {
+  int v __attribute__((vector_size (N * sizeof (int))));
+  int f1 () { return this->v[N-1]; }
+  int f2 () { return v[N-1]; }
+};
+
+int
+main ()
+{
+  D<4> a = { { 0, 1, 2, 3 } };
+  D<8> b = { { 0, 1, 2, 3, 4, 5, 6, 7 } };
+  if (a.f1 () != 3 || a.f2 () != 3
+      || b.f1 () != 7 || b.f2 () != 7)
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/ext/vector32a.C b/gcc/testsuite/g++.dg/ext/vector32a.C
new file mode 100644
index 0000000..8901d0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/vector32a.C
@@ -0,0 +1,18 @@
+// PR c++/68703
+
+template <int N>
+struct D {
+  int v __attribute__((vector_size (N * sizeof (int))));
+  int f1 () { return this->v[N-1]; }
+  int f2 () { return v[N-1]; }
+};
+
+int
+main ()
+{
+  D<4> a = { { 0, 1, 2, 3 } };
+  D<8> b = { { 0, 1, 2, 3, 4, 5, 6, 7 } };
+  if (a.f1 () != 3 || a.f2 () != 3
+      || b.f1 () != 7 || b.f2 () != 7)
+    __builtin_abort ();
+}

Re: C++ PATCH for c++/68703 (dependent vector length)

[Jakub Jelinek]

On Tue, Aug 09, 2016 at 06:02:45PM -0400, Jason Merrill wrote:
> diff --git a/gcc/testsuite/g++.dg/ext/vector32.C b/gcc/testsuite/g++.dg/ext/vector32.C
> new file mode 100644
> index 0000000..8901d0b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/vector32.C
> @@ -0,0 +1,18 @@
> +// PR c++/68703
> +
> +template <int N>
> +struct D {
> +  int v __attribute__((vector_size (N * sizeof (int))));
> +  int f1 () { return this->v[N-1]; }
> +  int f2 () { return v[N-1]; }
> +};
> +
> +int
> +main ()
> +{
> +  D<4> a = { { 0, 1, 2, 3 } };
> +  D<8> b = { { 0, 1, 2, 3, 4, 5, 6, 7 } };
> +  if (a.f1 () != 3 || a.f2 () != 3
> +      || b.f1 () != 7 || b.f2 () != 7)
> +    __builtin_abort ();
> +}
> diff --git a/gcc/testsuite/g++.dg/ext/vector32a.C b/gcc/testsuite/g++.dg/ext/vector32a.C
> new file mode 100644
> index 0000000..8901d0b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/vector32a.C

This test is the same as the other one.  Did you mean to use
  typedef int V __attribute__((vector_size (N * sizeof (int))));
  V v;
instead or something similar (where the attribute would live on
the type rather than on the FIELD_DECL?

	Jakub

Re: C++ PATCH for c++/68703 (dependent vector length)

[Jason Merrill]

On Tue, Aug 9, 2016 at 6:11 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> This test is the same as the other one.  Did you mean to use
>   typedef int V __attribute__((vector_size (N * sizeof (int))));
>   V v;
> instead or something similar (where the attribute would live on
> the type rather than on the FIELD_DECL?

Argh, yes, I thought I had copied your other testcase.  Which isn't fixed yet...

Jason

Re: C++ PATCH for c++/68703 (dependent vector length)

[Jason Merrill]

On Wed, Sep 28, 2016 at 2:43 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> Hi, Jason
>
> This patch added testcases g++.dg/ext/vector32.C and vector32a.C, but
> there is no gcc/testsuite/ChangeLog entry.
>
> The testcases are failing on AIX with a strange ICE:
>
> src/gcc/testsuite/g++.dg/ext/vector32.C: In function 'int main()':
> src/gcc/testsuite/g++.dg/ext/vector32.C:18:1: internal compiler error:
> in get, at cgraph.h:395
>
> Do you want me to open a new PR for this?

Just add it to the PR, I think, the fix wasn't complete.

Jason

Re: C++ PATCH for c++/68703 (dependent vector length)

[David Edelsohn]

Hi, Jason

This patch added testcases g++.dg/ext/vector32.C and vector32a.C, but
there is no gcc/testsuite/ChangeLog entry.

The testcases are failing on AIX with a strange ICE:

src/gcc/testsuite/g++.dg/ext/vector32.C: In function 'int main()':
src/gcc/testsuite/g++.dg/ext/vector32.C:18:1: internal compiler error:
in get, at cgraph.h:395

Do you want me to open a new PR for this?

Thanks, David