Patchwork [C++] PR 14263

login
register
mail settings
Submitter Paolo Carlini
Date July 5, 2013, 2:43 p.m.
Message ID <51D6DB8A.7030507@oracle.com>
Download mbox | patch
Permalink /patch/257181/
State New
Headers show

Comments

Paolo Carlini - July 5, 2013, 2:43 p.m.
Hi,

this issue dates back to 2004 and got stalled pretty soon. Essentially 
Gaby wanted to see *dynamic_cast* explicitly mentioned in this kind of 
diagnostic (we used to explicitly talk about static_cast), whereas Mark 
found the current status an improvement over the past, proposed some 
further, minor, improvements (which I tried to implement in the below)

Today I noticed that current ICC, clang, OracleStudio, all produce 
diagnostic quite similar, modulo the latter improvements, to GCC and 
decided to explore whether Gaby (and Jason) would like to reassess the 
bug, whether today we want to talk about dynamic_cast.

Tested x86_64-linux.

Thanks,
Paolo.

////////////////////////////
/cp
2013-07-05  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14263
	* class.c (build_base_path): Improve diagnostic.

/testsuite
2013-07-05  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14263
	* g++.dg/inherit/virtual10.C: New.
Gabriel Dos Reis - July 5, 2013, 3:36 p.m.
Paolo Carlini <paolo.carlini@oracle.com> writes:

| Hi,
| 
| this issue dates back to 2004 and got stalled pretty soon. Essentially
| Gaby wanted to see *dynamic_cast* explicitly mentioned in this kind of
| diagnostic (we used to explicitly talk about static_cast), whereas
| Mark found the current status an improvement over the past, proposed
| some further, minor, improvements (which I tried to implement in the
| below)
| 
| Today I noticed that current ICC, clang, OracleStudio, all produce
| diagnostic quite similar, modulo the latter improvements, to GCC and
| decided to explore whether Gaby (and Jason) would like to reassess the
| bug, whether today we want to talk about dynamic_cast.
| 
| Tested x86_64-linux.
| 
| Thanks,
| Paolo.

I still consider talk of 'static_cast' in this context a non-starter.
However, maybe just saying "convert" would be OK -- though clearly
giving hints of dynamic_cast is much better.

Minor nits:  say "base class" instead of "base", and "derived class"
instead of "derived type".

Otherwise, patch OK with these suggestions.

-- Gaby
Paolo Carlini - July 5, 2013, 3:50 p.m.
Hi,

On 07/05/2013 05:36 PM, Gabriel Dos Reis wrote:
> Paolo Carlini <paolo.carlini@oracle.com> writes:
>
> | Hi,
> |
> | this issue dates back to 2004 and got stalled pretty soon. Essentially
> | Gaby wanted to see *dynamic_cast* explicitly mentioned in this kind of
> | diagnostic (we used to explicitly talk about static_cast), whereas
> | Mark found the current status an improvement over the past, proposed
> | some further, minor, improvements (which I tried to implement in the
> | below)
> |
> | Today I noticed that current ICC, clang, OracleStudio, all produce
> | diagnostic quite similar, modulo the latter improvements, to GCC and
> | decided to explore whether Gaby (and Jason) would like to reassess the
> | bug, whether today we want to talk about dynamic_cast.
> |
> | Tested x86_64-linux.
> |
> | Thanks,
> | Paolo.
>
> I still consider talk of 'static_cast' in this context a non-starter.
> However, maybe just saying "convert" would be OK -- though clearly
> giving hints of dynamic_cast is much better.
To be clear, currently we do *not* talk of static_cast, likewise the 
other compilers I have access to. We say "cannot convert ...".
> Minor nits:  say "base class" instead of "base", and "derived class"
> instead of "derived type".
I see, I wondered whether that would be better.
> Otherwise, patch OK with these suggestions.
Great, thanks a lot.

Paolo.
Jason Merrill - July 5, 2013, 4:38 p.m.
On 07/05/2013 08:36 AM, Gabriel Dos Reis wrote:
> I still consider talk of 'static_cast' in this context a non-starter.
> However, maybe just saying "convert" would be OK -- though clearly
> giving hints of dynamic_cast is much better.

I wouldn't mind mentioning dynamic_cast, along the lines that Gaby 
suggested in the PR.

Jason
Paolo Carlini - July 5, 2013, 4:55 p.m.
Hi,

Jason Merrill <jason@redhat.com> ha scritto:

>I wouldn't mind mentioning dynamic_cast, along the lines that Gaby
>suggested in the PR.

Sure, but let me think about it, because isn't immediately obvious (to me at least!) *when* that would really help.

Paolo

Patch

Index: cp/class.c
===================================================================
--- cp/class.c	(revision 200681)
+++ cp/class.c	(working copy)
@@ -291,9 +291,31 @@  build_base_path (enum tree_code code,
   if (code == MINUS_EXPR && v_binfo)
     {
       if (complain & tf_error)
-	error ("cannot convert from base %qT to derived type %qT via "
-	       "virtual base %qT", BINFO_TYPE (binfo), BINFO_TYPE (d_binfo),
-	       BINFO_TYPE (v_binfo));
+	{
+	  if (SAME_BINFO_TYPE_P (BINFO_TYPE (binfo), BINFO_TYPE (v_binfo)))
+	    {
+	      if (want_pointer)
+		error ("cannot convert from pointer to base %qT to pointer "
+		       "to derived type %qT because the base is virtual",
+		       BINFO_TYPE (binfo), BINFO_TYPE (d_binfo));
+	      else
+		error ("cannot convert from base %qT to derived type %qT "
+		       "because the base is virtual", BINFO_TYPE (binfo),
+		       BINFO_TYPE (d_binfo));
+	    }	      
+	  else
+	    {
+	      if (want_pointer)
+		error ("cannot convert from pointer to base %qT to pointer "
+		       "to derived type %qT via virtual base %qT",
+		       BINFO_TYPE (binfo), BINFO_TYPE (d_binfo),
+		       BINFO_TYPE (v_binfo));
+	      else
+		error ("cannot convert from base %qT to derived type %qT via "
+		       "virtual base %qT", BINFO_TYPE (binfo),
+		       BINFO_TYPE (d_binfo), BINFO_TYPE (v_binfo));
+	    }
+	}
       return error_mark_node;
     }
 
Index: testsuite/g++.dg/inherit/virtual10.C
===================================================================
--- testsuite/g++.dg/inherit/virtual10.C	(revision 0)
+++ testsuite/g++.dg/inherit/virtual10.C	(working copy)
@@ -0,0 +1,11 @@ 
+// PR c++/14263
+
+struct A { };
+
+struct B : virtual A { };
+
+int main()
+{
+   A* p = new B;
+   B* q = static_cast<B*>(p);  // { dg-error "cannot convert from pointer to base 'A' to pointer to derived type 'B' because the base is virtual" }
+}