public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch, c++ openmp] Improve diagnostics for unmappable types
@ 2019-06-28 10:46 Andrew Stubbs
  2019-06-28 16:21 ` Jason Merrill
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Stubbs @ 2019-06-28 10:46 UTC (permalink / raw)
  To: gcc-patches

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

This patch improves the diagnostics given for unmappable C++ types in 
OpenMP programs.

Here is the output *without* the patch, for the new testcase:

----
unmappable-1.C: In function 'int main()':
unmappable-1.C:16:24: error: 'v' does not have a mappable type in 'map' 
clause
    16 | #pragma omp target map(v)
----

This is correct, but not very helpful for anything but the most trivial 
C++ types. Anything involving inheritance, templates, typedefs, etc. 
could be extremely difficult to track down.

With the patch applied we now get this (I removed the "dg-message" 
comments for clarity):

----
unmappable-1.C: In function 'int main()':
unmappable-1.C:16:24: error: 'v' does not have a mappable type in 'map' 
clause
    16 | #pragma omp target map(v)
       |                        ^
cc1plus: note: incomplete types are not mappable
unmappable-1.C:4:7: note: types with virtual members are not mappable
     4 | class C
       |       ^
unmappable-1.C:7:14: note: static fields are not mappable
     7 |   static int static_member;
       |              ^~~~~~~~~~~~~
----

The compiler now reports all the problematic fields in the whole type, 
recursively. If anybody knows how to report the location of incomplete 
array declarations then that would be nice to add.

OK to commit?

Andrew

[-- Attachment #2: 190628-gcc-unmappable.patch --]
[-- Type: text/x-patch, Size: 6576 bytes --]

Improve OpenMP map diagnostics.

2019-06-27  Andrew Stubbs  <ams@codesourcery.com>

	gcc/cp/
	* cp-tree.h (cp_omp_emit_unmappable_type_notes): New prototype.
	* decl.c (cp_finish_decl): Call cp_omp_emit_unmappable_type_notes.
	* decl2.c (cp_omp_mappable_type): Move contents to ...
	(cp_omp_mappable_type_1):  ... here and add note output.
	(cp_omp_emit_unmappable_type_notes): New function.
	* semantics.c (finish_omp_clauses): Call
	cp_omp_emit_unmappable_type_notes in four places.

	gcc/testsuite/
	* g++.dg/gomp/unmappable-1.C: New file.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index bf47f67721e..a7b2151e6dd 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6533,6 +6533,7 @@ extern int parm_index                           (tree);
 extern tree vtv_start_verification_constructor_init_function (void);
 extern tree vtv_finish_verification_constructor_init_function (tree);
 extern bool cp_omp_mappable_type		(tree);
+extern bool cp_omp_emit_unmappable_type_notes	(tree);
 extern void cp_check_const_attributes (tree);
 
 /* in error.c */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5d49535b0d9..74343bc1ec4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7433,8 +7433,11 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 			    DECL_ATTRIBUTES (decl));
       complete_type (TREE_TYPE (decl));
       if (!cp_omp_mappable_type (TREE_TYPE (decl)))
-	error ("%q+D in declare target directive does not have mappable type",
-	       decl);
+	{
+	  error ("%q+D in declare target directive does not have mappable"
+		 " type", decl);
+	  cp_omp_emit_unmappable_type_notes (TREE_TYPE (decl));
+	}
       else if (!lookup_attribute ("omp declare target",
 				  DECL_ATTRIBUTES (decl))
 	       && !lookup_attribute ("omp declare target link",
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 206f04c6320..17deeda75f8 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1406,32 +1406,83 @@ cp_check_const_attributes (tree attributes)
     }
 }
 
-/* Return true if TYPE is an OpenMP mappable type.  */
-bool
-cp_omp_mappable_type (tree type)
+/* Return true if TYPE is an OpenMP mappable type.
+   If NOTES is non-zero, emit a note message for each problem.  */
+static bool
+cp_omp_mappable_type_1 (tree type, bool notes)
 {
+  bool result = true;
+
   /* Mappable type has to be complete.  */
   if (type == error_mark_node || !COMPLETE_TYPE_P (type))
-    return false;
+    {
+      if (notes)
+	{
+	  tree decl = TYPE_MAIN_DECL (type);
+	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : UNKNOWN_LOCATION),
+		  "incomplete types are not mappable");
+	  result = false;
+	}
+      else
+        return false;
+    }
   /* Arrays have mappable type if the elements have mappable type.  */
   while (TREE_CODE (type) == ARRAY_TYPE)
     type = TREE_TYPE (type);
   /* A mappable type cannot contain virtual members.  */
   if (CLASS_TYPE_P (type) && CLASSTYPE_VTABLES (type))
-    return false;
+    {
+      if (notes)
+	{
+	  inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)),
+		  "types with virtual members are not mappable");
+	  result = false;
+	}
+      else
+	return false;
+    }
   /* All data members must be non-static.  */
   if (CLASS_TYPE_P (type))
     {
       tree field;
       for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
 	if (VAR_P (field))
-	  return false;
+	  {
+	    if (notes)
+	      {
+		inform (DECL_SOURCE_LOCATION (field),
+			"static fields are not mappable");
+		result = false;
+	      }
+	    else
+	      return false;
+	  }
 	/* All fields must have mappable types.  */
 	else if (TREE_CODE (field) == FIELD_DECL
-		 && !cp_omp_mappable_type (TREE_TYPE (field)))
-	  return false;
+		 && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
+	  {
+	    if (notes)
+	      result = false;
+	    else
+	      return false;
+	  }
     }
-  return true;
+  return result;
+}
+
+/* Return true if TYPE is an OpenMP mappable type.  */
+bool
+cp_omp_mappable_type (tree type)
+{
+  return cp_omp_mappable_type_1 (type, false);
+}
+
+/* Return true if TYPE is an OpenMP mappable type.
+   Emit an error messages if not.  */
+bool
+cp_omp_emit_unmappable_type_notes (tree type)
+{
+  return cp_omp_mappable_type_1 (type, true);
 }
 
 /* Return the last pushed declaration for the symbol DECL or NULL
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 92c48753d42..8f019580d0f 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7090,6 +7090,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 				"array section does not have mappable type "
 				"in %qs clause",
 				omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+		      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 		      remove = true;
 		    }
 		  while (TREE_CODE (t) == ARRAY_REF)
@@ -7158,6 +7159,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 		  error_at (OMP_CLAUSE_LOCATION (c),
 			    "%qE does not have a mappable type in %qs clause",
 			    t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+		  cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 		  remove = true;
 		}
 	      while (TREE_CODE (t) == COMPONENT_REF)
@@ -7236,6 +7238,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
@@ -7384,6 +7387,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  if (remove)
diff --git a/gcc/testsuite/g++.dg/gomp/unmappable-1.C b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
new file mode 100644
index 00000000000..29739240620
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp" } */
+
+class C /* { dg-message "types with virtual members are not mappable" } */
+{
+public:
+  static int static_member; /* { dg-message "static fields are not mappable" } */
+  virtual void f() {}
+};
+
+extern C v[];  /* { dg-message "note: incomplete types are not mappable" "" { target *-*-* } 0 } */
+
+int
+main ()
+{
+#pragma omp target map(v) /* { dg-error ".v. does not have a mappable type in .map. clause" } */
+  {
+  }
+}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-06-28 10:46 [patch, c++ openmp] Improve diagnostics for unmappable types Andrew Stubbs
@ 2019-06-28 16:21 ` Jason Merrill
  2019-07-01 11:16   ` Andrew Stubbs
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2019-06-28 16:21 UTC (permalink / raw)
  To: Andrew Stubbs, gcc-patches

On 6/28/19 6:46 AM, Andrew Stubbs wrote:
> This patch improves the diagnostics given for unmappable C++ types in 
> OpenMP programs.
> 
> Here is the output *without* the patch, for the new testcase:
> 
> ----
> unmappable-1.C: In function 'int main()':
> unmappable-1.C:16:24: error: 'v' does not have a mappable type in 'map' 
> clause
>     16 | #pragma omp target map(v)
> ----
> 
> This is correct, but not very helpful for anything but the most trivial 
> C++ types. Anything involving inheritance, templates, typedefs, etc. 
> could be extremely difficult to track down.
> 
> With the patch applied we now get this (I removed the "dg-message" 
> comments for clarity):
> 
> ----
> unmappable-1.C: In function 'int main()':
> unmappable-1.C:16:24: error: 'v' does not have a mappable type in 'map' 
> clause
>     16 | #pragma omp target map(v)
>        |                        ^
> cc1plus: note: incomplete types are not mappable
> unmappable-1.C:4:7: note: types with virtual members are not mappable
>      4 | class C
>        |       ^
> unmappable-1.C:7:14: note: static fields are not mappable
>      7 |   static int static_member;
>        |              ^~~~~~~~~~~~~
> ----
> 
> The compiler now reports all the problematic fields in the whole type, 
> recursively. If anybody knows how to report the location of incomplete 
> array declarations then that would be nice to add.
> 
> OK to commit?

> +	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : UNKNOWN_LOCATION),
> +		  "incomplete types are not mappable");

It's better to use input_location as a fallback; essentially no 
diagnostics should use UNKNOWN_LOCATION.  And let's print the type with %qT.

> +	    if (notes)
> +	      result = false;
> +	    else
> +	      return false;

Returning early when !notes doesn't seem worth the extra lines of code.

Jason

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-06-28 16:21 ` Jason Merrill
@ 2019-07-01 11:16   ` Andrew Stubbs
  2019-07-03 18:36     ` Jason Merrill
  2019-07-04 14:38     ` Andrew Stubbs
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Stubbs @ 2019-07-01 11:16 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches

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

On 28/06/2019 17:21, Jason Merrill wrote:
>> +      inform ((decl ? DECL_SOURCE_LOCATION (decl) : UNKNOWN_LOCATION),
>> +          "incomplete types are not mappable");
> 
> It's better to use input_location as a fallback; essentially no 
> diagnostics should use UNKNOWN_LOCATION.  And let's print the type with 
> %qT.
> 
>> +        if (notes)
>> +          result = false;
>> +        else
>> +          return false;
> 
> Returning early when !notes doesn't seem worth the extra lines of code.

How is this version?

Andrew


[-- Attachment #2: 190701-gcc-unmappable.patch --]
[-- Type: text/x-patch, Size: 6452 bytes --]

Improve OpenMP map diagnostics.

2019-07-01  Andrew Stubbs  <ams@codesourcery.com>

	gcc/cp/
	* cp-tree.h (cp_omp_emit_unmappable_type_notes): New prototype.
	* decl.c (cp_finish_decl): Call cp_omp_emit_unmappable_type_notes.
	* decl2.c (cp_omp_mappable_type): Move contents to ...
	(cp_omp_mappable_type_1):  ... here and add note output.
	(cp_omp_emit_unmappable_type_notes): New function.
	* semantics.c (finish_omp_clauses): Call
	cp_omp_emit_unmappable_type_notes in four places.

	gcc/testsuite/
	* g++.dg/gomp/unmappable-1.C: New file.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index bf47f67721e..a7b2151e6dd 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6533,6 +6533,7 @@ extern int parm_index                           (tree);
 extern tree vtv_start_verification_constructor_init_function (void);
 extern tree vtv_finish_verification_constructor_init_function (tree);
 extern bool cp_omp_mappable_type		(tree);
+extern bool cp_omp_emit_unmappable_type_notes	(tree);
 extern void cp_check_const_attributes (tree);
 
 /* in error.c */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5d49535b0d9..74343bc1ec4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7433,8 +7433,11 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 			    DECL_ATTRIBUTES (decl));
       complete_type (TREE_TYPE (decl));
       if (!cp_omp_mappable_type (TREE_TYPE (decl)))
-	error ("%q+D in declare target directive does not have mappable type",
-	       decl);
+	{
+	  error ("%q+D in declare target directive does not have mappable"
+		 " type", decl);
+	  cp_omp_emit_unmappable_type_notes (TREE_TYPE (decl));
+	}
       else if (!lookup_attribute ("omp declare target",
 				  DECL_ATTRIBUTES (decl))
 	       && !lookup_attribute ("omp declare target link",
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 206f04c6320..b415716c7dd 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1406,32 +1406,68 @@ cp_check_const_attributes (tree attributes)
     }
 }
 
-/* Return true if TYPE is an OpenMP mappable type.  */
-bool
-cp_omp_mappable_type (tree type)
+/* Return true if TYPE is an OpenMP mappable type.
+   If NOTES is non-zero, emit a note message for each problem.  */
+static bool
+cp_omp_mappable_type_1 (tree type, bool notes)
 {
+  bool result = true;
+
   /* Mappable type has to be complete.  */
   if (type == error_mark_node || !COMPLETE_TYPE_P (type))
-    return false;
+    {
+      if (notes)
+	{
+	  tree decl = TYPE_MAIN_DECL (type);
+	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
+		  "incomplete type %qT is not mappable", type);
+	}
+      result = false;
+    }
   /* Arrays have mappable type if the elements have mappable type.  */
   while (TREE_CODE (type) == ARRAY_TYPE)
     type = TREE_TYPE (type);
   /* A mappable type cannot contain virtual members.  */
   if (CLASS_TYPE_P (type) && CLASSTYPE_VTABLES (type))
-    return false;
+    {
+      if (notes)
+	inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)),
+		"type %qT with virtual members is not mappable", type);
+      result = false;
+    }
   /* All data members must be non-static.  */
   if (CLASS_TYPE_P (type))
     {
       tree field;
       for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
 	if (VAR_P (field))
-	  return false;
+	  {
+	    if (notes)
+	      inform (DECL_SOURCE_LOCATION (field),
+		      "static field %qD is not mappable", field);
+	    result = false;
+	  }
 	/* All fields must have mappable types.  */
 	else if (TREE_CODE (field) == FIELD_DECL
-		 && !cp_omp_mappable_type (TREE_TYPE (field)))
-	  return false;
+		 && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
+	  result = false;
     }
-  return true;
+  return result;
+}
+
+/* Return true if TYPE is an OpenMP mappable type.  */
+bool
+cp_omp_mappable_type (tree type)
+{
+  return cp_omp_mappable_type_1 (type, false);
+}
+
+/* Return true if TYPE is an OpenMP mappable type.
+   Emit an error messages if not.  */
+bool
+cp_omp_emit_unmappable_type_notes (tree type)
+{
+  return cp_omp_mappable_type_1 (type, true);
 }
 
 /* Return the last pushed declaration for the symbol DECL or NULL
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 92c48753d42..8f019580d0f 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7090,6 +7090,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 				"array section does not have mappable type "
 				"in %qs clause",
 				omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+		      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 		      remove = true;
 		    }
 		  while (TREE_CODE (t) == ARRAY_REF)
@@ -7158,6 +7159,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 		  error_at (OMP_CLAUSE_LOCATION (c),
 			    "%qE does not have a mappable type in %qs clause",
 			    t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+		  cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 		  remove = true;
 		}
 	      while (TREE_CODE (t) == COMPONENT_REF)
@@ -7236,6 +7238,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
@@ -7384,6 +7387,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
+	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  if (remove)
diff --git a/gcc/testsuite/g++.dg/gomp/unmappable-1.C b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
new file mode 100644
index 00000000000..d00ccb5ad79
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp" } */
+
+class C /* { dg-message "type .C. with virtual members is not mappable" } */
+{
+public:
+  static int static_member; /* { dg-message "static field .C::static_member. is not mappable" } */
+  virtual void f() {}
+};
+
+extern C v[];
+
+int
+main ()
+{
+#pragma omp target map(v) /* { dg-error ".v. does not have a mappable type in .map. clause" } */
+  /* { dg-message "incomplete type .C \\\[\\\]. is not mappable" "" { target *-*-* } .-1 } */
+  {
+  }
+}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-07-01 11:16   ` Andrew Stubbs
@ 2019-07-03 18:36     ` Jason Merrill
  2019-07-04 12:14       ` Andrew Stubbs
  2019-07-04 14:38     ` Andrew Stubbs
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2019-07-03 18:36 UTC (permalink / raw)
  To: Andrew Stubbs, gcc-patches

On 7/1/19 7:16 AM, Andrew Stubbs wrote:
> On 28/06/2019 17:21, Jason Merrill wrote:
>>> +      inform ((decl ? DECL_SOURCE_LOCATION (decl) : UNKNOWN_LOCATION),
>>> +          "incomplete types are not mappable");
>>
>> It's better to use input_location as a fallback; essentially no 
>> diagnostics should use UNKNOWN_LOCATION.  And let's print the type 
>> with %qT.
>>
>>> +        if (notes)
>>> +          result = false;
>>> +        else
>>> +          return false;
>>
>> Returning early when !notes doesn't seem worth the extra lines of code.
> 
> How is this version?

OK, thanks.

Jason

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-07-03 18:36     ` Jason Merrill
@ 2019-07-04 12:14       ` Andrew Stubbs
  2019-07-08 22:13         ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Stubbs @ 2019-07-04 12:14 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches

On 03/07/2019 18:58, Jason Merrill wrote:
> OK, thanks.

Committed.

Thanks for the reviews.

Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [OG9] Improve diagnostics for unmappable types
  2019-07-01 11:16   ` Andrew Stubbs
  2019-07-03 18:36     ` Jason Merrill
@ 2019-07-04 14:38     ` Andrew Stubbs
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Stubbs @ 2019-07-04 14:38 UTC (permalink / raw)
  To: gcc-patches

