[PATCH] Fix compile-cplus-types.c build errors

[PATCH] Fix compile-cplus-types.c build errors

[Simon Marchi]

I see these errors when building with clang:

  CXX    compile/compile-cplus-types.o
/home/emaisin/src/binutils-gdb/gdb/compile/compile-cplus-types.c:262:61: error: cannot pass non-trivial object of type 'compile_scope' to variadic function; expected type from format string was 'void *' [-Wnon-pod-varargs]
        fprintf_unfiltered (gdb_stdlog, "entering new scope %p\n", new_scope);
                                                            ~~     ^~~~~~~~~
/home/emaisin/src/binutils-gdb/gdb/compile/compile-cplus-types.c:306:56: error: cannot pass non-trivial object of type 'compile_scope' to variadic function; expected type from format string was 'void *' [-Wnon-pod-varargs]
        fprintf_unfiltered (gdb_stdlog, "leaving scope %p\n", current);
                                                       ~~     ^~~~~~~
/home/emaisin/src/binutils-gdb/gdb/compile/compile-cplus-types.c:1058:13: error: comparison of two values with different enumeration types ('enum_flags<gcc_cp_qualifiers>::enum_type' (aka 'gcc_cp_qualifiers') and 'gcc_cp_ref_qualifiers') [-Werror,-Wenum-compare]
  if (quals != GCC_CP_REF_QUAL_NONE)
      ~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~

Fix the first two by taking the address of the object.

Fix the third one by comparing to 0 instead.  I think the current
comparison simply uses the wrong enum type.  Comparing to 0 seems like
the right thing to do, because we want to check whether any flags are
specified.

gdb/ChangeLog:

	* compile/compile-cplus-types.c
	(compile_cplus_instance::enter_scope): Take the address of scope
	object.
	(compile_cplus_instance::leave_scope): Likewise.
	(compile_cplus_instance::convert_qualified_base): Compare quals
	to 0.
