[patch, c++ openmp] Improve diagnostics for unmappable types

[patch, c++ openmp] Improve diagnostics for unmappable types

[Andrew Stubbs]

[-- 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" } */
+  {
+  }
+}

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Jason Merrill]

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

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Andrew Stubbs]

[-- 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 } */
+  {
+  }
+}

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Jason Merrill]

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

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Andrew Stubbs]

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

Committed.

Thanks for the reviews.

Andrew

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Jakub Jelinek]

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

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

[Andrew Stubbs]

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

[OG9] Improve diagnostics for unmappable types

[Andrew Stubbs]

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
> 

[OG9] Improve diagnostics for unmappable types

[Andrew Stubbs]

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 } */
> + {
> + }
> +}