This patch has now been backported to openacc-gcc-9-branch (git).

Andre

On 01/07/2019 12:16, Andrew Stubbs wrote:
> Improve OpenMP map diagnostics.
> 
> 2019-07-01  Andrew Stubbs<ams@codesourcery.com>
> 
> 	gcc/cp/
> 	* cp-tree.h (cp_omp_emit_unmappable_type_notes): New prototype.
> 	* decl.c (cp_finish_decl): Call cp_omp_emit_unmappable_type_notes.
> 	* decl2.c (cp_omp_mappable_type): Move contents to ...
> 	(cp_omp_mappable_type_1):  ... here and add note output.
> 	(cp_omp_emit_unmappable_type_notes): New function.
> 	* semantics.c (finish_omp_clauses): Call
> 	cp_omp_emit_unmappable_type_notes in four places.
> 
> 	gcc/testsuite/
> 	* g++.dg/gomp/unmappable-1.C: New file.
> 
> =======================================
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index bf47f67721e..a7b2151e6dd 100644
> 
> --- a/gcc/cp/cp-tree.h
> 
> +++ b/gcc/cp/cp-tree.h
> 
> @@ -6533,6 +6533,7 @@
> 
>   extern int parm_index                           (tree);
> 
> extern tree vtv_start_verification_constructor_init_function (void);
> extern tree vtv_finish_verification_constructor_init_function (tree);
> extern bool cp_omp_mappable_type (tree);
> +extern bool cp_omp_emit_unmappable_type_notes (tree);
> extern void cp_check_const_attributes (tree);
> /* in error.c */
> 
> =======================================
> 
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 5d49535b0d9..74343bc1ec4 100644
> 
> --- a/gcc/cp/decl.c
> 
> +++ b/gcc/cp/decl.c
> 
> @@ -7433,8 +7433,11 @@
> 
>   cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
> 
> DECL_ATTRIBUTES (decl));
> complete_type (TREE_TYPE (decl));
> if (!cp_omp_mappable_type (TREE_TYPE (decl)))
> - error ("%q+D in declare target directive does not have mappable type",
> - decl);
> + {
> + error ("%q+D in declare target directive does not have mappable"
> + " type", decl);
> + cp_omp_emit_unmappable_type_notes (TREE_TYPE (decl));
> + }
> else if (!lookup_attribute ("omp declare target",
> DECL_ATTRIBUTES (decl))
> && !lookup_attribute ("omp declare target link",
> 
> =======================================
> 
> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> index 206f04c6320..b415716c7dd 100644
> 
> --- a/gcc/cp/decl2.c
> 
> +++ b/gcc/cp/decl2.c
> 
> @@ -1406,32 +1406,68 @@
> 
>   cp_check_const_attributes (tree attributes)
> 
> }
> }
> -/* Return true if TYPE is an OpenMP mappable type. */
> -bool
> -cp_omp_mappable_type (tree type)
> +/* Return true if TYPE is an OpenMP mappable type.
> + If NOTES is non-zero, emit a note message for each problem. */
> +static bool
> +cp_omp_mappable_type_1 (tree type, bool notes)
> {
> + bool result = true;
> +
> /* Mappable type has to be complete. */
> if (type == error_mark_node || !COMPLETE_TYPE_P (type))
> - return false;
> + {
> + if (notes)
> + {
> + tree decl = TYPE_MAIN_DECL (type);
> + inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
> + "incomplete type %qT is not mappable", type);
> + }
> + result = false;
> + }
> /* Arrays have mappable type if the elements have mappable type. */
> while (TREE_CODE (type) == ARRAY_TYPE)
> type = TREE_TYPE (type);
> /* A mappable type cannot contain virtual members. */
> if (CLASS_TYPE_P (type) && CLASSTYPE_VTABLES (type))
> - return false;
> + {
> + if (notes)
> + inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)),
> + "type %qT with virtual members is not mappable", type);
> + result = false;
> + }
> /* All data members must be non-static. */
> if (CLASS_TYPE_P (type))
> {
> tree field;
> for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> if (VAR_P (field))
> - return false;
> + {
> + if (notes)
> + inform (DECL_SOURCE_LOCATION (field),
> + "static field %qD is not mappable", field);
> + result = false;
> + }
> /* All fields must have mappable types. */
> else if (TREE_CODE (field) == FIELD_DECL
> - && !cp_omp_mappable_type (TREE_TYPE (field)))
> - return false;
> + && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
> + result = false;
> }
> - return true;
> + return result;
> +}
> +
> +/* Return true if TYPE is an OpenMP mappable type. */
> +bool
> +cp_omp_mappable_type (tree type)
> +{
> + return cp_omp_mappable_type_1 (type, false);
> +}
> +
> +/* Return true if TYPE is an OpenMP mappable type.
> + Emit an error messages if not. */
> +bool
> +cp_omp_emit_unmappable_type_notes (tree type)
> +{
> + return cp_omp_mappable_type_1 (type, true);
> }
> /* Return the last pushed declaration for the symbol DECL or NULL
> 
> =======================================
> 
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 92c48753d42..8f019580d0f 100644
> 
> --- a/gcc/cp/semantics.c
> 
> +++ b/gcc/cp/semantics.c
> 
> @@ -7090,6 +7090,7 @@
> 
>   finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
> 
> "array section does not have mappable type "
> "in %qs clause",
> omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> + cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
> remove = true;
> }
> while (TREE_CODE (t) == ARRAY_REF)
> 
> @@ -7158,6 +7159,7 @@
> 
>   finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
> 
> error_at (OMP_CLAUSE_LOCATION (c),
> "%qE does not have a mappable type in %qs clause",
> t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> + cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
> remove = true;
> }
> while (TREE_CODE (t) == COMPONENT_REF)
> 
> @@ -7236,6 +7238,7 @@
> 
>   finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
> 
> error_at (OMP_CLAUSE_LOCATION (c),
> "%qD does not have a mappable type in %qs clause", t,
> omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> + cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
> remove = true;
> }
> else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> 
> @@ -7384,6 +7387,7 @@
> 
>   finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
> 
> error_at (OMP_CLAUSE_LOCATION (c),
> "%qD does not have a mappable type in %qs clause", t,
> omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> + cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
> remove = true;
> }
> if (remove)
> 
> =======================================
> 
> diff --git a/gcc/testsuite/g++.dg/gomp/unmappable-1.C b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
> new file mode 100644
> index 00000000000..d00ccb5ad79
> 
> --- /dev/null
> 
> +++ b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
> 
> @@ -0,0 +1,20 @@
> 
> +/* { dg-do compile } */
> +/* { dg-options "-fopenmp" } */
> +
> +class C /* { dg-message "type .C. with virtual members is not mappable" 
> } */
> +{
> +public:
> + static int static_member; /* { dg-message "static field 
> .C::static_member. is not mappable" } */
> + virtual void f() {}
> +};
> +
> +extern C v[];
> +
> +int
> +main ()
> +{
> +#pragma omp target map(v) /* { dg-error ".v. does not have a mappable 
> type in .map. clause" } */
> + /* { dg-message "incomplete type .C \\\[\\\]. is not mappable" "" { 
> target *-*-* } .-1 } */
> + {
> + }
> +}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-07-04 12:14       ` Andrew Stubbs
@ 2019-07-08 22:13         ` Jakub Jelinek
  2019-07-09  9:59           ` Andrew Stubbs
  2019-07-09 11:03           ` [OG9] " Andrew Stubbs
  0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2019-07-08 22:13 UTC (permalink / raw)
  To: Andrew Stubbs; +Cc: Jason Merrill, gcc-patches