---
 gdb/compile/compile-cplus-types.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 9425fc6..844c8ce 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -259,7 +259,7 @@ compile_cplus_instance::enter_scope (compile_scope &new_scope)
   if (must_push)
     {
       if (debug_compile_cplus_scopes)
-	fprintf_unfiltered (gdb_stdlog, "entering new scope %p\n", new_scope);
+	fprintf_unfiltered (gdb_stdlog, "entering new scope %p\n", &new_scope);
 
       /* Push the global namespace. */
       plugin ().push_namespace ("");
@@ -303,7 +303,7 @@ compile_cplus_instance::leave_scope ()
   if (current.m_pushed)
     {
       if (debug_compile_cplus_scopes)
-	fprintf_unfiltered (gdb_stdlog, "leaving scope %p\n", current);
+	fprintf_unfiltered (gdb_stdlog, "leaving scope %p\n", &current);
 
       /* Pop namespaces.  */
       std::for_each
@@ -1055,7 +1055,7 @@ compile_cplus_instance::convert_qualified_base (gcc_type base,
 {
   gcc_type result = base;
 
-  if (quals != GCC_CP_REF_QUAL_NONE)
+  if (quals != 0)
     result = plugin ().build_qualified_type (base, quals);
 
   return result;
-- 
2.7.4

Re: [PATCH] Fix compile-cplus-types.c build errors

[Joel Brobecker]

Hi Simon,

> gdb/ChangeLog:
> 
> 	* compile/compile-cplus-types.c
> 	(compile_cplus_instance::enter_scope): Take the address of scope
> 	object.
> 	(compile_cplus_instance::leave_scope): Likewise.
> 	(compile_cplus_instance::convert_qualified_base): Compare quals
> 	to 0.
[...]
> diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
> index 9425fc6..844c8ce 100644
> --- a/gdb/compile/compile-cplus-types.c
> +++ b/gdb/compile/compile-cplus-types.c
> @@ -259,7 +259,7 @@ compile_cplus_instance::enter_scope (compile_scope &new_scope)
>    if (must_push)
>      {
>        if (debug_compile_cplus_scopes)
> -	fprintf_unfiltered (gdb_stdlog, "entering new scope %p\n", new_scope);
> +	fprintf_unfiltered (gdb_stdlog, "entering new scope %p\n", &new_scope);

While at it, can you change the use of %p by using %s combined with
host_address_to_string? Using %p triggers an ARI warning.

> @@ -1055,7 +1055,7 @@ compile_cplus_instance::convert_qualified_base (gcc_type base,
>  {
>    gcc_type result = base;
>  
> -  if (quals != GCC_CP_REF_QUAL_NONE)
> +  if (quals != 0)
>      result = plugin ().build_qualified_type (base, quals);
>  
>    return result;
> -- 
> 2.7.4

-- 
Joel

Re: [PATCH] Fix compile-cplus-types.c build errors

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> Fix the third one by comparing to 0 instead.  I think the current
Simon> comparison simply uses the wrong enum type.  Comparing to 0 seems like
Simon> the right thing to do, because we want to check whether any flags are
Simon> specified.

I think this is fine, but at the same time, it seems reasonable to me to
want to compare an enum flags object against an enumerator from the
underlying type.  So maybe enum_flags should have operator== and operator!=.

What do you think of this?  I am trying to think of a counter-example
where this would cause problems.

Simon> 	* compile/compile-cplus-types.c
Simon> 	(compile_cplus_instance::enter_scope): Take the address of scope
Simon> 	object.
Simon> 	(compile_cplus_instance::leave_scope): Likewise.
Simon> 	(compile_cplus_instance::convert_qualified_base): Compare quals
Simon> 	to 0.

Looks good.

Tom

Re: [PATCH] Fix compile-cplus-types.c build errors

[Simon Marchi]

On 2018-08-30 11:06 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> Fix the third one by comparing to 0 instead.  I think the current
> Simon> comparison simply uses the wrong enum type.  Comparing to 0 seems like
> Simon> the right thing to do, because we want to check whether any flags are
> Simon> specified.
> 
> I think this is fine, but at the same time, it seems reasonable to me to
> want to compare an enum flags object against an enumerator from the
> underlying type.  So maybe enum_flags should have operator== and operator!=.
> 
> What do you think of this?  I am trying to think of a counter-example
> where this would cause problems.

I think this is already possible (but I have not tested).  In this case, it's
actually comparing against an enumerator from a completely different enum type.

Here's what I pushed.  Keith has already fixed the first error, this fixes the
others.

@Joel, I used host_address_to_string.


From a0dc02a6df86011462293cc11b4e35c0f18effd0 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 30 Aug 2018 11:09:48 -0400
Subject: [PATCH] Fix compile-cplus-types.c build errors

I see these errors when building with clang:

  CXX    compile/compile-cplus-types.o
/home/emaisin/src/binutils-gdb/gdb/compile/compile-cplus-types.c:306:56: error: cannot pass non-trivial object of type 'compile_scope' to variadic function; expected type from format string was 'void *' [-Wnon-pod-varargs]
        fprintf_unfiltered (gdb_stdlog, "leaving scope %p\n", current);
                                                       ~~     ^~~~~~~
/home/emaisin/src/binutils-gdb/gdb/compile/compile-cplus-types.c:1058:13: error: comparison of two values with different enumeration types ('enum_flags<gcc_cp_qualifiers>::enum_type' (aka 'gcc_cp_qualifiers') and 'gcc_cp_ref_qualifiers') [-Werror,-Wenum-compare]
  if (quals != GCC_CP_REF_QUAL_NONE)
      ~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~

Fix the first one by using host_address_to_string.

Fix the second one by comparing to 0 instead.  I think the current
comparison simply uses the wrong enum type.  Comparing to 0 seems like
the right thing to do, because we want to check whether any flags are
specified.

gdb/ChangeLog:

	* compile/compile-cplus-types.c
	(compile_cplus_instance::leave_scope): Take the address of scope
	object.
	(compile_cplus_instance::convert_qualified_base): Compare quals
	to 0.
---
 gdb/ChangeLog                     | 8 ++++++++
 gdb/compile/compile-cplus-types.c | 7 +++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e59caf..66c2e33 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2018-08-30  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* compile/compile-cplus-types.c
+	(compile_cplus_instance::leave_scope): Take the address of scope
+	object.
+	(compile_cplus_instance::convert_qualified_base): Compare quals
+	to 0.
+
 2018-08-30  Keith Seitz  <keiths@redhat.com>

 	* compile/compile-cplus-types.c (compile_cplus_instance::enter_scope):
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index e86a573..7fc4136 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -306,7 +306,10 @@ compile_cplus_instance::leave_scope ()
   if (current.m_pushed)
     {
       if (debug_compile_cplus_scopes)
-	fprintf_unfiltered (gdb_stdlog, "leaving scope %p\n", current);
+	{
+	  fprintf_unfiltered (gdb_stdlog, "leaving scope %s\n",
+			      host_address_to_string (&current));
+	}

       /* Pop namespaces.  */
       std::for_each
@@ -1058,7 +1061,7 @@ compile_cplus_instance::convert_qualified_base (gcc_type base,
 {
   gcc_type result = base;

-  if (quals != GCC_CP_REF_QUAL_NONE)
+  if (quals != 0)
     result = plugin ().build_qualified_type (base, quals);

   return result;
-- 
2.7.4

Re: [PATCH] Fix compile-cplus-types.c build errors

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I think this is already possible (but I have not tested).  In this case, it's
Simon> actually comparing against an enumerator from a completely different enum type.

Yeah, you mentioned that, and I looked at the code and then missed the
"_ref" in there.  Sorry about that, I agree with your patch.

Tom