On Thu, Jul 04, 2019 at 12:44:32PM +0100, Andrew Stubbs wrote:
> On 03/07/2019 18:58, Jason Merrill wrote:
> > OK, thanks.
> 
> Committed.

This broke following testcase.
error_mark_node type isn't really incomplete, it is errorneous, doesn't have
TYPE_MAIN_DECL and we should have diagnosed it earlier, so it makes no sense
to emit extra explanation messages.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.

2019-07-08  Jakub Jelinek  <jakub@redhat.com>

	PR c++/91110
	* decl2.c (cp_omp_mappable_type_1): Don't emit any note for
	error_mark_node type.

	* g++.dg/gomp/pr91110.C: New test.

--- gcc/cp/decl2.c.jj	2019-07-04 23:39:02.579106113 +0200
+++ gcc/cp/decl2.c	2019-07-08 13:22:52.552898230 +0200
@@ -1416,7 +1416,7 @@ cp_omp_mappable_type_1 (tree type, bool
   /* Mappable type has to be complete.  */
   if (type == error_mark_node || !COMPLETE_TYPE_P (type))
     {
-      if (notes)
+      if (notes && type != error_mark_node)
 	{
 	  tree decl = TYPE_MAIN_DECL (type);
 	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
--- gcc/testsuite/g++.dg/gomp/pr91110.C.jj	2019-07-08 13:29:43.803163534 +0200
+++ gcc/testsuite/g++.dg/gomp/pr91110.C	2019-07-08 13:29:17.550593456 +0200
@@ -0,0 +1,11 @@
+// PR c++/91110
+// { dg-do compile }
+
+void
+foo ()
+{
+  X b[2];	// { dg-error "'X' was not declared in this scope" }
+  b[0] = 1;	// { dg-error "'b' was not declared in this scope" }
+  #pragma omp target map(to: b)	// { dg-error "'b' does not have a mappable type in 'map' clause" }
+  ;
+}


	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch, c++ openmp] Improve diagnostics for unmappable types
  2019-07-08 22:13         ` Jakub Jelinek
@ 2019-07-09  9:59           ` Andrew Stubbs
  2019-07-09 11:03           ` [OG9] " Andrew Stubbs
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Stubbs @ 2019-07-09  9:59 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches

On 08/07/2019 23:10, Jakub Jelinek wrote:
> This broke following testcase.
> error_mark_node type isn't really incomplete, it is errorneous, doesn't have
> TYPE_MAIN_DECL and we should have diagnosed it earlier, so it makes no sense
> to emit extra explanation messages.

Apologies. Did I miss something in the regression tests? _Atomic-5 seems 
fine?

Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [OG9] Improve diagnostics for unmappable types
  2019-07-08 22:13         ` Jakub Jelinek
  2019-07-09  9:59           ` Andrew Stubbs
@ 2019-07-09 11:03           ` Andrew Stubbs
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Stubbs @ 2019-07-09 11:03 UTC (permalink / raw)
  Cc: gcc-patches

I've backported Jakub's patch to openacc-gcc-9-branch.

Andrew

On 08/07/2019 23:10, Jakub Jelinek wrote:
> On Thu, Jul 04, 2019 at 12:44:32PM +0100, Andrew Stubbs wrote:
>> On 03/07/2019 18:58, Jason Merrill wrote:
>>> OK, thanks.
>>
>> Committed.
> 
> This broke following testcase.
> error_mark_node type isn't really incomplete, it is errorneous, doesn't have
> TYPE_MAIN_DECL and we should have diagnosed it earlier, so it makes no sense
> to emit extra explanation messages.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> committed to trunk.
> 
> 2019-07-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/91110
> 	* decl2.c (cp_omp_mappable_type_1): Don't emit any note for
> 	error_mark_node type.
> 
> 	* g++.dg/gomp/pr91110.C: New test.
> 
> --- gcc/cp/decl2.c.jj	2019-07-04 23:39:02.579106113 +0200
> +++ gcc/cp/decl2.c	2019-07-08 13:22:52.552898230 +0200
> @@ -1416,7 +1416,7 @@ cp_omp_mappable_type_1 (tree type, bool
>     /* Mappable type has to be complete.  */
>     if (type == error_mark_node || !COMPLETE_TYPE_P (type))
>       {
> -      if (notes)
> +      if (notes && type != error_mark_node)
>   	{
>   	  tree decl = TYPE_MAIN_DECL (type);
>   	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
> --- gcc/testsuite/g++.dg/gomp/pr91110.C.jj	2019-07-08 13:29:43.803163534 +0200
> +++ gcc/testsuite/g++.dg/gomp/pr91110.C	2019-07-08 13:29:17.550593456 +0200
> @@ -0,0 +1,11 @@
> +// PR c++/91110
> +// { dg-do compile }
> +
> +void
> +foo ()
> +{
> +  X b[2];	// { dg-error "'X' was not declared in this scope" }
> +  b[0] = 1;	// { dg-error "'b' was not declared in this scope" }
> +  #pragma omp target map(to: b)	// { dg-error "'b' does not have a mappable type in 'map' clause" }
> +  ;
> +}
> 
> 
> 	Jakub
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-07-09 11:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-28 10:46 [patch, c++ openmp] Improve diagnostics for unmappable types Andrew Stubbs
2019-06-28 16:21 ` Jason Merrill
2019-07-01 11:16   ` Andrew Stubbs
2019-07-03 18:36     ` Jason Merrill
2019-07-04 12:14       ` Andrew Stubbs
2019-07-08 22:13         ` Jakub Jelinek
2019-07-09  9:59           ` Andrew Stubbs
2019-07-09 11:03           ` [OG9] " Andrew Stubbs
2019-07-04 14:38     ` Andrew Stubbs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).